Merge pull request #842 from priyanka19-98/refactor-code

Use resource infrastructure instead of Nodes

cmd/manager/main.go

@@ -9,13 +9,11 @@ import (
 	"runtime"
 	"time"
 
-	"github.com/go-logr/zapr"
-	"go.uber.org/zap"
-	"go.uber.org/zap/zapcore"
-
 	monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
+	"github.com/go-logr/zapr"
 	snapapi "github.com/kubernetes-csi/external-snapshotter/v2/pkg/apis/volumesnapshot/v1beta1"
 	nbapis "github.com/noobaa/noobaa-operator/v2/pkg/apis"
+	openshiftConfigv1 "github.com/openshift/api/config/v1"
 	openshiftv1 "github.com/openshift/api/template/v1"
 	"github.com/openshift/ocs-operator/pkg/apis"
 	ocsv1 "github.com/openshift/ocs-operator/pkg/apis/ocs/v1"
@@ -26,6 +24,8 @@ import (
 	"github.com/operator-framework/operator-sdk/pkg/ready"
 	sdkVersion "github.com/operator-framework/operator-sdk/version"
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 	corev1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -140,6 +140,11 @@ func main() {
 		os.Exit(1)
 	}
 
+	if err := openshiftConfigv1.AddToScheme(mgrScheme); err != nil {
+		log.Error(err, "Failed adding openshift/api/config/v1 to scheme")
+		os.Exit(1)
+	}
+
 	// Setup all Controllers
 	if err := controller.AddToManager(mgr); err != nil {
 		log.Error(err, "")

deploy/csv-templates/ocs-operator.csv.yaml.in

@@ -199,6 +199,14 @@ spec:
           - templates
           verbs:
           - '*'
+        - apiGroups:
+          - config.openshift.io
+          resources:
+          - infrastructures
+          verbs:
+          - get
+          - list
+          - watch
         serviceAccountName: ocs-operator
       deployments:
       - name: ocs-operator

deploy/olm-catalog/ocs-operator/manifests/ocs-operator.clusterserviceversion.yaml

@@ -1426,6 +1426,14 @@ spec:
           - templates
           verbs:
           - '*'
+        - apiGroups:
+          - config.openshift.io
+          resources:
+          - infrastructures
+          verbs:
+          - get
+          - list
+          - watch
         serviceAccountName: ocs-operator
       - rules:
         - apiGroups:

deploy/role.yaml

@@ -120,3 +120,11 @@ rules:
   - templates
   verbs:
   - '*'
+- apiGroups:
+  - config.openshift.io
+  resources:
+  - infrastructures
+  verbs:
+  - get
+  - list
+  - watch  

pkg/controller/storagecluster/cephblockpools_test.go

@@ -24,7 +24,7 @@ func TestCephBlockPools(t *testing.T) {
 		},
 	}
 	for _, eachPlatform := range allPlatforms {
-		cp := &CloudPlatform{platform: eachPlatform}
+		cp := &Platform{platform: eachPlatform}
 		for _, c := range cases {
 			var objects []runtime.Object
 			if c.createRuntimeObjects {

pkg/controller/storagecluster/cephfilesystem_test.go

@@ -23,7 +23,7 @@ func TestCephFileSystem(t *testing.T) {
 		},
 	}
 	for _, eachPlatform := range allPlatforms {
-		cp := &CloudPlatform{platform: eachPlatform}
+		cp := &Platform{platform: eachPlatform}
 		for _, c := range cases {
 			var objects []runtime.Object
 			if c.createRuntimeObjects {

pkg/controller/storagecluster/cephobjectstores.go

@@ -25,7 +25,7 @@ func (r *ReconcileStorageCluster) ensureCephObjectStores(instance *ocsv1.Storage
 	if err != nil {
 		return err
 	}
-	if isValidCloudPlatform(platform) {
+	if avoidObjectStore(platform) {
 		reqLogger.Info(fmt.Sprintf("not creating a CephObjectStore because the platform is '%s'", platform))
 		return nil
 	}
@@ -94,7 +94,7 @@ func (r *ReconcileStorageCluster) newCephObjectStoreInstances(initData *ocsv1.St
 				DataPool: cephv1.PoolSpec{
 					FailureDomain: initData.Status.FailureDomain,
 					Replicated: cephv1.ReplicatedSpec{
-						Size: 3,
+						Size:            3,
 						TargetSizeRatio: .49,
 					},
 				},

pkg/controller/storagecluster/cephobjectstores_test.go

@@ -23,7 +23,7 @@ func TestCephObjectStores(t *testing.T) {
 		},
 	}
 	for _, eachPlatform := range allPlatforms {
-		cp := &CloudPlatform{platform: eachPlatform}
+		cp := &Platform{platform: eachPlatform}
 		for _, c := range cases {
 			var objects []runtime.Object
 			if c.createRuntimeObjects {
@@ -48,7 +48,7 @@ func assertCephObjectStores(t *testing.T, reconciler ReconcileStorageCluster, cr
 	err = reconciler.client.Get(nil, request.NamespacedName, actualCos)
 	// for any cloud platform, 'cephobjectstore' should not be created
 	// 'Get' should have thrown an error
-	if isValidCloudPlatform(reconciler.platform.platform) {
+	if avoidObjectStore(reconciler.platform.platform) {
 		assert.Error(t, err)
 	} else {
 		assert.NoError(t, err)

pkg/controller/storagecluster/cephobjectstoreusers.go

@@ -48,7 +48,7 @@ func (r *ReconcileStorageCluster) ensureCephObjectStoreUsers(instance *ocsv1.Sto
 	if err != nil {
 		return err
 	}
-	if isValidCloudPlatform(platform) {
+	if avoidObjectStore(platform) {
 		return nil
 	}
 

pkg/controller/storagecluster/cephobjectstoreusers_test.go

@@ -22,7 +22,7 @@ func TestCephObjectStoreUsers(t *testing.T) {
 		},
 	}
 	for _, eachPlatform := range allPlatforms {
-		cp := &CloudPlatform{platform: eachPlatform}
+		cp := &Platform{platform: eachPlatform}
 		for _, c := range cases {
 			var objects []runtime.Object
 			if c.createRuntimeObjects {
@@ -47,7 +47,7 @@ func assertCephObjectStoreUsers(t *testing.T, reconciler ReconcileStorageCluster
 	}
 	request.Name = "ocsinit-cephobjectstoreuser"
 	err = reconciler.client.Get(nil, request.NamespacedName, actualCosu)
-	if isValidCloudPlatform(reconciler.platform.platform) {
+	if avoidObjectStore(reconciler.platform.platform) {
 		assert.Error(t, err)
 	} else {
 		assert.NoError(t, err)

pkg/controller/storagecluster/initialization_reconciler_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	nbv1 "github.com/noobaa/noobaa-operator/v2/pkg/apis/noobaa/v1alpha1"
+	configv1 "github.com/openshift/api/config/v1"
 	appsv1 "k8s.io/api/apps/v1"
 
 	monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
@@ -47,7 +48,7 @@ func createStorageCluster(scName, failureDomainName string,
 	return cr
 }
 
-func createUpdateRuntimeObjects(cp *CloudPlatform) []runtime.Object {
+func createUpdateRuntimeObjects(cp *Platform) []runtime.Object {
 	csfs := &storagev1.StorageClass{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "ocsinit-cephfs",
@@ -70,7 +71,7 @@ func createUpdateRuntimeObjects(cp *CloudPlatform) []runtime.Object {
 	}
 	updateRTObjects := []runtime.Object{csfs, csrbd, cfs, cbp}
 
-	if !isValidCloudPlatform(cp.platform) {
+	if !avoidObjectStore(cp.platform) {
 		csobc := &storagev1.StorageClass{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "ocsinit-ceph-rgw",
@@ -80,7 +81,7 @@ func createUpdateRuntimeObjects(cp *CloudPlatform) []runtime.Object {
 	}
 
 	// Create 'cephobjectstoreuser' only for non-aws platforms
-	if !isValidCloudPlatform(cp.platform) {
+	if !avoidObjectStore(cp.platform) {
 		cosu := &cephv1.CephObjectStoreUser{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "ocsinit-cephobjectstoreuser",
@@ -90,7 +91,7 @@ func createUpdateRuntimeObjects(cp *CloudPlatform) []runtime.Object {
 	}
 
 	// Create 'cephobjectstore' only for non-cloud platforms
-	if !isValidCloudPlatform(cp.platform) {
+	if !avoidObjectStore(cp.platform) {
 		cos := &cephv1.CephObjectStore{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "ocsinit-cephobjectstore",
@@ -103,7 +104,7 @@ func createUpdateRuntimeObjects(cp *CloudPlatform) []runtime.Object {
 }
 
 func initStorageClusterResourceCreateUpdateTestWithPlatform(
-	t *testing.T, platform *CloudPlatform, runtimeObjs []runtime.Object) (*testing.T, ReconcileStorageCluster, *api.StorageCluster, reconcile.Request) {
+	t *testing.T, platform *Platform, runtimeObjs []runtime.Object) (*testing.T, ReconcileStorageCluster, *api.StorageCluster, reconcile.Request) {
 	cr := createDefaultStorageCluster()
 	request := reconcile.Request{
 		NamespacedName: types.NamespacedName{
@@ -141,17 +142,17 @@ func initStorageClusterResourceCreateUpdateTestWithPlatform(
 
 func createFakeInitializationStorageClusterReconciler(t *testing.T, obj ...runtime.Object) ReconcileStorageCluster {
 	return createFakeInitializationStorageClusterReconcilerWithPlatform(
-		t, &CloudPlatform{platform: PlatformUnknown}, obj...)
+		t, &Platform{platform: configv1.NonePlatformType}, obj...)
 }
 
 func createFakeInitializationStorageClusterReconcilerWithPlatform(t *testing.T,
-	platform *CloudPlatform,
+	platform *Platform,
 	obj ...runtime.Object) ReconcileStorageCluster {
 	scheme := createFakeInitializationScheme(t, obj...)
 	obj = append(obj, mockNodeList)
 	client := fake.NewFakeClientWithScheme(scheme, obj...)
 	if platform == nil {
-		platform = &CloudPlatform{platform: PlatformUnknown}
+		platform = &Platform{platform: configv1.NonePlatformType}
 	}
 
 	return ReconcileStorageCluster{

pkg/controller/storagecluster/noobaa_system_reconciler_test.go

@@ -386,6 +386,6 @@ func getReconciler(t *testing.T, objs ...runtime.Object) ReconcileStorageCluster
 	return ReconcileStorageCluster{
 		scheme:   scheme,
 		client:   client,
-		platform: &CloudPlatform{},
+		platform: &Platform{},
 	}
 }

pkg/controller/storagecluster/platform_detection.go

@@ -3,38 +3,29 @@ package storagecluster
 import (
 	"context"
 	"fmt"
-	"strings"
 	"sync"
 
-	corev1 "k8s.io/api/core/v1"
+	configv1 "github.com/openshift/api/config/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-// CloudPlatformType is a string representing cloud platform type. Eg: aws, unknown
-type CloudPlatformType string
-
-const (
-	// PlatformAWS represents the Amazon Web Services platform
-	PlatformAWS CloudPlatformType = "aws"
-	// PlatformGCP represents the Google cloud Platform
-	PlatformGCP CloudPlatformType = "gce"
-	// PlatformAzure represents the Azure Platform
-	PlatformAzure CloudPlatformType = "azure"
-	// PlatformUnknown represents an unknown validly formatted cloud platform
-	PlatformUnknown CloudPlatformType = "unknown"
-)
-
-// ValidCloudPlatforms is a list of all CloudPlatformTypes recognized by the package other than PlatformUnknown
-var ValidCloudPlatforms = []CloudPlatformType{PlatformAWS, PlatformGCP, PlatformAzure}
+// AvoidObjectStorePlatforms is a list of all PlatformTypes where CephObjectStores will not be deployed.
+var AvoidObjectStorePlatforms = []configv1.PlatformType{
+	configv1.AWSPlatformType,
+	configv1.GCPPlatformType,
+	configv1.AzurePlatformType,
+}
 
-// CloudPlatform is used to get the CloudPlatformType of the running cluster in a thread-safe manner.
-type CloudPlatform struct {
-	platform CloudPlatformType
+// Platform is used to get the CloudPlatformType of the running cluster in a thread-safe manner
+type Platform struct {
+	platform configv1.PlatformType
 	mux      sync.Mutex
 }
 
 // GetPlatform is used to get the CloudPlatformType of the running cluster
-func (p *CloudPlatform) GetPlatform(c client.Client) (CloudPlatformType, error) {
+func (p *Platform) GetPlatform(c client.Client) (configv1.PlatformType, error) {
 	// if 'platform' is already set just return it
 	if p.platform != "" {
 		return p.platform, nil
@@ -45,29 +36,20 @@ func (p *CloudPlatform) GetPlatform(c client.Client) (CloudPlatformType, error)
 	return p.getPlatform(c)
 }
 
-func (p *CloudPlatform) getPlatform(c client.Client) (CloudPlatformType, error) {
-	nodeList := &corev1.NodeList{}
-	err := c.List(context.TODO(), nodeList)
+func (p *Platform) getPlatform(c client.Client) (configv1.PlatformType, error) {
+	infrastructure := &configv1.Infrastructure{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}}
+	err := c.Get(context.TODO(), types.NamespacedName{Name: infrastructure.ObjectMeta.Name}, infrastructure)
 	if err != nil {
-		return "", fmt.Errorf("could not get storage nodes to determine cloud platform: %v", err)
+		return "", fmt.Errorf("could not get infrastructure details to determine cloud platform: %v", err)
 	}
-	for _, node := range nodeList.Items {
-		providerID := node.Spec.ProviderID
-		for _, cp := range ValidCloudPlatforms {
-			prefix := fmt.Sprintf("%s://", cp)
-			if strings.HasPrefix(providerID, prefix) {
-				p.platform = cp
-				return p.platform, nil
-			}
-		}
-	}
-	p.platform = PlatformUnknown
+
+	p.platform = infrastructure.Status.Platform
 	return p.platform, nil
 }
 
-func isValidCloudPlatform(p CloudPlatformType) bool {
-	for _, cp := range ValidCloudPlatforms {
-		if p == cp {
+func avoidObjectStore(p configv1.PlatformType) bool {
+	for _, platform := range AvoidObjectStorePlatforms {
+		if p == platform {
 			return true
 		}
 	}

pkg/controller/storagecluster/storageclasses.go

@@ -146,7 +146,7 @@ func (r *ReconcileStorageCluster) newStorageClasses(initData *ocsv1.StorageClust
 	// OR
 	// b. current platform is not a cloud-based platform
 	platform, err := r.platform.GetPlatform(r.client)
-	if initData.Spec.ExternalStorage.Enable || err == nil && !isValidCloudPlatform(platform) {
+	if initData.Spec.ExternalStorage.Enable || err == nil && !avoidObjectStore(platform) {
 		ret = append(ret, r.newOBCStorageClass(initData))
 	}
 	return ret, nil

pkg/controller/storagecluster/storageclasses_test.go

@@ -1,19 +1,21 @@
 package storagecluster
 
 import (
+	"testing"
+
+	configv1 "github.com/openshift/api/config/v1"
 	api "github.com/openshift/ocs-operator/pkg/apis/ocs/v1"
 	"github.com/stretchr/testify/assert"
 	storagev1 "k8s.io/api/storage/v1"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	"testing"
 )
 
-var allPlatforms = append(ValidCloudPlatforms,
-	PlatformUnknown, CloudPlatformType("NonCloudPlatform"))
+var allPlatforms = append(AvoidObjectStorePlatforms,
+	configv1.NonePlatformType, configv1.PlatformType("NonCloudPlatform"))
 
 func TestStorageClasses(t *testing.T) {
 	for _, eachPlatform := range allPlatforms {
-		cp := &CloudPlatform{platform: eachPlatform}
+		cp := &Platform{platform: eachPlatform}
 		t, reconciler, cr, request := initStorageClusterResourceCreateUpdateTestWithPlatform(
 			t, cp, nil)
 		assertStorageClasses(t, reconciler, cr, request)
@@ -40,7 +42,7 @@ func assertStorageClasses(t *testing.T, reconciler ReconcileStorageCluster, cr *
 	err = reconciler.client.Get(nil, request.NamespacedName, actualSc3)
 	// on a cloud platform, 'Get' should throw an error,
 	// as OBC StorageClass won't be created
-	if isValidCloudPlatform(reconciler.platform.platform) {
+	if avoidObjectStore(reconciler.platform.platform) {
 		// we should be expecting only 2 storage classes
 		assert.Equal(t, len(expected), 2)
 		assert.Error(t, err)

pkg/controller/storagecluster/storagecluster_controller.go

@@ -80,7 +80,7 @@ func newReconciler(mgr manager.Manager) reconcile.Reconciler {
 		client:    mgr.GetClient(),
 		scheme:    mgr.GetScheme(),
 		reqLogger: log,
-		platform:  &CloudPlatform{},
+		platform:  &Platform{},
 	}
 
 	err := r.initializeImageVars()
@@ -161,5 +161,5 @@ type ReconcileStorageCluster struct {
 	noobaaDBImage   string
 	noobaaCoreImage string
 	nodeCount       int
-	platform        *CloudPlatform
+	platform        *Platform
 }