Powerflex secret zone validation

controllers/csm_controller.go

@@ -1334,6 +1334,11 @@ func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *cs
 		if err != nil {
 			return fmt.Errorf("failed powerflex validation: %v", err)
 		}
+		// zoning initially applies only to pflex
+		err = r.ZoneValidation(ctx, cr)
+		if err != nil {
+			return fmt.Errorf("error during zone validation: %v", err)
+		}
 	case csmv1.PowerStore:
 		err := drivers.PrecheckPowerStore(ctx, cr, operatorConfig, r.GetClient())
 		if err != nil {
@@ -1524,12 +1529,12 @@ func (r *ContainerStorageModuleReconciler) GetK8sClient() kubernetes.Interface {
 	return r.K8sClient
 }
 
-func (r *ContainerStorageModuleReconciler) GetMatchingNodes(ctx context.Context, labelKey string, labelValue string) (*corev1.NodeList, error) {
-	nodeList := &corev1.NodeList{}
-	opts := []client.ListOption{
-		client.MatchingLabels{labelKey: labelValue},
+// ZoneValidation - If zones are configured performs validation and returns an error if the zone validation fails
+func (r *ContainerStorageModuleReconciler) ZoneValidation(ctx context.Context, cr *csmv1.ContainerStorageModule) error {
+	err := drivers.ValidateZones(ctx, cr, r.Client)
+	if err != nil {
+		return fmt.Errorf("zone validation failed with error: %v", err)
 	}
-	err := r.List(ctx, nodeList, opts...)
 
-	return nodeList, err
+	return err
 }

controllers/csm_controller_test.go

@@ -585,7 +585,7 @@ func (suite *CSMControllerTestSuite) TestCsmDowngrade() {
 	if err != nil {
 		panic(err)
 	}
-	sec := shared.MakeSecret(csmName+"-config", suite.namespace, pFlexConfigVersion)
+	sec := shared.MakeSecretPowerFlex(csmName+"-config", suite.namespace, pFlexConfigVersion)
 	err = suite.fakeClient.Create(ctx, sec)
 	if err != nil {
 		panic(err)
@@ -653,7 +653,7 @@ func (suite *CSMControllerTestSuite) TestCsmDowngradeSkipVersion() {
 	if err != nil {
 		panic(err)
 	}
-	sec := shared.MakeSecret(csmName+"-config", suite.namespace, pFlexConfigVersion)
+	sec := shared.MakeSecretPowerFlex(csmName+"-config", suite.namespace, pFlexConfigVersion)
 	err = suite.fakeClient.Create(ctx, sec)
 	if err != nil {
 		panic(err)
@@ -2383,56 +2383,44 @@ func (suite *CSMControllerTestSuite) makeFakeRevProxyCSM(name string, ns string,
 	assert.Nil(suite.T(), err)
 }
 
-func (suite *CSMControllerTestSuite) TestGetNodeLabels() {
-	// TBD: crclient/client.go needs to be augmented to filter on labels during
-	// the List return for a viable thorough test. Since this functionality is
-	// missing, this test is quite elementary as a result.
-
+func (suite *CSMControllerTestSuite) TestZoneValidation() {
 	csm := shared.MakeCSM(csmName, suite.namespace, configVersion)
-	csm.Spec.Driver.CSIDriverType = csmv1.PowerScale
+	csm.Spec.Driver.CSIDriverType = csmv1.PowerFlex
 	csm.Spec.Driver.Common.Image = "image"
 	csm.Annotations[configVersionKey] = configVersion
 
-	sec := shared.MakeSecret(csmName+"-creds", suite.namespace, configVersion)
-	err := suite.fakeClient.Create(ctx, sec)
-	if err != nil {
-		panic(err)
-	}
-
 	csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
-	err = suite.fakeClient.Create(ctx, &csm)
-	if err != nil {
-		panic(err)
-	}
+	err := suite.fakeClient.Create(ctx, &csm)
+	assert.Nil(suite.T(), err)
 
 	reconciler := suite.createReconciler()
 
-	// create node object, add to fakeclient, reconcile.GetMatchingNodes
-	node := shared.MakeNode("node1", suite.namespace)
-	node.Labels["topology.kubernetes.io/zone"] = "US-EAST"
-
-	err = suite.fakeClient.Create(ctx, &node)
+	// add secret with NO zone to the namespace
+	sec := shared.MakeSecretPowerFlex(csmName+"-config", suite.namespace, pFlexConfigVersion)
+	err = suite.fakeClient.Create(ctx, sec)
 	assert.Nil(suite.T(), err)
 
-	nodeList := &corev1.NodeList{}
-	err = suite.fakeClient.List(ctx, nodeList, nil)
+	err = reconciler.ZoneValidation(ctx, &csm)
 	assert.Nil(suite.T(), err)
+}
 
-	nodeListMatching, err := reconciler.GetMatchingNodes(ctx, "topology.kubernetes.io/zone", "US-EAST")
-	ctrl.Log.Info("node list response (1)", "number of nodes is: ", len(nodeListMatching.Items))
+func (suite *CSMControllerTestSuite) TestZoneValidation2() {
+	csm := shared.MakeCSM(csmName, suite.namespace, configVersion)
+	csm.Spec.Driver.CSIDriverType = csmv1.PowerFlex
+	csm.Spec.Driver.Common.Image = "image"
+	csm.Annotations[configVersionKey] = configVersion
 
-	// Check the len to be 1 else fail
-	if len(nodeListMatching.Items) != 1 {
-		ctrl.Log.Error(err, "Unexpected length on node list.", "length", len(nodeListMatching.Items))
-		panic(err)
-	}
+	csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
+	err := suite.fakeClient.Create(ctx, &csm)
+	assert.Nil(suite.T(), err)
 
-	for _, node := range nodeListMatching.Items {
-		ctrl.Log.Info("Matching node item", "name ", node.ObjectMeta.GetName())
-	}
-	if node.ObjectMeta.GetName() != "node1" {
-		ctrl.Log.Error(err, "Unmatched label on node.")
-		panic(err)
-	}
+	reconciler := suite.createReconciler()
+
+	// add secret with an invalid multi zone to the namespace
+	secretZone := shared.MakeSecretPowerFlexMultiZoneInvalid(csmName+"-config", suite.namespace, pFlexConfigVersion)
+	err = suite.fakeClient.Create(ctx, secretZone)
 	assert.Nil(suite.T(), err)
+
+	err = reconciler.ZoneValidation(ctx, &csm)
+	assert.NotNil(suite.T(), err)
 }

go.mod

@@ -55,11 +55,11 @@ require (
 	github.com/x448/float16 v0.8.4 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	golang.org/x/exp v0.0.0-20240416160154-fe59bbe5cc7f // indirect
-	golang.org/x/net v0.31.0 // indirect
+	golang.org/x/net v0.33.0 // indirect
 	golang.org/x/oauth2 v0.21.0 // indirect
-	golang.org/x/sys v0.27.0 // indirect
-	golang.org/x/term v0.26.0 // indirect
-	golang.org/x/text v0.20.0 // indirect
+	golang.org/x/sys v0.28.0 // indirect
+	golang.org/x/term v0.27.0 // indirect
+	golang.org/x/text v0.21.0 // indirect
 	golang.org/x/time v0.5.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/protobuf v1.35.1 // indirect

go.sum

@@ -120,8 +120,8 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
-golang.org/x/net v0.31.0 h1:68CPQngjLL0r2AlUKiSxtQFKvzRVbnzLwMUn5SzcLHo=
-golang.org/x/net v0.31.0/go.mod h1:P4fl1q7dY2hnZFxEk4pPSkDHF+QqjitcnDjUQyMM+pM=
+golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I=
+golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
 golang.org/x/oauth2 v0.21.0 h1:tsimM75w1tF/uws5rbeHzIWxEqElMehnc+iW793zsZs=
 golang.org/x/oauth2 v0.21.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
@@ -130,14 +130,14 @@ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s=
-golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
-golang.org/x/term v0.26.0 h1:WEQa6V3Gja/BhNxg540hBip/kkaYtRg3cxg4oXSw4AU=
-golang.org/x/term v0.26.0/go.mod h1:Si5m1o57C5nBNQo5z1iq+XDijt21BDBDp2bK0QI8e3E=
+golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
+golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q=
+golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
-golang.org/x/text v0.20.0 h1:gK/Kv2otX8gz+wn7Rmb3vT96ZwuoxnQlY+HlJVj7Qug=
-golang.org/x/text v0.20.0/go.mod h1:D4IsuqiFMhST5bX19pQ9ikHC2GsaKyk/oF+pn3ducp4=
+golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
+golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
 golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk=
 golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=

pkg/drivers/powerflex.go

@@ -16,6 +16,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"reflect"
 	"regexp"
 	"strings"
 
@@ -347,52 +348,87 @@ func ModifyPowerflexCR(yamlString string, cr csmv1.ContainerStorageModule, fileT
 	return yamlString
 }
 
-// ExtractZonesFromSecret - Reads the array config secret and returns the zone label information
-func ExtractZonesFromSecret(ctx context.Context, kube client.Client, namespace string, secret string) (map[string]string, error) {
+// ValidateZones - zone validation for topology aware clusters
+func ValidateZones(ctx context.Context, cr *csmv1.ContainerStorageModule, ct client.Client) error {
+	secretName := cr.Name + "-config"
+	err := ValidateZonesInSecret(ctx, ct, cr.Namespace, secretName)
+	return err
+}
+
+// ValidateZonesInSecret - inspects incoming secret for zone validity
+func ValidateZonesInSecret(ctx context.Context, kube client.Client, namespace string, secret string) error {
 	log := logger.GetLogger(ctx)
 
 	arraySecret, err := utils.GetSecret(ctx, secret, namespace, kube)
 	if err != nil {
-		return nil, fmt.Errorf("reading secret [%s] error [%s]", secret, err)
+		return fmt.Errorf("reading secret [%s] error %v", secret, err)
+	}
+
+	type Zone struct {
+		Name     string `json:"name,omitempty"`
+		LabelKey string `json:"labelKey,omitempty"`
 	}
 
 	type StorageArrayConfig struct {
-		SystemID string `json:"systemId"`
-		Zone     struct {
-			Name string `json:"name"`
-		} `json:"zone"`
+		SystemID string `json:"systemID"`
+		Zone     Zone   `json:"zone,omitempty"`
 	}
 
 	data := arraySecret.Data
 	configBytes := data["config"]
-	zonesMapData := make(map[string]string)
 
 	if string(configBytes) != "" {
 		yamlConfig := make([]StorageArrayConfig, 0)
 		configs, err := yaml.JSONToYAML(configBytes)
 		if err != nil {
-			return nil, fmt.Errorf("unable to parse multi-array configuration[%v]", err)
+			return fmt.Errorf("malformed json in array secret - unable to parse multi-array configuration %v", err)
 		}
 		err = yaml.Unmarshal(configs, &yamlConfig)
 		if err != nil {
-			return nil, fmt.Errorf("unable to unmarshal multi-array configuration[%v]", err)
+			return fmt.Errorf("unable to unmarshal array secret %v", err)
 		}
 
+		var labelKey string
+		var numArrays, numArraysWithZone int
+		numArrays = len(yamlConfig)
 		for _, configParam := range yamlConfig {
 			if configParam.SystemID == "" {
-				return nil, fmt.Errorf("invalid value for SystemID")
+				return fmt.Errorf("invalid value for SystemID")
 			}
-			if configParam.Zone.Name != "" {
-				zonesMapData[configParam.SystemID] = configParam.Zone.Name
-				log.Infof("Zoning information configured for systemID %s: %v ", configParam.SystemID, zonesMapData)
+			if reflect.DeepEqual(configParam.Zone, Zone{}) {
+				log.Infof("Zone is not specified for SystemID: %s", configParam.SystemID)
 			} else {
-				log.Info("Zoning information not found in the array config. Continue with topology-unaware driver installation mode")
-				return zonesMapData, nil
+				log.Infof("Zone is specified for SystemID: %s", configParam.SystemID)
+				if configParam.Zone.LabelKey == "" {
+					return fmt.Errorf("zone LabelKey is empty or not specified for SystemID: %s",
+						configParam.SystemID)
+				}
+
+				if labelKey == "" {
+					labelKey = configParam.Zone.LabelKey
+				} else {
+					if labelKey != configParam.Zone.LabelKey {
+						return fmt.Errorf("labelKey is not consistent across all arrays in secret")
+					}
+				}
+
+				if configParam.Zone.Name == "" {
+					return fmt.Errorf("zone name is empty or not specified for SystemID: %s",
+						configParam.SystemID)
+				}
+				numArraysWithZone++
 			}
 		}
+
+		log.Infof("found %d arrays zoning on %d", numArrays, numArraysWithZone)
+		if numArraysWithZone > 0 && numArrays != numArraysWithZone {
+			return fmt.Errorf("not all arrays have zoning configured. Check the array info secret, zone key should be the same for all arrays")
+		} else if numArraysWithZone == 0 {
+			log.Info("Zoning information not found in the array secret. Continue with topology-unaware driver installation mode")
+		}
 	} else {
-		return nil, fmt.Errorf("Array details are not provided in vxflexos-config secret")
+		return fmt.Errorf("array details are not provided in secret")
 	}
 
-	return zonesMapData, nil
+	return nil
 }