diff --git a/controllers/storageclassrequest/storageclassrequest_controller_test.go b/controllers/storageclassrequest/storageclassrequest_controller_test.go index 3612f6f16e..4d62095c09 100644 --- a/controllers/storageclassrequest/storageclassrequest_controller_test.go +++ b/controllers/storageclassrequest/storageclassrequest_controller_test.go @@ -16,6 +16,7 @@ package storageclassrequest import ( "context" "fmt" + "strings" "testing" v1 "github.com/red-hat-storage/ocs-operator/api/v4/v1" @@ -33,6 +34,9 @@ import ( ) const ( + pgAutoscaleMode = "pg_autoscale_mode" + pgNum = "pg_num" + pgpNum = "pgp_num" namespaceName = "test-ns" deviceClass = "ssd" storageProfileKind = "StorageProfile" @@ -49,6 +53,46 @@ var fakeStorageProfile = &v1.StorageProfile{ }, } +var validStorageProfile = &v1.StorageProfile{ + TypeMeta: metav1.TypeMeta{Kind: storageProfileKind}, + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + Namespace: namespaceName, + }, + Spec: v1.StorageProfileSpec{ + DeviceClass: deviceClass, + BlockPoolConfiguration: v1.BlockPoolConfigurationSpec{ + Parameters: map[string]string{ + pgAutoscaleMode: "on", + pgNum: "128", + pgpNum: "128", + }, + }, + }, + Status: v1.StorageProfileStatus{Phase: ""}, +} + +// A rejected StorageProfile is one that is invalid due to having a blank device class field and is set to +// Rejected in its phase. +var rejectedStorageProfile = &v1.StorageProfile{ + TypeMeta: metav1.TypeMeta{Kind: storageProfileKind}, + ObjectMeta: metav1.ObjectMeta{ + Name: "rejected", + Namespace: namespaceName, + }, + Spec: v1.StorageProfileSpec{ + DeviceClass: "", + BlockPoolConfiguration: v1.BlockPoolConfigurationSpec{ + Parameters: map[string]string{ + pgAutoscaleMode: "on", + pgNum: "128", + pgpNum: "128", + }, + }, + }, + Status: v1.StorageProfileStatus{Phase: ""}, +} + var fakeStorageCluster = &v1.StorageCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "test-storagecluster", @@ -225,6 +269,200 @@ func TestProfileReconcile(t *testing.T) { assert.NoError(t, err, caseLabel) } +func TestStorageProfileCephBlockPool(t *testing.T) { + var err error + var caseCounter int + + var primaryTestCases = []struct { + label string + expectedPoolName string + failureExpected bool + createObjects []runtime.Object + storageProfile *v1.StorageProfile + }{ + { + label: "valid profile", + expectedPoolName: "test-valid-blockpool", + failureExpected: false, + storageProfile: validStorageProfile, + createObjects: []runtime.Object{ + &rookCephv1.CephBlockPool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-valid-blockpool", + Namespace: namespaceName, + Labels: map[string]string{ + controllers.StorageConsumerNameLabel: fakeStorageConsumer.Name, + controllers.StorageProfileSpecLabel: validStorageProfile.GetSpecHash(), + }, + }, Spec: rookCephv1.NamedBlockPoolSpec{ + Name: "spec", + PoolSpec: rookCephv1.PoolSpec{ + FailureDomain: "zone", + DeviceClass: deviceClass, + Parameters: map[string]string{}, + }, + }, + }, + }, + }, + { + label: "rejected profile", + expectedPoolName: "test-rejected-blockpool", + failureExpected: true, + storageProfile: rejectedStorageProfile, + createObjects: []runtime.Object{ + &rookCephv1.CephBlockPool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rejected-blockpool", + Namespace: namespaceName, + Labels: map[string]string{ + controllers.StorageConsumerNameLabel: fakeStorageConsumer.Name, + controllers.StorageProfileSpecLabel: rejectedStorageProfile.GetSpecHash(), + }, + }, Spec: rookCephv1.NamedBlockPoolSpec{ + Name: "spec", + PoolSpec: rookCephv1.PoolSpec{ + FailureDomain: "zone", + DeviceClass: deviceClass, + Parameters: map[string]string{}, + }, + }, + }, + }, + }, + } + + for _, c := range primaryTestCases { + caseCounter++ + caseLabel := fmt.Sprintf("Case %d: %s", caseCounter, c.label) + fmt.Println(caseLabel) + + r := createFakeReconciler(t) + r.storageCluster.Spec.DefaultStorageProfile = c.storageProfile.Name + r.StorageClassRequest.Spec.Type = "blockpool" + + r.StorageClassRequest.Spec.StorageProfile = c.storageProfile.Name + + c.createObjects = append(c.createObjects, c.storageProfile) + c.createObjects = append(c.createObjects, fakeStorageConsumer) + + fakeClient := fake.NewClientBuilder().WithScheme(r.Scheme).WithRuntimeObjects(c.createObjects...) + r.Client = fakeClient.Build() + + _, err = r.reconcilePhases() + if c.failureExpected { + assert.Error(t, err, caseLabel) + continue + } + assert.NoError(t, err, caseLabel) + + assert.Equal(t, c.expectedPoolName, r.cephBlockPool.Name, caseLabel) + + if strings.Contains(c.expectedPoolName, "valid") { + expectedStorageProfileParameters := validStorageProfile.Spec.BlockPoolConfiguration.Parameters + actualBlockPoolParameters := r.cephBlockPool.Spec.Parameters + assert.Equal(t, expectedStorageProfileParameters, actualBlockPoolParameters, caseLabel) + assert.NotEqual(t, v1.StorageProfilePhaseRejected, c.storageProfile.Status.Phase) + } else { + actualBlockPoolParameters := r.cephBlockPool.Spec.Parameters + assert.Equal(t, v1.StorageProfilePhaseRejected, c.storageProfile.Status.Phase) + assert.Nil(t, actualBlockPoolParameters, caseLabel) + } + } + +} + +func TestStorageProfileCephFsSubVolGroup(t *testing.T) { + var err error + var caseCounter int + + var primaryTestCases = []struct { + label string + expectedGroupName string + failureExpected bool + createObjects []runtime.Object + cephResources []*v1alpha1.CephResourcesSpec + storageProfile *v1.StorageProfile + cephFs *rookCephv1.CephFilesystem + }{ + { + label: "valid profile", + expectedGroupName: "test-subvolgroup", + storageProfile: fakeStorageProfile, + cephFs: fakeCephFs, + failureExpected: false, + cephResources: []*v1alpha1.CephResourcesSpec{ + { + Name: "test-subvolgroup", + Kind: "CephFilesystemSubVolumeGroup", + }, + }, + createObjects: []runtime.Object{ + &rookCephv1.CephFilesystemSubVolumeGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-subvolgroup", + Namespace: namespaceName, + }, + Status: &rookCephv1.CephFilesystemSubVolumeGroupStatus{}, + }, + }, + }, + { + label: "rejected profile", + expectedGroupName: "test-subvolgroup", + storageProfile: rejectedStorageProfile, + cephFs: fakeCephFs, + failureExpected: true, + cephResources: []*v1alpha1.CephResourcesSpec{ + { + Name: "test-subvolgroup", + Kind: "CephFilesystemSubVolumeGroup", + }, + }, + createObjects: []runtime.Object{ + &rookCephv1.CephFilesystemSubVolumeGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-subvolgroup", + Namespace: namespaceName, + }, + Status: &rookCephv1.CephFilesystemSubVolumeGroupStatus{}, + }, + }, + }, + } + + for _, c := range primaryTestCases { + caseCounter++ + caseLabel := fmt.Sprintf("Case %d: %s", caseCounter, c.label) + fmt.Println(caseLabel) + + r := createFakeReconciler(t) + if strings.Contains(c.label, "rejected") { + r.storageCluster.Spec.DefaultStorageProfile = rejectedStorageProfile.Name + } + + r.StorageClassRequest.Status.CephResources = c.cephResources + r.StorageClassRequest.Spec.Type = "sharedfilesystem" + r.StorageClassRequest.Spec.StorageProfile = c.storageProfile.Name + + c.createObjects = append(c.createObjects, c.cephFs) + c.createObjects = append(c.createObjects, c.storageProfile) + c.createObjects = append(c.createObjects, fakeStorageConsumer) + fakeClient := fake.NewClientBuilder().WithScheme(r.Scheme).WithRuntimeObjects(c.createObjects...) + + r.Client = fakeClient.Build() + r.StorageClassRequest.Status.CephResources = c.cephResources + + _, err = r.reconcilePhases() + if c.failureExpected { + assert.Error(t, err, caseLabel) + continue + } + assert.NoError(t, err, caseLabel) + assert.Equal(t, c.expectedGroupName, r.cephFilesystemSubVolumeGroup.Name, caseLabel) + } +} + func TestCephBlockPool(t *testing.T) { var err error var caseCounter int diff --git a/controllers/storagecluster/cephfilesystem_test.go b/controllers/storagecluster/cephfilesystem_test.go index ba6fc1881f..bfbb0611ee 100644 --- a/controllers/storagecluster/cephfilesystem_test.go +++ b/controllers/storagecluster/cephfilesystem_test.go @@ -2,6 +2,7 @@ package storagecluster import ( "context" + "strings" "testing" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" @@ -17,23 +18,95 @@ import ( func TestCephFileSystem(t *testing.T) { var cases = []struct { - label string - createRuntimeObjects bool + label string + createRuntimeObjects bool + remoteStorageConsumers bool }{ { - label: "case 1", - createRuntimeObjects: false, + label: "Not in provider mode", + createRuntimeObjects: false, + remoteStorageConsumers: false, + }, + { + label: "In provider mode", + createRuntimeObjects: false, + remoteStorageConsumers: true, }, } + spList := getMockStorageProfiles() + for _, c := range cases { var objects []client.Object - t, reconciler, cr, request := initStorageClusterResourceCreateUpdateTest(t, objects, nil) + + providerModeSpec := &api.StorageClusterSpec{ + AllowRemoteStorageConsumers: c.remoteStorageConsumers, + ProviderAPIServerServiceType: "", + } + + t, reconcilerOCSInit, cr, requestOCSInit, requestsStorageProfiles := initStorageClusterResourceCreateUpdateTestProviderMode( + t, objects, providerModeSpec, spList, c.remoteStorageConsumers) if c.createRuntimeObjects { objects = createUpdateRuntimeObjects(t) //nolint:staticcheck //no need to use objects as they update in runtime } - assertCephFileSystem(t, reconciler, cr, request) + if c.remoteStorageConsumers { + assertCephFileSystemProviderMode(t, reconcilerOCSInit, cr, requestOCSInit, requestsStorageProfiles) + } else { + assertCephFileSystem(t, reconcilerOCSInit, cr, requestOCSInit) + } + + } +} + +func assertCephFileSystemProviderMode(t *testing.T, reconciler StorageClusterReconciler, cr *api.StorageCluster, requestOCSInit reconcile.Request, requestsStorageProfiles []reconcile.Request) { + actualFs := &cephv1.CephFilesystem{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ocsinit-cephfilesystem", + }, + Spec: cephv1.FilesystemSpec{ + DataPools: []cephv1.NamedPoolSpec{ + {Name: "fast", PoolSpec: cephv1.PoolSpec{DeviceClass: "fast"}}, + {Name: "med", PoolSpec: cephv1.PoolSpec{DeviceClass: "med"}}, + {Name: "slow", PoolSpec: cephv1.PoolSpec{DeviceClass: "slow"}}, + }, + }, } + requestOCSInit.Name = "ocsinit-cephfilesystem" + err := reconciler.Client.Get(context.TODO(), requestOCSInit.NamespacedName, actualFs) + assert.NoError(t, err) + storageProfiles := &api.StorageProfileList{} + err = reconciler.Client.List(context.TODO(), storageProfiles) + assert.NoError(t, err) + assert.Equal(t, len(storageProfiles.Items), len(requestsStorageProfiles)) + assert.Equal(t, len(storageProfiles.Items)-1, len(actualFs.Spec.DataPools)) + + expectedCephFS, err := reconciler.newCephFilesystemInstances(cr) + assert.NoError(t, err) + + assert.Equal(t, len(expectedCephFS[0].OwnerReferences), 1) + + assert.Equal(t, expectedCephFS[0].ObjectMeta.Name, actualFs.ObjectMeta.Name) + assert.Equal(t, expectedCephFS[0].Spec, actualFs.Spec) + assert.Equal(t, expectedCephFS[0].Spec.DataPools[0].Name, actualFs.Spec.DataPools[0].Name) + assert.Equal(t, expectedCephFS[0].Spec.DataPools[1].Name, actualFs.Spec.DataPools[1].Name) + assert.Equal(t, expectedCephFS[0].Spec.DataPools[2].Name, actualFs.Spec.DataPools[2].Name) + assert.Equal(t, expectedCephFS[0].Spec.DataPools[0].PoolSpec.DeviceClass, actualFs.Spec.DataPools[0].PoolSpec.DeviceClass) + assert.Equal(t, expectedCephFS[0].Spec.DataPools[1].PoolSpec.DeviceClass, actualFs.Spec.DataPools[1].PoolSpec.DeviceClass) + assert.Equal(t, expectedCephFS[0].Spec.DataPools[2].PoolSpec.DeviceClass, actualFs.Spec.DataPools[2].PoolSpec.DeviceClass) + + for i := range requestsStorageProfiles { + actualStorageProfile := &api.StorageProfile{} + requestStorageProfile := requestsStorageProfiles[i] + err = reconciler.Client.Get(context.TODO(), requestStorageProfile.NamespacedName, actualStorageProfile) + assert.NoError(t, err) + assert.Equal(t, requestStorageProfile.Name, actualStorageProfile.Name) + + phaseStorageProfile := api.StorageProfilePhase("") + if strings.Contains(requestStorageProfile.Name, "blank") { + phaseStorageProfile = api.StorageProfilePhaseRejected + } + assert.Equal(t, phaseStorageProfile, actualStorageProfile.Status.Phase) + } } func assertCephFileSystem(t *testing.T, reconciler StorageClusterReconciler, cr *api.StorageCluster, request reconcile.Request) { diff --git a/controllers/storagecluster/initialization_reconciler_test.go b/controllers/storagecluster/initialization_reconciler_test.go index cbda2323d8..5a5c18dcda 100644 --- a/controllers/storagecluster/initialization_reconciler_test.go +++ b/controllers/storagecluster/initialization_reconciler_test.go @@ -143,6 +143,120 @@ func createUpdateRuntimeObjects(t *testing.T) []client.Object { return updateRTObjects } +func initStorageClusterResourceCreateUpdateTestProviderMode(t *testing.T, runtimeObjs []client.Object, + customSpec *api.StorageClusterSpec, storageProfiles *api.StorageProfileList, remoteConsumers bool) (*testing.T, StorageClusterReconciler, + *api.StorageCluster, reconcile.Request, []reconcile.Request) { + cr := createDefaultStorageCluster() + if customSpec != nil { + _ = mergo.Merge(&cr.Spec, customSpec) + } + requestOCSInit := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "ocsinit", + Namespace: "", + }, + } + requests := []reconcile.Request{requestOCSInit} + + rtObjsToCreateReconciler := []runtime.Object{&nbv1.NooBaa{}} + // runtimeObjs are present, it means tests are for update + // add all the update required changes + if runtimeObjs != nil { + tbd := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rook-ceph-tools", + }, + } + rtObjsToCreateReconciler = append(rtObjsToCreateReconciler, tbd) + } + + 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(onboardingSecretGeneratorImage, "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}, + } + + addedRuntimeObjects := []runtime.Object{node, service, deployment, secret} + rtObjsToCreateReconciler = append(rtObjsToCreateReconciler, addedRuntimeObjects...) + + } + + // Unpacks StorageProfile list to runtime objects array + var storageProfileRuntimeObjects []runtime.Object + for _, sp := range storageProfiles.Items { + storageProfileRuntimeObjects = append(storageProfileRuntimeObjects, &sp) + } + rtObjsToCreateReconciler = append(rtObjsToCreateReconciler, storageProfileRuntimeObjects...) + + reconciler := createFakeInitializationStorageClusterReconciler(t, rtObjsToCreateReconciler...) + + _ = reconciler.Client.Create(context.TODO(), cr) + for _, rtObj := range runtimeObjs { + _ = reconciler.Client.Create(context.TODO(), rtObj) + } + + var requestsStorageProfiles []reconcile.Request + for _, sp := range storageProfiles.Items { + requestSP := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: sp.Name, + Namespace: cr.Namespace, + }, + } + _ = reconciler.Client.Create(context.TODO(), &sp) + requestsStorageProfiles = append(requestsStorageProfiles, requestSP) + requests = append(requests, requestSP) + + } + + spList := &api.StorageProfileList{} + _ = reconciler.Client.List(context.TODO(), spList) + for _, request := range requests { + err := os.Setenv("OPERATOR_NAMESPACE", request.Namespace) + assert.NoError(t, err) + result, err := reconciler.Reconcile(context.TODO(), request) + assert.NoError(t, err) + assert.Equal(t, reconcile.Result{}, result) + err = os.Setenv("WATCH_NAMESPACE", request.Namespace) + assert.NoError(t, err) + } + + return t, reconciler, cr, requestOCSInit, requestsStorageProfiles +} + func initStorageClusterResourceCreateUpdateTest(t *testing.T, runtimeObjs []client.Object, customSpec *api.StorageClusterSpec) (*testing.T, StorageClusterReconciler, *api.StorageCluster, reconcile.Request) { @@ -233,8 +347,21 @@ func createFakeInitializationStorageClusterReconciler(t *testing.T, obj ...runti }, } - obj = append(obj, mockNodeList.DeepCopy(), cbp, cfs, cnfs, cnfsbp, cnfssvc, infrastructure, networkConfig) - client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(obj...).WithStatusSubresource(sc).Build() + statusSubresourceObjs := []client.Object{sc} + var runtimeObjects []runtime.Object + + for _, objIt := range obj { + if objIt != nil { + if objIt.GetObjectKind().GroupVersionKind().Kind == "StorageProfile" { + statusSubresourceObjs = append(statusSubresourceObjs, objIt.(client.Object)) + } else { + runtimeObjects = append(runtimeObjects, objIt) + } + } + } + + runtimeObjects = append(runtimeObjects, mockNodeList.DeepCopy(), cbp, cfs, cnfs, cnfsbp, cnfssvc, infrastructure, networkConfig) + client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(runtimeObjects...).WithStatusSubresource(statusSubresourceObjs...).Build() return StorageClusterReconciler{ Client: client, diff --git a/controllers/storagecluster/storagecluster_controller_test.go b/controllers/storagecluster/storagecluster_controller_test.go index 289b813b03..2748e7b5ab 100644 --- a/controllers/storagecluster/storagecluster_controller_test.go +++ b/controllers/storagecluster/storagecluster_controller_test.go @@ -193,6 +193,111 @@ var mockDeviceSets = []api.StorageDeviceSet{ }, } +func getMockStorageProfiles() *api.StorageProfileList { + const pgAutoscaleMode = "pg_autoscale_mode" + const pgNum = "pg_num" + const pgpNum = "pgp_num" + const namespace = "" + const kind = "StorageProfile" + const apiVersion = "ocs.openshift.io/v1" + spfast := &api.StorageProfile{ + TypeMeta: metav1.TypeMeta{ + Kind: kind, + APIVersion: apiVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "fast-performance", + Namespace: namespace, + }, + Spec: api.StorageProfileSpec{ + DeviceClass: "fast", + BlockPoolConfiguration: api.BlockPoolConfigurationSpec{ + Parameters: map[string]string{ + pgAutoscaleMode: "on", + pgNum: "128", + pgpNum: "128", + }, + }, + SharedFilesystemConfiguration: api.SharedFilesystemConfigurationSpec{ + Parameters: map[string]string{ + pgAutoscaleMode: "on", + pgNum: "128", + pgpNum: "128", + }, + }, + }, + } + spmed := &api.StorageProfile{ + TypeMeta: metav1.TypeMeta{ + Kind: kind, + APIVersion: apiVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "med-performance", + Namespace: namespace, + }, + Spec: api.StorageProfileSpec{ + DeviceClass: "med", + BlockPoolConfiguration: api.BlockPoolConfigurationSpec{ + Parameters: map[string]string{ + pgAutoscaleMode: "on", + pgNum: "128", + pgpNum: "128", + }, + }, + SharedFilesystemConfiguration: api.SharedFilesystemConfigurationSpec{ + Parameters: map[string]string{ + pgAutoscaleMode: "on", + pgNum: "128", + pgpNum: "128", + }, + }, + }, + } + spslow := &api.StorageProfile{ + TypeMeta: metav1.TypeMeta{ + Kind: kind, + APIVersion: apiVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "slow-performance", + Namespace: namespace, + }, + Spec: api.StorageProfileSpec{ + DeviceClass: "slow", + BlockPoolConfiguration: api.BlockPoolConfigurationSpec{ + Parameters: map[string]string{ + pgAutoscaleMode: "on", + pgNum: "128", + pgpNum: "128", + }, + }, + SharedFilesystemConfiguration: api.SharedFilesystemConfigurationSpec{ + Parameters: map[string]string{ + pgAutoscaleMode: "on", + pgNum: "128", + pgpNum: "128", + }, + }, + }, + } + spblankdeviceclass := &api.StorageProfile{ + TypeMeta: metav1.TypeMeta{ + Kind: kind, + APIVersion: apiVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "blank-performance", + Namespace: namespace, + }, + Spec: api.StorageProfileSpec{ + DeviceClass: "", + }, + } + storageProfiles := &api.StorageProfileList{Items: []api.StorageProfile{*spmed, *spslow, *spfast, *spblankdeviceclass}} + return storageProfiles +} + var mockNodeList = &corev1.NodeList{ TypeMeta: metav1.TypeMeta{ Kind: "NodeList",