Skip to content

Commit

Permalink
[Bugfix]: Powerflex minimal manifest issue (#855)
Browse files Browse the repository at this point in the history
* fix issue w/ min manifests

* fix: updated the scenarios

* remove bad line from scenarios.yaml

* change replicas to 1 when deploying to control plane node

* add UT to cover missing scenario

* fix unit tests

---------

Co-authored-by: JacobGros <jacobgrosner4@gmail.com>
  • Loading branch information
mgandharva and JacobGros authored Jan 10, 2025
1 parent e984dc1 commit 6fd8269
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 8 deletions.
7 changes: 6 additions & 1 deletion pkg/drivers/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func csmWithTolerations(driver csmv1.DriverType, version string) csmv1.Container
return res
}

// JJL make a functino that uses the standard name and calls this function
// JJL make a function that uses the standard name and calls this function
// makes a pflex csm object
func csmForPowerFlex(customCSMName string) csmv1.ContainerStorageModule {
res := shared.MakeCSM(customCSMName, pFlexNS, shared.PFlexConfigVersion)
Expand Down Expand Up @@ -158,6 +158,11 @@ func csmForPowerFlex(customCSMName string) csmv1.ContainerStorageModule {
// Add driver common image
res.Spec.Driver.Common.Image = "driverimage"

// minimal manifests will not have a common section
if customCSMName == "no-common-section" {
res.Spec.Driver.Common = nil
}

return res
}

Expand Down
14 changes: 10 additions & 4 deletions pkg/drivers/commonconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,11 @@ func GetController(ctx context.Context, cr csmv1.ContainerStorageModule, operato
}

controllerYAML := driverYAML.(utils.ControllerYAML)
controllerYAML.Deployment.Spec.Replicas = &cr.Spec.Driver.Replicas

// if using a minimal manifest, replicas may not be present.
if cr.Spec.Driver.Replicas != 0 {
controllerYAML.Deployment.Spec.Replicas = &cr.Spec.Driver.Replicas
}
if cr.Spec.Driver.Controller != nil && len(cr.Spec.Driver.Controller.Tolerations) != 0 {
tols := make([]acorev1.TolerationApplyConfiguration, 0)
for _, t := range cr.Spec.Driver.Controller.Tolerations {
Expand Down Expand Up @@ -340,9 +343,12 @@ func GetNode(ctx context.Context, cr csmv1.ContainerStorageModule, operatorConfi
utils.UpdateInitContainerApply(updatedCr.Spec.Driver.InitContainers, &initcontainers[i])
// mdm-container is exclusive to powerflex driver deamonset, will use the driver image as an init container
if *initcontainers[i].Name == "mdm-container" {
if string(cr.Spec.Driver.Common.Image) != "" {
image := string(cr.Spec.Driver.Common.Image)
initcontainers[i].Image = &image
// driver minimial manifest may not have common section
if cr.Spec.Driver.Common != nil {
if string(cr.Spec.Driver.Common.Image) != "" {
image := string(cr.Spec.Driver.Common.Image)
initcontainers[i].Image = &image
}
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/drivers/commonconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ var (
{"powerscale happy path", pScaleCSM, csmv1.PowerScaleName, "node.yaml", ""},
{"pflex happy path", pFlexCSM, csmv1.PowerFlex, "node.yaml", ""},
{"pflex no-sdc path", csmForPowerFlex("no-sdc"), csmv1.PowerFlex, "node.yaml", ""},
{"pflex with no common section", csmForPowerFlex("no-common-section"), csmv1.PowerFlex, "node.yaml", ""},
{"pstore happy path", pStoreCSM, csmv1.PowerStore, "node.yaml", ""},
{"unity happy path", unityCSM, csmv1.Unity, "node.yaml", ""},
{"unity happy path when secrets with certificates provided", unityCSMCertProvided, csmv1.Unity, "node.yaml", ""},
Expand Down Expand Up @@ -142,7 +143,10 @@ func TestGetNode(t *testing.T) {
for i := range initcontainers {
if *initcontainers[i].Name == "mdm-container" {
foundInitMdm = true
assert.Equal(t, string(tt.csm.Spec.Driver.Common.Image), *initcontainers[i].Image)
// if min manifest test case, there will be no common section
if tt.name != "pflex with no common section" {
assert.Equal(t, string(tt.csm.Spec.Driver.Common.Image), *initcontainers[i].Image)
}
}
}
// if driver is powerflex, then check that mdm-container is present
Expand Down
1 change: 0 additions & 1 deletion tests/e2e/testfiles/scenarios.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@
- "Delete custom resource [4]"
- "Validate [powerflex] driver from CR [4] is not installed"
- "Delete custom resource [2]"
- "Validate [authorization-proxy-server] module from CR [2] is not installed"
- "Restore template [testfiles/powerflex-templates/powerflex-storageclass-template.yaml] for [pflex]"
customTest:
- name: Cert CSI
Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/testfiles/storage_csm_powerflex_alt_vals_2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ spec:
# Default value: ReadWriteOnceWithFSType
fSGroupPolicy: "None"
configVersion: v2.13.0
replicas: 2
# setting replicas to 1 since this file is using control node to install deployment on control plane node
replicas: 1
dnsPolicy: ClusterFirstWithHostNet
forceRemoveDriver: false
common:
Expand Down

0 comments on commit 6fd8269

Please sign in to comment.