Uncached client for instance admission webhook (#1516)

Summary:
we've seen increased flakiness lately with some ITs e.g. `terminal-failed-job` and `dependencies-hash` where a previously created `OperatorVersion` is not observed by the instance admission webhook. This PR switches to an uncached client for the IAW which should alleviate the problem.

Signed-off-by: Aleksey Dukhovniy <alex.dukhovniy@googlemail.com>

cmd/manager/main.go

@@ -135,11 +135,17 @@ func main() {
 	if strings.ToLower(os.Getenv("ENABLE_WEBHOOKS")) == "true" {
 		log.Printf("Setting up webhooks")
 
-		if err := registerWebhook("/admit", &v1beta1.Instance{}, &webhook.Admission{Handler: &kudohook.InstanceAdmission{}}, mgr); err != nil {
+		iac, err := kudohook.NewInstanceAdmission(mgr.GetConfig(), mgr.GetScheme())
+		if err != nil {
+			log.Printf("Unable to create an uncached client for the webhook: %v", err)
+			os.Exit(1)
+		}
+
+		if err := registerWebhook("/admit", &v1beta1.Instance{}, &webhook.Admission{Handler: iac}, mgr); err != nil {
 			log.Printf("Unable to create instance admission webhook: %v", err)
 			os.Exit(1)
 		}
-		log.Printf("Instance admission webhook")
+		log.Printf("Instance admission webhook set up")
 
 		// Add more webhooks below using the above registerWebhook method
 	}

pkg/webhook/instance_admission.go

@@ -10,7 +10,9 @@ import (
 
 	"github.com/thoas/go-funk"
 	"k8s.io/api/admission/v1beta1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
@@ -26,6 +28,20 @@ type InstanceAdmission struct {
 	decoder *admission.Decoder
 }
 
+func NewInstanceAdmission(cfg *rest.Config, s *runtime.Scheme) (*InstanceAdmission, error) {
+	// client.New returns a new Client using the provided config and Options.
+	// The returned client reads *and* writes directly from the server
+	// (it doesn't use object caches). Using a cached client might lead to racy
+	// behaviour when installing operators e.g. and `OperatorVersion` is already created
+	// but not yet in cache which leads to an error during `Instance` creation.
+	c, err := client.New(cfg, client.Options{Scheme: s})
+	if err != nil {
+		return nil, err
+	}
+
+	return &InstanceAdmission{client: c}, nil
+}
+
 // InstanceAdmission validates updates to an Instance, guarding from conflicting plan executions
 func (ia *InstanceAdmission) Handle(ctx context.Context, req admission.Request) admission.Response {
 
@@ -341,15 +357,6 @@ func changedParameterDefinitions(old map[string]string, new map[string]string, o
 	return append(cpd, rpd...), nil
 }
 
-// InstanceAdmission implements inject.Client.
-// A client will be automatically injected.
-
-// InjectClient injects the client.
-func (ia *InstanceAdmission) InjectClient(c client.Client) error {
-	ia.client = c
-	return nil
-}
-
 // InstanceAdmission implements admission.DecoderInjector.
 // A decoder will be automatically injected.
 

pkg/webhook/instance_admission_integration_test.go

@@ -82,23 +82,23 @@ var _ = Describe("Test", func() {
 		err = apis.AddToScheme(mgr.GetScheme())
 		Expect(err).NotTo(HaveOccurred())
 
-		// 3. registering instance admission controller
+		// 3. creating the client. **Note:** client.New method will create an uncached client, a cached one
+		// (e.g. mgr.GetClient) leads to caching issues in this test.
+		log.Print("test.BeforeEach: initializing client")
+		c, err = client.New(env.Config, client.Options{Scheme: mgr.GetScheme()})
+		Expect(err).NotTo(HaveOccurred())
+
+		// 4. registering instance admission controller
 		log.Print("test.BeforeEach: initializing webhook server")
 		server := mgr.GetWebhookServer()
-		server.Register(instanceAdmissionWebhookPath, &ctrhook.Admission{Handler: &InstanceAdmission{}})
+		server.Register(instanceAdmissionWebhookPath, &ctrhook.Admission{Handler: &InstanceAdmission{client: c}})
 
-		// 4. starting the manager
+		// 5. starting the manager
 		stop = make(chan struct{})
 		go func() {
 			err = mgr.Start(stop)
 			Expect(err).NotTo(HaveOccurred())
 		}()
-
-		// 5. creating the client. **Note:** client.New method will create an uncached client, a cached one
-		// (e.g. mgr.GetClient) leads to caching issues in this test.
-		log.Print("test.BeforeEach: initializing client")
-		c, err = client.New(env.Config, client.Options{Scheme: mgr.GetScheme()})
-		Expect(err).NotTo(HaveOccurred())
 	})
 
 	AfterEach(func() {