Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Zone validation function for PowerFlex #821

Closed
wants to merge 9 commits into from
Closed

Adding Zone validation function for PowerFlex #821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
53d1450
Adding WIP ZoneValidation() to PowerFlex PreChecks that calls into
anathoodell Dec 10, 2024
b7b9198
GetMatchingNodes() using HasLabes() to match on Key rather than
anathoodell Dec 16, 2024
967ba93
2 code fixes, unit-test fakeclient fixes and ut augmentation, and uni…
anathoodell Dec 17, 2024
3573bd1
Updated client.go.
anathoodell Dec 17, 2024
03f67f4
Updating client.go.
anathoodell Dec 17, 2024
ee5355c
lint updates.
anathoodell Dec 17, 2024
d238c96
lint update on csm controller.
anathoodell Dec 17, 2024
580c5da
fixing gofumpt errors.
anathoodell Dec 17, 2024
33834fd
Merge branch 'main' into usr/anathoodell/zonevalidation
anathoodell Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 65 additions & 4 deletions controllers/csm_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func (r *ContainerStorageModuleReconciler) Reconcile(_ context.Context, req ctrl
}

// perform prechecks
err = r.PreChecks(ctx, csm, *operatorConfig)
err = r.PreChecks(ctx, csm, *operatorConfig, req.Namespace)
if err != nil {
csm.GetCSMStatus().State = constants.InvalidConfig
r.EventRecorder.Event(csm, corev1.EventTypeWarning, csmv1.EventUpdated, fmt.Sprintf("Failed Prechecks: %s", err))
Expand Down Expand Up @@ -1320,7 +1320,7 @@ func (r *ContainerStorageModuleReconciler) removeModule(ctx context.Context, ins
}

// PreChecks - validate input values
func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *csmv1.ContainerStorageModule, operatorConfig utils.OperatorConfig) error {
func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *csmv1.ContainerStorageModule, operatorConfig utils.OperatorConfig, namespace string) error {
log := logger.GetLogger(ctx)
// Check drivers
switch cr.Spec.Driver.CSIDriverType {
Expand All @@ -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, namespace)
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -1524,12 +1529,68 @@ func (r *ContainerStorageModuleReconciler) GetK8sClient() kubernetes.Interface {
return r.K8sClient
}

func (r *ContainerStorageModuleReconciler) GetMatchingNodes(ctx context.Context, labelKey string, labelValue string) (*corev1.NodeList, error) {
func (r *ContainerStorageModuleReconciler) GetMatchingNodes(ctx context.Context, labelKey string) (*corev1.NodeList, error) {
nodeList := &corev1.NodeList{}
opts := []client.ListOption{
client.MatchingLabels{labelKey: labelValue},
client.HasLabels{labelKey},
}
err := r.List(ctx, nodeList, opts...)

return nodeList, err
}

// Invalid zoning configurations:
// 1. Some arrays have a zone label A, but one of the arrays has zone label B, or no label
// 2. Array secret has zone label A, but one of the nodes has zone label B, or no label
// 3. Some of the nodes have a zone label, but arrays have no zone labels.
// 4. All of the arrays have a zone label, but nodes have no zone label.
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
func (r *ContainerStorageModuleReconciler) ZoneValidation(ctx context.Context, cr *csmv1.ContainerStorageModule, namespace string) error {
log := logger.GetLogger(ctx)
nodeList := &corev1.NodeList{}

err := r.Client.List(ctx, nodeList)
if err != nil {
return fmt.Errorf("error listing nodes %v", err)
}
numberOfNodes := len(nodeList.Items)

zones, err := drivers.ExtractZones(ctx, cr, r.Client, namespace)
if err != nil {
return fmt.Errorf("ZoneValidation failed with error: %v", err)
}

// Make sure all zone keys are the same, if not fail
var firstKeyLabel string
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
for key := range zones {
if firstKeyLabel == "" {
firstKeyLabel = key
continue
}
if key != firstKeyLabel {
return fmt.Errorf("detected mismatched Keys in zones")
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
}
}
// TBD: also check if there any arrays specified in the secret that do not have
// a zone label. this constitutes a zone configuration failure.
// Determine how many arrays are configured and match that with the len(map)

// The labelKey, GetMatchingNodes with labelKey, verify that the list
// returned is the same size as the current number of nodes on the cluster
// i.e. all nodes must have this zoning label key. if the number of nodes
// returned does not match the number of nodes on the cluster, then fail,
// otherwise return true
if firstKeyLabel != "" {
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
matchingNodeList, err := r.GetMatchingNodes(ctx, firstKeyLabel)
if err != nil {
return fmt.Errorf("Failed to retrieve list of nodes for label: %s", firstKeyLabel)
}
for _, node := range matchingNodeList.Items {
log.Infof(" matching node found with node name %s", node.Name)
}
if len(matchingNodeList.Items) != numberOfNodes {
return fmt.Errorf("zone and node configuration mismatch %d", len(matchingNodeList.Items))
}
}

return err
}
175 changes: 152 additions & 23 deletions controllers/csm_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,8 @@ func (suite *CSMControllerTestSuite) TestCsmDowngrade() {
if err != nil {
panic(err)
}
sec := shared.MakeSecret(csmName+"-config", suite.namespace, pFlexConfigVersion)
// 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)
Expand Down Expand Up @@ -653,7 +654,8 @@ func (suite *CSMControllerTestSuite) TestCsmDowngradeSkipVersion() {
if err != nil {
panic(err)
}
sec := shared.MakeSecret(csmName+"-config", suite.namespace, pFlexConfigVersion)
// 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)
Expand Down Expand Up @@ -1030,32 +1032,32 @@ func (suite *CSMControllerTestSuite) TestCsmPreCheckModuleError() {

// error in Authorization
csm.Spec.Modules = getAuthModule()
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig)
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig, "")
assert.NotNil(suite.T(), err)

// error in Authorization Proxy Server
csm.Spec.Modules = getAuthProxyServer()
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig)
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig, "")
assert.NotNil(suite.T(), err)

// error in App-Mobility
csm.Spec.Modules = getAppMob()
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig)
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig, "")
assert.NotNil(suite.T(), err)

// error in Replication
csm.Spec.Modules = getReplicaModule()
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig)
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig, "")
assert.NotNil(suite.T(), err)

// error in Resiliency
csm.Spec.Modules = getResiliencyModule()
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig)
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig, "")
assert.NotNil(suite.T(), err)

// error in Observability
csm.Spec.Modules = getObservabilityModule()
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig)
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig, "")
assert.NotNil(suite.T(), err)

// error unsupported module
Expand All @@ -1065,7 +1067,7 @@ func (suite *CSMControllerTestSuite) TestCsmPreCheckModuleError() {
Enabled: true,
},
}
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig)
err = reconciler.PreChecks(ctx, &csm, badOperatorConfig, "")
assert.NotNil(suite.T(), err)
}

Expand All @@ -1091,31 +1093,31 @@ func (suite *CSMControllerTestSuite) TestCsmPreCheckModuleUnsupportedVersion() {
// error in Authorization
csm.Spec.Modules = getAuthModule()
csm.Spec.Modules[0].ConfigVersion = "1.0.0"
err = reconciler.PreChecks(ctx, &csm, operatorConfig)
err = reconciler.PreChecks(ctx, &csm, operatorConfig, "")
assert.NotNil(suite.T(), err)

// error in Authorization Proxy Server
csm.Spec.Modules = getAuthProxyServer()
csm.Spec.Modules[0].ConfigVersion = "1.0.0"
err = reconciler.PreChecks(ctx, &csm, operatorConfig)
err = reconciler.PreChecks(ctx, &csm, operatorConfig, "")
assert.NotNil(suite.T(), err)

// error in Replication
csm.Spec.Modules = getReplicaModule()
csm.Spec.Modules[0].ConfigVersion = "1.0.0"
err = reconciler.PreChecks(ctx, &csm, operatorConfig)
err = reconciler.PreChecks(ctx, &csm, operatorConfig, "")
assert.NotNil(suite.T(), err)

// error in Resiliency
csm.Spec.Modules = getResiliencyModule()
csm.Spec.Modules[0].ConfigVersion = "1.0.0"
err = reconciler.PreChecks(ctx, &csm, operatorConfig)
err = reconciler.PreChecks(ctx, &csm, operatorConfig, "")
assert.NotNil(suite.T(), err)

// error in Observability
csm.Spec.Modules = getObservabilityModule()
csm.Spec.Modules[0].ConfigVersion = "1.0.0"
err = reconciler.PreChecks(ctx, &csm, operatorConfig)
err = reconciler.PreChecks(ctx, &csm, operatorConfig, "")
assert.NotNil(suite.T(), err)

// error unsupported module
Expand All @@ -1125,7 +1127,7 @@ func (suite *CSMControllerTestSuite) TestCsmPreCheckModuleUnsupportedVersion() {
Enabled: true,
},
}
err = reconciler.PreChecks(ctx, &csm, operatorConfig)
err = reconciler.PreChecks(ctx, &csm, operatorConfig, "")
assert.NotNil(suite.T(), err)
}

Expand Down Expand Up @@ -2384,10 +2386,6 @@ func (suite *CSMControllerTestSuite) makeFakeRevProxyCSM(name string, ns string,
}

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.

csm := shared.MakeCSM(csmName, suite.namespace, configVersion)
csm.Spec.Driver.CSIDriverType = csmv1.PowerScale
csm.Spec.Driver.Common.Image = "image"
Expand All @@ -2414,12 +2412,13 @@ func (suite *CSMControllerTestSuite) TestGetNodeLabels() {
err = suite.fakeClient.Create(ctx, &node)
assert.Nil(suite.T(), err)

nodeList := &corev1.NodeList{}
err = suite.fakeClient.List(ctx, nodeList, nil)
node2 := shared.MakeNode("node2", suite.namespace)

err = suite.fakeClient.Create(ctx, &node2)
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))
nodeListMatching, err := reconciler.GetMatchingNodes(ctx, "topology.kubernetes.io/zone")
ctrl.Log.Info("node list response expecting (1)", "number of nodes matching is: ", len(nodeListMatching.Items))

// Check the len to be 1 else fail
if len(nodeListMatching.Items) != 1 {
Expand All @@ -2436,3 +2435,133 @@ func (suite *CSMControllerTestSuite) TestGetNodeLabels() {
}
assert.Nil(suite.T(), err)
}

func (suite *CSMControllerTestSuite) TestZoneValidation() {
csm := shared.MakeCSM(csmName, suite.namespace, configVersion)
csm.Spec.Driver.CSIDriverType = csmv1.PowerFlex
csm.Spec.Driver.Common.Image = "image"
csm.Annotations[configVersionKey] = configVersion

csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
err := suite.fakeClient.Create(ctx, &csm)
if err != nil {
panic(err)
}
anathoodell marked this conversation as resolved.
Show resolved Hide resolved

reconciler := suite.createReconciler()

node := shared.MakeNode("node1", suite.namespace)
node.Labels["topology.kubernetes.io/zone"] = "US-EAST"

err = suite.fakeClient.Create(ctx, &node)
assert.Nil(suite.T(), err)

node2 := shared.MakeNode("node2", suite.namespace)

err = suite.fakeClient.Create(ctx, &node2)
assert.Nil(suite.T(), err)

// add secret with NO zone to the namespace
sec := shared.MakeSecretPowerFlex(csmName+"-config", suite.namespace, pFlexConfigVersion)
err = suite.fakeClient.Create(ctx, sec)
if err != nil {
panic(err)
}

err = reconciler.ZoneValidation(ctx, &csm, suite.namespace)
if err != nil {
panic(err)
}
}

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

csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
err := suite.fakeClient.Create(ctx, &csm)
if err != nil {
panic(err)
}

reconciler := suite.createReconciler()

node := shared.MakeNode("node1", suite.namespace)
node.Labels["zone.csi-vxflexos.dellemc.com"] = "US-WEST"
err = suite.fakeClient.Create(ctx, &node)
assert.Nil(suite.T(), err)

node2 := shared.MakeNode("node2", suite.namespace)
node2.Labels["zone.csi-vxflexos.dellemc.com"] = "US-EAST"
err = suite.fakeClient.Create(ctx, &node2)
assert.Nil(suite.T(), err)

node3 := shared.MakeNode("node3", suite.namespace)
node3.Labels["zone.csi-vxflexos.dellemc.com"] = "US-SOUTH"
err = suite.fakeClient.Create(ctx, &node3)
assert.Nil(suite.T(), err)

// add secret with zone to the namespace
secretZone := shared.MakeSecretPowerFlexWithZone(csmName+"-config", suite.namespace, pFlexConfigVersion)
err = suite.fakeClient.Create(ctx, secretZone)
if err != nil {
panic(err)
}

err = reconciler.ZoneValidation(ctx, &csm, suite.namespace)
if err != nil {
panic(err)
}

// Add node4 that has a different zone label, and zone validate should fail
node4 := shared.MakeNode("node4", suite.namespace)
node4.Labels["my-special-zone-label"] = "mydrzone"
err = suite.fakeClient.Create(ctx, &node4)
assert.Nil(suite.T(), err)

err = reconciler.ZoneValidation(ctx, &csm, suite.namespace)
if err == nil {
panic(err)
}
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
}

func (suite *CSMControllerTestSuite) TestZoneValidation3() {
csm := shared.MakeCSM(csmName, suite.namespace, configVersion)
csm.Spec.Driver.CSIDriverType = csmv1.PowerFlex
csm.Spec.Driver.Common.Image = "image"
csm.Annotations[configVersionKey] = configVersion

csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
err := suite.fakeClient.Create(ctx, &csm)
if err != nil {
panic(err)
}
anathoodell marked this conversation as resolved.
Show resolved Hide resolved

reconciler := suite.createReconciler()

node := shared.MakeNode("node1", suite.namespace)
node.Labels["zone.csi-vxflexos.dellemc.com"] = "US-SOUTH"

err = suite.fakeClient.Create(ctx, &node)
assert.Nil(suite.T(), err)

node2 := shared.MakeNode("node2", suite.namespace)
node2.Labels["zone.csi-vxflexos.dellemc.com"] = "US-SOUTH"

err = suite.fakeClient.Create(ctx, &node2)
assert.Nil(suite.T(), err)

// add secret with invalid Multi zone to the namespace
sec := shared.MakeSecretPowerFlexMultiZone(csmName+"-config", suite.namespace, pFlexConfigVersion)
err = suite.fakeClient.Create(ctx, sec)
if err != nil {
panic(err)
}

err = reconciler.ZoneValidation(ctx, &csm, suite.namespace)
if err == nil {
panic(err)
}
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
}
8 changes: 7 additions & 1 deletion pkg/drivers/powerflex.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,12 @@ func ModifyPowerflexCR(yamlString string, cr csmv1.ContainerStorageModule, fileT
return yamlString
}

func ExtractZones(ctx context.Context, cr *csmv1.ContainerStorageModule, ct client.Client, namespace string) (map[string]string, error) {
secretName := cr.Name + "-config"
zonesMapData, err := ExtractZonesFromSecret(ctx, ct, namespace, secretName)
return zonesMapData, err
}

// 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) {
log := logger.GetLogger(ctx)
Expand Down Expand Up @@ -375,7 +381,7 @@ func ExtractZonesFromSecret(ctx context.Context, kube client.Client, namespace s
}
err = yaml.Unmarshal(configs, &yamlConfig)
if err != nil {
return nil, fmt.Errorf("unable to unmarshal multi-array configuration[%v]", err)
return nil, fmt.Errorf("unable to unmarshal no Zone or Name or LabelKey found multi-array configuration[%v]", err)
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
anathoodell marked this conversation as resolved.
Show resolved Hide resolved
}

for _, configParam := range yamlConfig {
Expand Down
Loading
Loading