From c7cd50ecb224dd3bf7c8c5b04939a2a1387ad8c8 Mon Sep 17 00:00:00 2001 From: Rewant Soni Date: Tue, 27 Aug 2024 17:15:08 +0530 Subject: [PATCH] fix unit tests Signed-off-by: Rewant Soni --- .../storagecluster/cephblockpools_test.go | 4 +- .../storagecluster/cephrbdmirrors_test.go | 4 +- .../initialization_reconciler_test.go | 82 +++++++------- .../storagecluster/provider_server_test.go | 100 +++--------------- .../storagecluster_controller_test.go | 54 +++++++++- 5 files changed, 115 insertions(+), 129 deletions(-) diff --git a/controllers/storagecluster/cephblockpools_test.go b/controllers/storagecluster/cephblockpools_test.go index d3690a55c8..b24d2908a0 100644 --- a/controllers/storagecluster/cephblockpools_test.go +++ b/controllers/storagecluster/cephblockpools_test.go @@ -83,6 +83,8 @@ func TestInjectingPeerTokenToCephBlockPool(t *testing.T) { }, } + obj := &ocsCephBlockPools{} + for _, c := range cases { cr := getInitData(c.spec) request := reconcile.Request{ @@ -92,7 +94,7 @@ func TestInjectingPeerTokenToCephBlockPool(t *testing.T) { }, } reconciler := createReconcilerFromCustomResources(t, cr) - _, err := reconciler.Reconcile(context.TODO(), request) + _, err := obj.ensureCreated(&reconciler, cr) assert.NoError(t, err) if c.label == "test-injecting-peer-token-to-cephblockpool" { assertCephBlockPools(t, reconciler, cr, request, true, true) diff --git a/controllers/storagecluster/cephrbdmirrors_test.go b/controllers/storagecluster/cephrbdmirrors_test.go index 83ef424c1b..8b820272ab 100644 --- a/controllers/storagecluster/cephrbdmirrors_test.go +++ b/controllers/storagecluster/cephrbdmirrors_test.go @@ -41,6 +41,8 @@ func TestCephRbdMirror(t *testing.T) { }, } + obj := &ocsCephRbdMirrors{} + for _, c := range cases { cr := getInitData(c.spec) request := reconcile.Request{ @@ -50,7 +52,7 @@ func TestCephRbdMirror(t *testing.T) { }, } reconciler := createReconcilerFromCustomResources(t, cr) - _, err := reconciler.Reconcile(context.TODO(), request) + _, err := obj.ensureCreated(&reconciler, cr) assert.NoError(t, err) switch c.label { case "create-ceph-rbd-mirror": diff --git a/controllers/storagecluster/initialization_reconciler_test.go b/controllers/storagecluster/initialization_reconciler_test.go index dd39da05dd..bfb444ded2 100644 --- a/controllers/storagecluster/initialization_reconciler_test.go +++ b/controllers/storagecluster/initialization_reconciler_test.go @@ -177,50 +177,12 @@ func initStorageClusterResourceCreateUpdateTestProviderMode(t *testing.T, runtim } if remoteConsumers { - node := &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "node-role.kubernetes.io/worker": "", - }, - }, - Status: v1.NodeStatus{ - Addresses: []v1.NodeAddress{ - { - Type: v1.NodeInternalIP, - Address: "0:0:0:0", - }, - }, - }, - } - - os.Setenv(providerAPIServerImage, "fake-image") - os.Setenv(util.WatchNamespaceEnvVar, "") - os.Setenv(onboardingValidationKeysGeneratorImage, "fake-image") - - deployment := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, - } - deployment.Status.AvailableReplicas = 1 - - service := &v1.Service{ - ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, - } - service.Status.LoadBalancer.Ingress = []v1.LoadBalancerIngress{ - { - Hostname: "fake", - }, - } - - secret := &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, - } clientConfigMap := &v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: ocsClientConfigMapName}, } - addedRuntimeObjects := []runtime.Object{node, service, deployment, secret, clientConfigMap} - rtObjsToCreateReconciler = append(rtObjsToCreateReconciler, addedRuntimeObjects...) + rtObjsToCreateReconciler = append(rtObjsToCreateReconciler, clientConfigMap) util.AddAnnotation(cr, "ocs.openshift.io/deployment-mode", "provider") } @@ -358,6 +320,46 @@ func createFakeInitializationStorageClusterReconciler(t *testing.T, obj ...runti }, } + workerNode := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "workerNode", + Labels: map[string]string{ + "node-role.kubernetes.io/worker": "", + }, + }, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "0:0:0:0", + }, + }, + }, + } + + os.Setenv(providerAPIServerImage, "fake-image") + os.Setenv(onboardingValidationKeysGeneratorImage, "fake-image") + + ocsProviderServiceDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, + Status: appsv1.DeploymentStatus{ + AvailableReplicas: 1, + }, + } + + ocsProviderService := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, + Status: v1.ServiceStatus{ + LoadBalancer: v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{{Hostname: "fake"}}, + }, + }, + } + + ocsProviderServiceSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, + } + operatorNamespace := os.Getenv("OPERATOR_NAMESPACE") verOcs, err := semver.Make(ocsversion.Version) if err != nil { @@ -393,7 +395,7 @@ func createFakeInitializationStorageClusterReconciler(t *testing.T, obj ...runti } } - runtimeObjects = append(runtimeObjects, mockNodeList.DeepCopy(), cbp, cfs, cnfs, cnfsbp, cnfssvc, infrastructure, networkConfig, rookCephMonSecret, csv) + runtimeObjects = append(runtimeObjects, mockNodeList.DeepCopy(), cbp, cfs, cnfs, cnfsbp, cnfssvc, infrastructure, networkConfig, rookCephMonSecret, csv, workerNode, ocsProviderServiceSecret, ocsProviderServiceDeployment, ocsProviderService) client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(runtimeObjects...).WithStatusSubresource(statusSubresourceObjs...).Build() return StorageClusterReconciler{ diff --git a/controllers/storagecluster/provider_server_test.go b/controllers/storagecluster/provider_server_test.go index 0feaa33fe9..9059dc1f6c 100644 --- a/controllers/storagecluster/provider_server_test.go +++ b/controllers/storagecluster/provider_server_test.go @@ -25,9 +25,9 @@ import ( func TestOcsProviderServerEnsureCreated(t *testing.T) { - t.Run("Ensure that Deployment,Service,Secret is created when AllowRemoteStorageConsumers is enabled", func(t *testing.T) { + t.Run("Ensure that Deployment,Service is created when storageCluster is created", func(t *testing.T) { - r, instance := createSetupForOcsProviderTest(t, true, "") + r, instance := createSetupForOcsProviderTest(t, "") obj := &ocsProviderServer{} res, err := obj.ensureCreated(r, instance) @@ -81,22 +81,11 @@ func TestOcsProviderServerEnsureCreated(t *testing.T) { assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(service), service)) expectedService := GetProviderAPIServerServiceForTest(instance) assert.Equal(t, expectedService.Spec, service.Spec) - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, - } - assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(secret), secret)) - - cm := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: ocsClientConfigMapName}, - } - assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(cm), cm)) - assert.Equal(t, "true", cm.Data[deployCSIKey]) }) - t.Run("Ensure that Deployment,Service,Secret is created when AllowRemoteStorageConsumers and ProviderAPIServerServiceType set to loadBalancer", func(t *testing.T) { + t.Run("Ensure that Deployment,Service is created when ProviderAPIServerServiceType set to loadBalancer", func(t *testing.T) { - r, instance := createSetupForOcsProviderTest(t, true, corev1.ServiceTypeLoadBalancer) + r, instance := createSetupForOcsProviderTest(t, corev1.ServiceTypeLoadBalancer) obj := &ocsProviderServer{} res, err := obj.ensureCreated(r, instance) @@ -150,22 +139,11 @@ func TestOcsProviderServerEnsureCreated(t *testing.T) { assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(service), service)) expectedService := GetLoadBalancerProviderAPIServerServiceForTest(instance) assert.Equal(t, expectedService.Spec, service.Spec) - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, - } - assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(secret), secret)) - - cm := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: ocsClientConfigMapName}, - } - assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(cm), cm)) - assert.Equal(t, "true", cm.Data[deployCSIKey]) }) - t.Run("Ensure that Deployment,Service,Secret is created when AllowRemoteStorageConsumers and ProviderAPIServerServiceType set to ClusterIP", func(t *testing.T) { + t.Run("Ensure that Deployment,Service is created when ProviderAPIServerServiceType set to ClusterIP", func(t *testing.T) { - r, instance := createSetupForOcsProviderTest(t, true, corev1.ServiceTypeClusterIP) + r, instance := createSetupForOcsProviderTest(t, corev1.ServiceTypeClusterIP) obj := &ocsProviderServer{} res, err := obj.ensureCreated(r, instance) @@ -214,22 +192,11 @@ func TestOcsProviderServerEnsureCreated(t *testing.T) { assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(service), service)) expectedService := GetClusterIPProviderAPIServerServiceForTest(instance) assert.Equal(t, expectedService.Spec, service.Spec) - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, - } - assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(secret), secret)) - - cm := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: ocsClientConfigMapName}, - } - assert.NoError(t, r.Client.Get(context.TODO(), client.ObjectKeyFromObject(cm), cm)) - assert.Equal(t, "true", cm.Data[deployCSIKey]) }) - t.Run("Ensure that Service is not created when AllowRemoteStorageConsumers is enabled and ProviderAPIServerServiceType is set to any other value than NodePort, ClusterIP or LoadBalancer", func(t *testing.T) { + t.Run("Ensure that Service is not created when ProviderAPIServerServiceType is set to any other value than NodePort, ClusterIP or LoadBalancer", func(t *testing.T) { - r, instance := createSetupForOcsProviderTest(t, true, corev1.ServiceTypeExternalName) + r, instance := createSetupForOcsProviderTest(t, corev1.ServiceTypeExternalName) obj := &ocsProviderServer{} _, err := obj.ensureCreated(r, instance) @@ -239,40 +206,13 @@ func TestOcsProviderServerEnsureCreated(t *testing.T) { } assert.True(t, errors.IsNotFound(r.Client.Get(context.TODO(), client.ObjectKeyFromObject(service), service))) }) - - t.Run("Ensure that Deployment,Service,Secret is not created when AllowRemoteStorageConsumers is disabled", func(t *testing.T) { - - r, instance := createSetupForOcsProviderTest(t, false, "") - - obj := &ocsProviderServer{} - _, err := obj.ensureCreated(r, instance) - - assert.NoError(t, err) - - assertNotFoundProviderResources(t, r.Client) - }) } func TestOcsProviderServerEnsureDeleted(t *testing.T) { - t.Run("Ensure that Deployment,Service,Secret is deleted when AllowRemoteStorageConsumers is disabled", func(t *testing.T) { - - r, instance := createSetupForOcsProviderTest(t, true, "") - obj := &ocsProviderServer{} - // create resources and ignore error as it should be tested via TestOcsProviderServerEnsureCreated - _, _ = obj.ensureCreated(r, instance) - - instance.Spec.AllowRemoteStorageConsumers = false - // the resources will be deleted through the ensureCreated func as we are not in the deletion phase - _, err := obj.ensureCreated(r, instance) - assert.NoError(t, err) - - assertNotFoundProviderResources(t, r.Client) - }) + t.Run("Ensure that Deployment,Service is deleted while uninstalling", func(t *testing.T) { - t.Run("Ensure that Deployment,Service,Secret is deleted while uninstalling", func(t *testing.T) { - - r, instance := createSetupForOcsProviderTest(t, true, "") + r, instance := createSetupForOcsProviderTest(t, "") obj := &ocsProviderServer{} // create resources and ignore error as it should be tested via TestOcsProviderServerEnsureCreated _, _ = obj.ensureCreated(r, instance) @@ -303,7 +243,7 @@ func assertNotFoundProviderResources(t *testing.T, cli client.Client) { } -func createSetupForOcsProviderTest(t *testing.T, allowRemoteStorageConsumers bool, providerAPIServerServiceType corev1.ServiceType) (*StorageClusterReconciler, *ocsv1.StorageCluster) { +func createSetupForOcsProviderTest(t *testing.T, providerAPIServerServiceType corev1.ServiceType) (*StorageClusterReconciler, *ocsv1.StorageCluster) { node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ @@ -339,14 +279,12 @@ func createSetupForOcsProviderTest(t *testing.T, allowRemoteStorageConsumers boo instance := &ocsv1.StorageCluster{ Spec: ocsv1.StorageClusterSpec{ - AllowRemoteStorageConsumers: allowRemoteStorageConsumers, ProviderAPIServerServiceType: providerAPIServerServiceType, }, } os.Setenv(providerAPIServerImage, "fake-image") os.Setenv(onboardingValidationKeysGeneratorImage, "fake-image") - os.Setenv(util.WatchNamespaceEnvVar, "openshift-storage") return r, instance } @@ -379,16 +317,12 @@ func GetProviderAPIServerDeploymentForTest(instance *ocsv1.StorageCluster) *apps Command: []string{"/usr/local/bin/provider-api"}, Env: []corev1.EnvVar{ { - Name: util.WatchNamespaceEnvVar, - Value: os.Getenv(util.WatchNamespaceEnvVar), - }, - { - Name: "STORAGE_CLUSTER_NAME", - Value: instance.Name, - }, - { - Name: "STORAGE_CLUSTER_UID", - Value: string(instance.UID), + Name: util.PodNamespaceEnvVar, + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.namespace", + }, + }, }, }, Ports: []corev1.ContainerPort{ diff --git a/controllers/storagecluster/storagecluster_controller_test.go b/controllers/storagecluster/storagecluster_controller_test.go index b9c5832a11..d05307dda8 100644 --- a/controllers/storagecluster/storagecluster_controller_test.go +++ b/controllers/storagecluster/storagecluster_controller_test.go @@ -33,6 +33,7 @@ import ( appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" storagev1 "k8s.io/api/storage/v1" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -40,6 +41,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client/fake" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -319,9 +321,18 @@ var mockNodeList = &corev1.NodeList{ ObjectMeta: metav1.ObjectMeta{ Name: "node1", Labels: map[string]string{ - hostnameLabel: "node1", - zoneTopologyLabel: "zone1", - defaults.NodeAffinityKey: "", + hostnameLabel: "node1", + zoneTopologyLabel: "zone1", + defaults.NodeAffinityKey: "", + "node-role.kubernetes.io/worker": "", + }, + }, + Status: corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "0:0:0:0", + }, }, }, }, @@ -1156,7 +1167,28 @@ func createFakeStorageClusterReconciler(t *testing.T, obj ...runtime.Object) Sto "fsid": []byte(cephFSID), }, } - obj = append(obj, cbp, cfs, rookCephMonSecret, csv) + + os.Setenv(providerAPIServerImage, "fake-image") + os.Setenv(onboardingValidationKeysGeneratorImage, "fake-image") + + ocsProviderServiceDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName, Namespace: namespace}, + } + + ocsProviderService := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{{Hostname: "fake"}}, + }, + }, + } + + ocsProviderServiceSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: ocsProviderServerName}, + } + + obj = append(obj, cbp, cfs, rookCephMonSecret, csv, ocsProviderService, ocsProviderServiceDeployment, ocsProviderServiceSecret) client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(obj...).WithStatusSubresource(sc).Build() clusters, err := statusutil.GetClusters(context.TODO(), client) @@ -1168,8 +1200,16 @@ func createFakeStorageClusterReconciler(t *testing.T, obj ...runtime.Object) Sto if err != nil { operatorNamespace = "openshift-storage" } + //Update the deployment replicas to 1 + ocsProviderServiceDeployment.Status.AvailableReplicas = 1 + err = client.Status().Update(context.TODO(), ocsProviderServiceDeployment) + assert.NoError(t, err) + + frecorder := record.NewFakeRecorder(1024) + reporter := statusutil.NewEventReporter(frecorder) return StorageClusterReconciler{ + recorder: reporter, Client: client, Scheme: scheme, OperatorCondition: newStubOperatorCondition(), @@ -1256,6 +1296,12 @@ func createFakeScheme(t *testing.T) *runtime.Scheme { if err != nil { assert.Fail(t, "unable to add opv1a1 to scheme") } + + err = rbacv1.AddToScheme(scheme) + if err != nil { + assert.Fail(t, "unable to add rbacv1 to scheme") + } + return scheme }