Merge pull request openshift#1605 from alvaroaleman/cpo-eventhandling

CPO: Unittest and fix eventhandling of HCP controller
gnufied · Jul 29, 2022 · 27384bf · 27384bf
2 parents 1cb59dd + f5824ab
commit 27384bf
Show file tree

Hide file tree

Showing 5 changed files with 658 additions and 264 deletions.
diff --git a/control-plane-operator/controllers/hostedcontrolplane/creatorupdate_ownerref_enforcer.go b/control-plane-operator/controllers/hostedcontrolplane/creatorupdate_ownerref_enforcer.go
@@ -0,0 +1,34 @@
+package hostedcontrolplane
+
+import (
+	"context"
+
+	hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
+	"github.com/openshift/hypershift/support/config"
+	"github.com/openshift/hypershift/support/upsert"
+	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+)
+
+func createOrUpdateWithOwnerRefFactory(upstream upsert.CreateOrUpdateFN) func(*hyperv1.HostedControlPlane) upsert.CreateOrUpdateFN {
+	return func(owner *hyperv1.HostedControlPlane) upsert.CreateOrUpdateFN {
+		ownerRef := config.OwnerRefFrom(owner)
+		return func(ctx context.Context, c crclient.Client, obj crclient.Object, f controllerutil.MutateFn) (controllerutil.OperationResult, error) {
+
+			// Wrap f() to add the ownerRef at the end
+			mutateFN := func() error {
+				if err := f(); err != nil {
+					return err
+				}
+				if obj.GetNamespace() == "" {
+					// don't tag cluster scoped resources
+					return nil
+				}
+				ownerRef.ApplyTo(obj)
+				return nil
+			}
+
+			return upstream(ctx, c, obj, mutateFN)
+		}
+	}
+}
diff --git a/...rol-plane-operator/controllers/hostedcontrolplane/creatorupdate_ownerref_enforcer_test.go b/...rol-plane-operator/controllers/hostedcontrolplane/creatorupdate_ownerref_enforcer_test.go
@@ -0,0 +1,86 @@
+package hostedcontrolplane
+
+import (
+	"context"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
+	"github.com/openshift/hypershift/support/config"
+	"github.com/openshift/hypershift/support/upsert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+)
+
+func TestCreateOrUpdateWithOwnerRefFactory(t *testing.T) {
+
+	hcp := &hyperv1.HostedControlPlane{ObjectMeta: metav1.ObjectMeta{Namespace: "bar", Name: "foo"}}
+	ownerRef := config.OwnerRefFrom(hcp)
+	testCases := []struct {
+		name     string
+		obj      crclient.Object
+		expected []metav1.OwnerReference
+		mutateFN func(crclient.Object) controllerutil.MutateFn
+	}{
+		{
+			name: "Owner ref is added",
+			obj: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "bar",
+				},
+			},
+			expected: []metav1.OwnerReference{*ownerRef.Reference},
+		},
+		{
+			name: "Adding takes precedence",
+			obj: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: "bar",
+				},
+			},
+			mutateFN: func(o crclient.Object) controllerutil.MutateFn {
+				return func() error {
+					o.SetOwnerReferences(nil)
+					return nil
+				}
+			},
+			expected: []metav1.OwnerReference{*ownerRef.Reference},
+		},
+		{
+			name: "Do not add ownerRef to cluster scoped resources",
+			obj: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "foo",
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			obj := tc.obj
+			if tc.mutateFN == nil {
+				tc.mutateFN = func(crclient.Object) controllerutil.MutateFn {
+					return func() error { return nil }
+				}
+			}
+			client := fake.NewClientBuilder().WithObjects(obj).Build()
+			providerFactory := createOrUpdateWithOwnerRefFactory(upsert.New(false).CreateOrUpdate)
+			if _, err := providerFactory(hcp)(context.Background(), client, obj, tc.mutateFN(obj)); err != nil {
+				t.Fatalf("CreateOrUpdate failed: %v", err)
+			}
+
+			if err := client.Get(context.Background(), crclient.ObjectKeyFromObject(obj), obj); err != nil {
+				t.Fatalf("failed to get object from client after running CreateOrUpdate: %v", err)
+			}
+			if diff := cmp.Diff(tc.expected, obj.GetOwnerReferences()); diff != "" {
+				t.Errorf("actual differs from expected: %v", diff)
+			}
+		})
+	}
+}