Skip to content

Commit

Permalink
Adds checks for StorageClient ref
Browse files Browse the repository at this point in the history
- 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 <vbadrina@redhat.com>
  • Loading branch information
vbnrh committed Sep 10, 2024
1 parent 24a3637 commit ebdb0b3
Show file tree
Hide file tree
Showing 11 changed files with 198 additions and 24 deletions.
22 changes: 14 additions & 8 deletions addons/agent_mirrorpeer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
23 changes: 22 additions & 1 deletion addons/agent_mirrorpeer_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,27 @@ import (
)

var (
odfInfoConfigMap = corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "odf-info",
Namespace: "test-namespace", // Use a generic namespace
UID: types.UID("268e6cdb-54fc-4f10-afab-67b106880be3"),
},
Data: map[string]string{
"test-namespace_test-storagecluster.config.yaml": `
version: 4.17.0-95.stable
deploymentType: internal
clients: []
storageCluster:
namespacedName:
namespace: test-namespace
name: test-storagecluster
storageProviderEndpoint: ""
cephClusterFSID: 986532da-8dba-4d35-a8d2-12f037712b39
storageSystemName: ocs-storagecluster-storagesystem
`,
},
}
mpItems = []multiclusterv1alpha1.PeerRef{
{
ClusterName: "cluster1",
Expand Down Expand Up @@ -141,7 +162,7 @@ func TestMirrorPeerReconcile(t *testing.T) {
rcm.Data = make(map[string]string)
rcm.Data[RookCSIEnableKey] = "false"

fakeSpokeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&storageCluster, &rcm, &clusterPeerToken, &exchangedSecret1, &exchangedSecret2, rbdStorageClass, cephfsStorageClass).Build()
fakeSpokeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&storageCluster, &rcm, &clusterPeerToken, &exchangedSecret1, &exchangedSecret2, rbdStorageClass, cephfsStorageClass, &odfInfoConfigMap).Build()

r := MirrorPeerReconciler{
HubClient: fakeHubClient,
Expand Down
2 changes: 1 addition & 1 deletion api/v1alpha1/mirrorpeer_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion controllers/drpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
20 changes: 19 additions & 1 deletion controllers/drpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package controllers
import (
"context"
"fmt"
"os"
"testing"

ramenv1alpha1 "github.com/ramendr/ramen/api/v1alpha1"
multiclusterv1alpha1 "github.com/red-hat-storage/odf-multicluster-orchestrator/api/v1alpha1"
"github.com/red-hat-storage/odf-multicluster-orchestrator/controllers/utils"
viewv1beta1 "github.com/stolostron/multicloud-operators-foundation/pkg/apis/view/v1beta1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -100,6 +102,7 @@ func TestDRPolicyReconcile(t *testing.T) {

func getFakeDRPolicyReconciler(drpolicy *ramenv1alpha1.DRPolicy, mp *multiclusterv1alpha1.MirrorPeer) DRPolicyReconciler {
scheme := mgrScheme
os.Setenv("POD_NAMESPACE", "openshift-operators")
ns1 := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: cName1,
Expand Down Expand Up @@ -139,8 +142,23 @@ func getFakeDRPolicyReconciler(drpolicy *ramenv1alpha1.DRPolicy, mp *multicluste
},
Type: "multicluster.odf.openshift.io/secret-type",
}
odfClientInfoConfigMap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "odf-client-info",
Namespace: os.Getenv("POD_NAMESPACE"),
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: viewv1beta1.GroupVersion.String(),
Kind: "ManagedClusterView",
Name: "mcv-1",
UID: "mcv-uid",
},
},
},
Data: map[string]string{},
}

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(drpolicy, mp, ns1, ns2, &hubSecret1, &hubSecret2).Build()
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(drpolicy, mp, ns1, ns2, &hubSecret1, &hubSecret2, odfClientInfoConfigMap).Build()

r := DRPolicyReconciler{
HubClient: fakeClient,
Expand Down
2 changes: 1 addition & 1 deletion controllers/managedclusterview_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (r *ManagedClusterViewReconciler) SetupWithManager(mgr ctrl.Manager) error
}

func hasODFInfoInScope(mc *viewv1beta1.ManagedClusterView) bool {
if mc.Spec.Scope.Name == ODFInfoConfigMapName && mc.Spec.Scope.Resource == ConfigMapResourceType {
if mc.Spec.Scope.Name == utils.ODFInfoConfigMapName && mc.Spec.Scope.Resource == ConfigMapResourceType {
return true
}
return false
Expand Down
25 changes: 19 additions & 6 deletions controllers/mirrorpeer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
22 changes: 20 additions & 2 deletions controllers/mirrorpeer_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ package controllers

import (
"context"
"os"
"testing"

"github.com/red-hat-storage/odf-multicluster-orchestrator/addons/setup"
multiclusterv1alpha1 "github.com/red-hat-storage/odf-multicluster-orchestrator/api/v1alpha1"
"github.com/red-hat-storage/odf-multicluster-orchestrator/controllers/utils"
viewv1beta1 "github.com/stolostron/multicloud-operators-foundation/pkg/apis/view/v1beta1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -91,7 +93,7 @@ func getFakeMirrorPeerReconciler(mirrorpeer multiclusterv1alpha1.MirrorPeer) Mir
// Using a different scheme for test might cause issues like
// missing scheme in manager
scheme := mgrScheme

os.Setenv("POD_NAMESPACE", "openshift-operators")
managedcluster1 := clusterv1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster1",
Expand All @@ -106,7 +108,23 @@ func getFakeMirrorPeerReconciler(mirrorpeer multiclusterv1alpha1.MirrorPeer) Mir
Spec: clusterv1.ManagedClusterSpec{},
}

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&mirrorpeer, &managedcluster1, &managedcluster2).Build()
var odfClientInfoConfigMap = &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "odf-client-info",
Namespace: os.Getenv("POD_NAMESPACE"),
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: viewv1beta1.GroupVersion.String(),
Kind: "ManagedClusterView",
Name: "mcv-1",
UID: "mcv-uid",
},
},
},
Data: map[string]string{},
}

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&mirrorpeer, &managedcluster1, &managedcluster2, odfClientInfoConfigMap).Build()

r := MirrorPeerReconciler{
Client: fakeClient,
Expand Down
24 changes: 24 additions & 0 deletions controllers/utils/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,20 @@ package utils
import (
"context"
"fmt"
"strings"

corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"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{}
Expand All @@ -16,7 +25,22 @@ func FetchConfigMap(ctx context.Context, c client.Client, name, namespace string
Namespace: namespace,
}, configMap)
if err != nil {
if k8serrors.IsNotFound(err) {
return nil, err
}
return nil, fmt.Errorf("failed to fetch ConfigMap %s in namespace %s: %v", name, namespace, err)
}
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]}
}
Loading

0 comments on commit ebdb0b3

Please sign in to comment.