Adds checks for StorageClient ref

- New utility functions for checking if MirrorPeer is having StorageClient ref - Functions cross check with the configmap to see if client - Functions will take a parameter for managedCluster and hub cluster separately to pick the configmap properly Signed-off-by: vbadrina <[email protected]>
red-hat-storage · Aug 21, 2024 · a196623 · a196623
1 parent 832ce81
commit a196623
Show file tree

Hide file tree

Showing 7 changed files with 133 additions and 28 deletions.
diff --git a/addons/agent_mirrorpeer_controller.go b/addons/agent_mirrorpeer_controller.go
@@ -115,14 +115,21 @@ func (r *MirrorPeerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, nil
 	}
 
+	hasStorageClientRef, err := utils.IsStorageClientType(ctx, r.SpokeClient, mirrorPeer, true)
+
+	if err != nil {
+		logger.Error("Failed to check if storage client ref exists", "error", err)
+		return ctrl.Result{}, err
+	}
+
 	logger.Info("Creating S3 buckets")
-	err = r.createS3(ctx, mirrorPeer, scr.Namespace)
+	err = r.createS3(ctx, mirrorPeer, scr.Namespace, hasStorageClientRef)
 	if err != nil {
 		logger.Error("Failed to create ODR S3 resources", "error", err)
 		return ctrl.Result{}, err
 	}
 
-	if mirrorPeer.Spec.Type == multiclusterv1alpha1.Async && !utils.IsStorageClientType(mirrorPeer.Spec.Items) {
+	if mirrorPeer.Spec.Type == multiclusterv1alpha1.Async && !hasStorageClientRef {
 		clusterFSIDs := make(map[string]string)
 		logger.Info("Fetching clusterFSIDs")
 		err = r.fetchClusterFSIDs(ctx, &mirrorPeer, clusterFSIDs)
@@ -156,7 +163,7 @@ func (r *MirrorPeerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 	}
 
-	if mirrorPeer.Spec.Type == multiclusterv1alpha1.Async {
+	if mirrorPeer.Spec.Type == multiclusterv1alpha1.Async && hasStorageClientRef {
 		if mirrorPeer.Status.Phase == multiclusterv1alpha1.ExchangedSecret {
 			logger.Info("Cleaning up stale onboarding token", "Token", string(mirrorPeer.GetUID()))
 			err = deleteStorageClusterPeerTokenSecret(ctx, r.HubClient, r.SpokeClusterName, string(mirrorPeer.GetUID()))
@@ -306,16 +313,15 @@ func (r *MirrorPeerReconciler) labelRBDStorageClasses(ctx context.Context, stora
 	return errs
 }
 
-func (r *MirrorPeerReconciler) createS3(ctx context.Context, mirrorPeer multiclusterv1alpha1.MirrorPeer, scNamespace string) error {
-	logger := r.Logger.With("MirrorPeer", mirrorPeer.Name)
+func (r *MirrorPeerReconciler) createS3(ctx context.Context, mirrorPeer multiclusterv1alpha1.MirrorPeer, scNamespace string, hasStorageClientRef bool) error {
 	bucketCount := 1
-	if utils.IsStorageClientType(mirrorPeer.Spec.Items) {
+	if hasStorageClientRef {
 		bucketCount = 2
 	}
 	for index := 0; index < bucketCount; index++ {
 		bucketNamespace := utils.GetEnv("ODR_NAMESPACE", scNamespace)
 		var bucketName string
-		if utils.IsStorageClientType(mirrorPeer.Spec.Items) {
+		if hasStorageClientRef {
 			bucketName = utils.GenerateBucketName(mirrorPeer, mirrorPeer.Spec.Items[index].StorageClusterRef.Name)
 		} else {
 			bucketName = utils.GenerateBucketName(mirrorPeer)
@@ -324,7 +330,7 @@ func (r *MirrorPeerReconciler) createS3(ctx context.Context, mirrorPeer multiclu
 		if err != nil {
 			return err
 		}
-		logger.Info(fmt.Sprintf("ObjectBucketClaim %s was %s in namespace %s", bucketName, operationResult, bucketNamespace))
+		r.Logger.Info(fmt.Sprintf("ObjectBucketClaim %s was %s in namespace %s", bucketName, operationResult, bucketNamespace))
 	}
 
 	return nil

diff --git a/api/v1alpha1/mirrorpeer_types.go b/api/v1alpha1/mirrorpeer_types.go
@@ -38,7 +38,7 @@ type StorageClusterRef struct {
 	Name string `json:"name"`
 
 	// +kubebuilder:validation:Optional
-	Namespace string `json:"namespace"`
+	Namespace string `json:"namespace,omitempty"`
 }
 
 // PeerRef holds a reference to a mirror peer

diff --git a/controllers/drpolicy_controller.go b/controllers/drpolicy_controller.go
@@ -113,7 +113,14 @@ func (r *DRPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		return ctrl.Result{}, err
 	}
 
-	if utils.IsStorageClientType(mirrorPeer.Spec.Items) {
+	// Check if the MirrorPeer contains StorageClient reference
+	hasStorageClientRef, err := utils.IsStorageClientType(ctx, r.HubClient, *mirrorPeer, false)
+	if err != nil {
+		logger.Error("Failed to determine if MirrorPeer contains StorageClient reference", "error", err)
+		return ctrl.Result{}, err
+	}
+
+	if hasStorageClientRef {
 		logger.Info("MirrorPeer contains StorageClient reference. Skipping creation of VolumeReplicationClasses", "MirrorPeer", mirrorPeer.Name)
 		return ctrl.Result{}, nil
 	}

diff --git a/controllers/managedclusterview_controller.go b/controllers/managedclusterview_controller.go
@@ -29,12 +29,6 @@ type ManagedClusterViewReconciler struct {
 	Logger *slog.Logger
 }
 
-const (
-	ODFInfoConfigMapName    = "odf-info"
-	ConfigMapResourceType   = "ConfigMap"
-	ClientInfoConfigMapName = "odf-client-info"
-)
-
 type ProviderInfo struct {
 	Version                    string               `json:"version"`
 	DeploymentType             string               `json:"deploymentType"`
@@ -77,7 +71,7 @@ func (r *ManagedClusterViewReconciler) SetupWithManager(mgr ctrl.Manager) error
 }
 
 func hasODFInfoInScope(mc *viewv1beta1.ManagedClusterView) bool {
-	if mc.Spec.Scope.Name == ODFInfoConfigMapName && mc.Spec.Scope.Kind == ConfigMapResourceType {
+	if mc.Spec.Scope.Name == utils.ODFInfoConfigMapName && mc.Spec.Scope.Kind == utils.ConfigMapResourceType {
 		return true
 	}
 	return false
@@ -151,11 +145,11 @@ func createOrUpdateConfigMap(ctx context.Context, c client.Client, managedCluste
 	operatorNamespace := os.Getenv("POD_NAMESPACE")
 	configMap := &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      ClientInfoConfigMapName,
+			Name:      utils.ClientInfoConfigMapName,
 			Namespace: operatorNamespace,
 		},
 	}
-	err = c.Get(ctx, types.NamespacedName{Name: ClientInfoConfigMapName, Namespace: operatorNamespace}, configMap)
+	err = c.Get(ctx, types.NamespacedName{Name: utils.ClientInfoConfigMapName, Namespace: operatorNamespace}, configMap)
 	if err != nil && !errors.IsNotFound(err) {
 		return fmt.Errorf("failed to get ConfigMap. %w", err)
 	}
@@ -193,7 +187,7 @@ func createOrUpdateConfigMap(ctx context.Context, c client.Client, managedCluste
 		return fmt.Errorf("failed to create or update ConfigMap. %w", err)
 	}
 
-	logger.Info(fmt.Sprintf("ConfigMap %s in namespace %s has been %s", ClientInfoConfigMapName, operatorNamespace, op))
+	logger.Info(fmt.Sprintf("ConfigMap %s in namespace %s has been %s", utils.ClientInfoConfigMapName, operatorNamespace, op))
 
 	return nil
 }
diff --git a/controllers/mirrorpeer_controller.go b/controllers/mirrorpeer_controller.go
@@ -59,7 +59,6 @@ type MirrorPeerReconciler struct {
 const (
 	mirrorPeerFinalizer         = "hub.multicluster.odf.openshift.io"
 	spokeClusterRoleBindingName = "spoke-clusterrole-bindings"
-	ClientConfigMapKeyTemplate  = "%s/%s"
 )
 
 //+kubebuilder:rbac:groups=multicluster.odf.openshift.io,resources=mirrorpeers,verbs=get;list;watch;create;update;patch;delete
@@ -247,7 +246,14 @@ func (r *MirrorPeerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, err
 	}
 
-	if utils.IsStorageClientType(mirrorPeer.Spec.Items) {
+	// Check if the MirrorPeer contains StorageClient reference
+	hasStorageClientRef, err := utils.IsStorageClientType(ctx, r.Client, mirrorPeer, false)
+	if err != nil {
+		logger.Error("Failed to determine if MirrorPeer contains StorageClient reference", "error", err)
+		return ctrl.Result{}, err
+	}
+
+	if hasStorageClientRef {
 		result, err := createStorageClusterPeer(ctx, r.Client, logger, mirrorPeer)
 		if err != nil {
 			logger.Error("Failed to create StorageClusterPeer", "error", err)
@@ -258,7 +264,7 @@ func (r *MirrorPeerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 }
 
 func getKey(clusterName, clientName string) string {
-	return fmt.Sprintf(ClientConfigMapKeyTemplate, clusterName, clientName)
+	return fmt.Sprintf("%s/%s", clusterName, clientName)
 }
 
 func createStorageClusterPeer(ctx context.Context, client client.Client, logger *slog.Logger, mirrorPeer multiclusterv1alpha1.MirrorPeer) (ctrl.Result, error) {
@@ -298,7 +304,7 @@ func createStorageClusterPeer(ctx context.Context, client client.Client, logger
 		// Provider B's onboarding token will be used for Provider A's StorageClusterPeer
 		onboardingToken, err := fetchOnboardingTicket(ctx, client, oppositeClient, mirrorPeer)
 		if err != nil {
-			return ctrl.Result{}, fmt.Errorf("failed to fetch onboarding token for provider %s. err %w", oppositeClient.ProviderInfo.ProviderManagedClusterName, err)
+			return ctrl.Result{}, fmt.Errorf("failed to fetch onboarding token for provider %s. %w", oppositeClient.ProviderInfo.ProviderManagedClusterName, err)
 		}
 		storageClusterPeer := ocsv1.StorageClusterPeer{
 			ObjectMeta: metav1.ObjectMeta{
@@ -374,7 +380,7 @@ func fetchClientInfoConfigMap(ctx context.Context, c client.Client) (*corev1.Con
 	if currentNamespace == "" {
 		return nil, fmt.Errorf("cannot detect the current namespace")
 	}
-	clientInfoMap, err := utils.FetchConfigMap(ctx, c, ClientInfoConfigMapName, currentNamespace)
+	clientInfoMap, err := utils.FetchConfigMap(ctx, c, utils.ClientInfoConfigMapName, currentNamespace)
 	if err != nil {
 		return nil, err
 	}
@@ -409,7 +415,14 @@ func getClientInfoFromConfigMap(clientInfoMap map[string]string, key string) (Cl
 
 func getConfig(ctx context.Context, c client.Client, mp multiclusterv1alpha1.MirrorPeer) ([]ManagedClusterAddonConfig, error) {
 	managedClusterAddonsConfig := make([]ManagedClusterAddonConfig, 0)
-	if utils.IsStorageClientType(mp.Spec.Items) {
+
+	// Check if the MirrorPeer contains StorageClient reference
+	hasStorageClientRef, err := utils.IsStorageClientType(ctx, c, mp, false)
+	if err != nil {
+		return []ManagedClusterAddonConfig{}, err
+	}
+
+	if hasStorageClientRef {
 		clientInfoMap, err := fetchClientInfoConfigMap(ctx, c)
 		if err != nil {
 			return []ManagedClusterAddonConfig{}, err

diff --git a/controllers/utils/configmap.go b/controllers/utils/configmap.go
@@ -3,11 +3,19 @@ package utils
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const (
+	ODFInfoConfigMapName    = "odf-info"
+	ConfigMapResourceType   = "ConfigMap"
+	ClientInfoConfigMapName = "odf-client-info"
+)
+
 // FetchConfigMap fetches a ConfigMap with a given name from a given namespace
 func FetchConfigMap(ctx context.Context, c client.Client, name, namespace string) (*corev1.ConfigMap, error) {
 	configMap := &corev1.ConfigMap{}
@@ -20,3 +28,15 @@ func FetchConfigMap(ctx context.Context, c client.Client, name, namespace string
 	}
 	return configMap, nil
 }
+
+// GetODFInfoConfigMap fetches the odf-info ConfigMap from the given namespace. This will only work on the managed cluster
+func GetODFInfoConfigMap(ctx context.Context, c client.Client, namespace string) (*corev1.ConfigMap, error) {
+	return FetchConfigMap(ctx, c, ODFInfoConfigMapName, namespace)
+}
+
+func SplitKeyForNamespacedName(key string) types.NamespacedName {
+	// key = openshift-storage_ocs-storagecluster.config.yaml
+	splitKey := strings.Split(key, ".")               // [openshift-storage_ocs-storagecluster,config,yaml]
+	namespacedName := strings.Split(splitKey[0], "_") // [openshift-storage,ocs-storagecluster]
+	return types.NamespacedName{Namespace: namespacedName[0], Name: namespacedName[1]}
+}
diff --git a/controllers/utils/peer_ref.go b/controllers/utils/peer_ref.go
@@ -3,11 +3,27 @@ package utils
 import (
 	"context"
 	"fmt"
+	"os"
 
+	ocsv1alpha1 "github.com/red-hat-storage/ocs-operator/api/v4/v1alpha1"
 	multiclusterv1alpha1 "github.com/red-hat-storage/odf-multicluster-orchestrator/api/v1alpha1"
+	"gopkg.in/yaml.v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+type PeerRefType string
+
+const (
+	// PeerRefTypeStorageClient represents a storage client
+	PeerRefTypeStorageClient PeerRefType = "StorageClient"
+
+	// PeerRefTypeStorageCluster represents a storage cluster
+	PeerRefTypeStorageCluster PeerRefType = "StorageCluster"
+
+	// PeerRefTypeUnknown represents an unknown type
+	PeerRefTypeUnknown PeerRefType = "Unknown"
+)
+
 // DoesAnotherMirrorPeerPointToPeerRef checks if another mirrorpeer is pointing to the provided peer ref
 func DoesAnotherMirrorPeerPointToPeerRef(ctx context.Context, rc client.Client, peerRef *multiclusterv1alpha1.PeerRef) (bool, error) {
 	mirrorPeers, err := FetchAllMirrorPeers(ctx, rc)
@@ -34,6 +50,55 @@ func GetPeerRefForSpokeCluster(mp *multiclusterv1alpha1.MirrorPeer, spokeCluster
 	return nil, fmt.Errorf("PeerRef for cluster %s under mirrorpeer %s not found", spokeClusterName, mp.Name)
 }
 
-func IsStorageClientType(peerRefs []multiclusterv1alpha1.PeerRef) bool {
-	return peerRefs[0].StorageClusterRef.Namespace == "" && peerRefs[1].StorageClusterRef.Namespace == ""
+func getPeerRefType(ctx context.Context, c client.Client, peerRef multiclusterv1alpha1.PeerRef, isManagedCluster bool) (PeerRefType, error) {
+	if isManagedCluster {
+		cm, err := GetODFInfoConfigMap(ctx, c, peerRef.StorageClusterRef.Namespace)
+		if err != nil {
+			return PeerRefTypeUnknown, err
+		}
+		var odfInfo ocsv1alpha1.OdfInfoData
+		for key, value := range cm.Data {
+			namespacedName := SplitKeyForNamespacedName(key)
+			if namespacedName.Name == peerRef.StorageClusterRef.Name {
+				err := yaml.Unmarshal([]byte(value), &odfInfo)
+				if err != nil {
+					return PeerRefTypeUnknown, fmt.Errorf("failed to unmarshal ODF info data. %w", err)
+				}
+
+				for _, client := range odfInfo.Clients {
+					if client.Name == peerRef.ClusterName {
+						return PeerRefTypeStorageClient, nil
+					}
+				}
+			}
+		}
+		return PeerRefTypeStorageCluster, nil
+	} else {
+		operatorNamespace := os.Getenv("POD_NAMESPACE")
+		cm, err := FetchConfigMap(ctx, c, ClientInfoConfigMapName, operatorNamespace)
+		if err != nil {
+			return PeerRefTypeUnknown, err
+		}
+
+		if _, ok := cm.Data[peerRef.ClusterName]; ok {
+			return PeerRefTypeStorageClient, nil
+		}
+		return PeerRefTypeStorageCluster, nil
+	}
+}
+
+// IsStorageClientType checks if peerRefs on MirrorPeer is of type StorageClient or StorageCluster
+func IsStorageClientType(ctx context.Context, c client.Client, mirrorPeer multiclusterv1alpha1.MirrorPeer, isManagedCluster bool) (bool, error) {
+	areStorageClientType := true
+	for _, v := range mirrorPeer.Spec.Items {
+		peerRefType, err := getPeerRefType(ctx, c, v, isManagedCluster)
+		if err != nil {
+			return false, err
+		}
+		if peerRefType != PeerRefTypeStorageClient {
+			areStorageClientType = false
+			break
+		}
+	}
+	return areStorageClientType, nil
 }