From 0d13067dc4b9f43e345b31d5395d14e0293e98f0 Mon Sep 17 00:00:00 2001 From: Wilson Radadia <159131702+WilsonRadadia20@users.noreply.github.com> Date: Mon, 14 Oct 2024 17:24:21 +0530 Subject: [PATCH] Added support for replication with minimal manifest (#732) * Add replication to minimal sample for PowerFlex and PowerScale * Minimal sample for replication in PowerScale * Fix for the yamlint issues * Fix for the go lint issue * yamllint fix * yamllint fix * yamllint fix --------- Co-authored-by: Surya --- .../replication/v1.8.1/controller.yaml | 2 +- .../replication/v1.9.0/controller.yaml | 2 +- pkg/modules/replication.go | 56 +++++++++++++------ pkg/utils/utils.go | 19 ++++--- samples/minimal-samples/powerflex.yaml | 2 + samples/minimal-samples/powerscale.yaml | 2 + 6 files changed, 55 insertions(+), 28 deletions(-) diff --git a/operatorconfig/moduleconfig/replication/v1.8.1/controller.yaml b/operatorconfig/moduleconfig/replication/v1.8.1/controller.yaml index c45bb6d02..42a7a9aa9 100644 --- a/operatorconfig/moduleconfig/replication/v1.8.1/controller.yaml +++ b/operatorconfig/moduleconfig/replication/v1.8.1/controller.yaml @@ -282,7 +282,7 @@ spec: value: /app/certs - name: X_CSI_REPLICATION_CONFIG_FILE_NAME value: config - image: + image: dellemc/dell-replication-controller:v1.8.1 imagePullPolicy: Always name: manager resources: diff --git a/operatorconfig/moduleconfig/replication/v1.9.0/controller.yaml b/operatorconfig/moduleconfig/replication/v1.9.0/controller.yaml index c45bb6d02..ddf3a2b17 100644 --- a/operatorconfig/moduleconfig/replication/v1.9.0/controller.yaml +++ b/operatorconfig/moduleconfig/replication/v1.9.0/controller.yaml @@ -282,7 +282,7 @@ spec: value: /app/certs - name: X_CSI_REPLICATION_CONFIG_FILE_NAME value: config - image: + image: dellemc/dell-replication-controller:v1.9.0 imagePullPolicy: Always name: manager resources: diff --git a/pkg/modules/replication.go b/pkg/modules/replication.go index 5eb19c0c0..019c11b00 100644 --- a/pkg/modules/replication.go +++ b/pkg/modules/replication.go @@ -27,6 +27,9 @@ import ( applyv1 "k8s.io/client-go/applyconfigurations/apps/v1" acorev1 "k8s.io/client-go/applyconfigurations/core/v1" + appsv1 "k8s.io/api/apps/v1" + crclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" ) @@ -92,9 +95,9 @@ func getRepctlPrefices(replicaModule csmv1.Module, driverType csmv1.DriverType) for _, component := range replicaModule.Components { if component.Name == utils.ReplicationSideCarName { for _, env := range component.Envs { - if env.Name == XCSIReplicaPrefix { + if env.Name == XCSIReplicaPrefix && env.Value != "" { replicationPrefix = env.Value - } else if env.Name == XCSIReplicaCTXPrefix { + } else if env.Name == XCSIReplicaCTXPrefix && env.Value != "" { replicationContextPrefix = env.Value } } @@ -132,6 +135,18 @@ func getReplicaApplyCR(cr csmv1.ContainerStorageModule, op utils.OperatorConfig) return nil, nil, err } + for _, component := range replicaModule.Components { + if component.Name == utils.ReplicationSideCarName { + if component.Image != "" { + image := string(component.Image) + container.Image = &image + } + if component.ImagePullPolicy != "" { + container.ImagePullPolicy = &component.ImagePullPolicy + } + } + } + return &replicaModule, &container, nil } @@ -320,17 +335,17 @@ func ReplicationPrecheck(ctx context.Context, op utils.OperatorConfig, replica c return nil } -func getReplicaController(op utils.OperatorConfig, cr csmv1.ContainerStorageModule) (string, error) { +func getReplicaController(op utils.OperatorConfig, cr csmv1.ContainerStorageModule) ([]crclient.Object, error) { YamlString := "" replica, err := getReplicaModule(cr) if err != nil { - return YamlString, err + return nil, err } buf, err := readConfigFile(replica, cr, op, "controller.yaml") if err != nil { - return YamlString, err + return nil, err } YamlString = utils.ModifyCommonCR(string(buf), cr) @@ -347,13 +362,13 @@ func getReplicaController(op utils.OperatorConfig, cr csmv1.ContainerStorageModu replicaImage = string(component.Image) } for _, env := range component.Envs { - if strings.Contains(DefaultLogLevel, env.Name) { + if strings.Contains(DefaultLogLevel, env.Name) && env.Value != "" { logLevel = env.Value - } else if strings.Contains(DefautlReplicaCount, env.Name) { + } else if strings.Contains(DefautlReplicaCount, env.Name) && env.Value != "" { replicaCount = env.Value - } else if strings.Contains(DefaultRetryMin, env.Name) { + } else if strings.Contains(DefaultRetryMin, env.Name) && env.Value != "" { retryMin = env.Value - } else if strings.Contains(DefaultRetryMax, env.Name) { + } else if strings.Contains(DefaultRetryMax, env.Name) && env.Value != "" { retryMax = env.Value } } @@ -366,11 +381,23 @@ func getReplicaController(op utils.OperatorConfig, cr csmv1.ContainerStorageModu YamlString = strings.ReplaceAll(YamlString, DefaultLogLevel, logLevel) YamlString = strings.ReplaceAll(YamlString, DefautlReplicaCount, replicaCount) - YamlString = strings.ReplaceAll(YamlString, DefaultReplicaImage, replicaImage) YamlString = strings.ReplaceAll(YamlString, DefaultReplicaInitImage, replicaInitImage) YamlString = strings.ReplaceAll(YamlString, DefaultRetryMax, retryMax) YamlString = strings.ReplaceAll(YamlString, DefaultRetryMin, retryMin) - return YamlString, nil + + ctrlObjects, err := utils.GetModuleComponentObj([]byte(YamlString)) + if err != nil { + return nil, err + } + // loop ctrlObjects to find the deployment and set the image + if len(replicaImage) != 0 { + for _, ctrlObj := range ctrlObjects { + if deployment, ok := ctrlObj.(*appsv1.Deployment); ok { + deployment.Spec.Template.Spec.Containers[0].Image = replicaImage + } + } + } + return ctrlObjects, nil } func getReplicaModule(cr csmv1.ContainerStorageModule) (csmv1.Module, error) { @@ -384,12 +411,7 @@ func getReplicaModule(cr csmv1.ContainerStorageModule) (csmv1.Module, error) { // ReplicationManagerController - func ReplicationManagerController(ctx context.Context, isDeleting bool, op utils.OperatorConfig, cr csmv1.ContainerStorageModule, ctrlClient client.Client) error { - YamlString, err := getReplicaController(op, cr) - if err != nil { - return err - } - - ctrlObjects, err := utils.GetModuleComponentObj([]byte(YamlString)) + ctrlObjects, err := getReplicaController(op, cr) if err != nil { return err } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index cc5210a47..f9043da7d 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -920,8 +920,10 @@ func MinVersionCheck(minVersion string, version string) (bool, error) { return false, nil } -func getClusterIDs(replica csmv1.Module) ([]string, error) { +func getClusterIDs(ctx context.Context, replica csmv1.Module) []string { + log := logger.GetLogger(ctx) var clusterIDs []string + if replica.Components == nil { // if we are entering this block then it is a minimal yaml and replication is enabled and we will set default cluster as self components := make([]csmv1.ContainerTemplate, 0) @@ -936,6 +938,7 @@ func getClusterIDs(replica csmv1.Module) ([]string, error) { }) } + for _, comp := range replica.Components { if comp.Name == ReplicationControllerManager { for _, env := range comp.Envs { @@ -946,11 +949,12 @@ func getClusterIDs(replica csmv1.Module) ([]string, error) { } } } - err := fmt.Errorf("TARGET_CLUSTERS_IDS on CR should have more than 0 commma seperated cluster IDs. Got %d", len(clusterIDs)) - if len(clusterIDs) >= 1 { - err = nil + + if len(clusterIDs) == 0 { + log.Infof("TARGET_CLUSTERS_IDS not found in CR. Using default value \"self\"") + clusterIDs = append(clusterIDs, "self") // defaults to same cluster for the replication } - return clusterIDs, err + return clusterIDs } func getConfigData(ctx context.Context, clusterID string, ctrlClient crclient.Client) ([]byte, error) { @@ -1025,10 +1029,7 @@ func GetDefaultClusters(ctx context.Context, instance csmv1.ContainerStorageModu for _, m := range instance.Spec.Modules { if m.Name == csmv1.Replication && m.Enabled { replicaEnabled = true - clusterIDs, err := getClusterIDs(m) - if err != nil { - return replicaEnabled, clusterClients, err - } + clusterIDs := getClusterIDs(ctx, m) for _, clusterID := range clusterIDs { /*Hack: skip-replication-cluster-check - skips check for csm_controller unit test diff --git a/samples/minimal-samples/powerflex.yaml b/samples/minimal-samples/powerflex.yaml index 114a5a67b..2b07eb1a7 100644 --- a/samples/minimal-samples/powerflex.yaml +++ b/samples/minimal-samples/powerflex.yaml @@ -22,6 +22,8 @@ spec: # false: disable Resiliency feature(do not deploy podmon sidecar) # Default value: false enabled: false + - name: replication + enabled: true # observability: allows to configure observability - name: observability # enabled: Enable/Disable observability diff --git a/samples/minimal-samples/powerscale.yaml b/samples/minimal-samples/powerscale.yaml index ba9a22f77..8c59fafdc 100644 --- a/samples/minimal-samples/powerscale.yaml +++ b/samples/minimal-samples/powerscale.yaml @@ -22,6 +22,8 @@ spec: # false: disable Resiliency feature(do not deploy podmon sidecar) # Default value: false enabled: false + - name: replication + enabled: true - name: observability # enabled: Enable/Disable observability enabled: false