fix: fixed 'addon enable' cmd with specified flags doesn't modified `…

…spec.install.enabled` attribute (apecloud#3082)
NoicFank · May 5, 2023 · fdbeceb · fdbeceb
1 parent 8294e1c
commit fdbeceb
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 11 deletions.
diff --git a/apis/extensions/v1alpha1/addon_types.go b/apis/extensions/v1alpha1/addon_types.go
@@ -531,6 +531,19 @@ func (r *HelmTypeInstallSpec) BuildMergedValues(installSpec *AddonInstallSpec) H
 	}
 	installValues := r.InstallValues
 	processor := func(installSpecItem AddonInstallSpecItem, valueMapping HelmValuesMappingItem) {
+		var pvEnabled *bool
+		defer func() {
+			if v := valueMapping.HelmValueMap.PVEnabled; v != "" && pvEnabled != nil {
+				installValues.SetValues = append(installValues.SetValues,
+					fmt.Sprintf("%s=%v", v, *pvEnabled))
+			}
+		}()
+
+		if installSpecItem.PVEnabled != nil {
+			b := *installSpecItem.PVEnabled
+			pvEnabled = &b
+		}
+
 		if installSpecItem.Replicas != nil && *installSpecItem.Replicas >= 0 {
 			if v := valueMapping.HelmValueMap.ReplicaCount; v != "" {
 				installValues.SetValues = append(installValues.SetValues,
@@ -550,13 +563,6 @@ func (r *HelmTypeInstallSpec) BuildMergedValues(installSpec *AddonInstallSpec) H
 			}
 		}
 
-		if installSpecItem.PVEnabled != nil {
-			if v := valueMapping.HelmValueMap.PVEnabled; v != "" {
-				installValues.SetValues = append(installValues.SetValues,
-					fmt.Sprintf("%s=%v", v, *installSpecItem.PVEnabled))
-			}
-		}
-
 		if installSpecItem.Tolerations != "" {
 			if v := valueMapping.HelmJSONMap.Tolerations; v != "" {
 				installValues.SetJSONValues = append(installValues.SetJSONValues,
@@ -602,6 +608,7 @@ func (r *HelmTypeInstallSpec) BuildMergedValues(installSpec *AddonInstallSpec) H
 				}
 			}
 		}
+
 	}
 	processor(installSpec.AddonInstallSpecItem, r.ValuesMapping.HelmValuesMappingItem)
 	for _, ei := range installSpec.ExtraItems {

diff --git a/docs/user_docs/cli/kbcli_addon_enable.md b/docs/user_docs/cli/kbcli_addon_enable.md
@@ -47,7 +47,9 @@ kbcli addon enable ADDON_NAME [flags]
       --set stringArray                set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2), it's only being processed if addon's type is helm.
       --show-managed-fields            If true, keep the managedFields when printing objects in JSON or YAML format.
       --storage stringArray            Sets addon storage size (--storage [extraName:]<request>) (can specify multiple if has extra items)). 
-                                       Additional notes for Helm type Addon, that resizing storage will fail if modified value is a storage request size 
+                                       Additional notes:
+                                       1. Specify '0' value will removed storage values settings and explicitly disabled 'persistentVolumeEnabled' attribute.
+                                       2. For Helm type Addon, that resizing storage will fail if modified value is a storage request size 
                                        that belongs to StatefulSet's volume claim template, to resolve 'Failed' Addon status possible action is disable and 
                                        re-enable the addon (More info on how-to resize a PVC: https://kubernetes.io/docs/concepts/storage/persistent-volumes#resources).
                                        

diff --git a/internal/cli/cmd/addon/addon.go b/internal/cli/cmd/addon/addon.go
@@ -203,7 +203,9 @@ func newEnableCmd(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra
 		"Sets addon CPU resource values (--cpu [extraName:]<request>/<limit>) (can specify multiple if has extra items))")
 	cmd.Flags().StringArrayVar(&o.addonEnableFlags.StorageSets, "storage", []string{},
 		`Sets addon storage size (--storage [extraName:]<request>) (can specify multiple if has extra items)). 
-Additional notes for Helm type Addon, that resizing storage will fail if modified value is a storage request size 
+Additional notes:
+1. Specify '0' value will removed storage values settings and explicitly disabled 'persistentVolumeEnabled' attribute.
+2. For Helm type Addon, that resizing storage will fail if modified value is a storage request size 
 that belongs to StatefulSet's volume claim template, to resolve 'Failed' Addon status possible action is disable and 
 re-enable the addon (More info on how-to resize a PVC: https://kubernetes.io/docs/concepts/storage/persistent-volumes#resources).
 `)
@@ -427,6 +429,7 @@ func addonEnableDisableHandler(o *addonCmdOpts, cmd *cobra.Command, args []strin
 func (o *addonCmdOpts) buildEnablePatch(flags []*pflag.Flag, spec, install map[string]interface{}) (err error) {
 	extraNames := o.addon.GetExtraNames()
 	installSpec := extensionsv1alpha1.AddonInstallSpec{
+		Enabled:              true,
 		AddonInstallSpecItem: extensionsv1alpha1.NewAddonInstallSpecItem(),
 	}
 	// only using named return value in defer function
@@ -445,7 +448,6 @@ func (o *addonCmdOpts) buildEnablePatch(flags []*pflag.Flag, spec, install map[s
 	}()
 
 	if o.addonEnableFlags.useDefault() {
-		installSpec.Enabled = true
 		return nil
 	}
 
@@ -599,7 +601,18 @@ func (o *addonCmdOpts) buildEnablePatch(flags []*pflag.Flag, spec, install map[s
 			}
 			return q, nil
 		}, func(item *extensionsv1alpha1.AddonInstallSpecItem, i interface{}) {
-			item.Resources.Requests[corev1.ResourceStorage] = i.(resource.Quantity)
+			q := i.(resource.Quantity)
+			// for 0 storage size, remove storage request value and explicitly disabled `persistentVolumeEnabled`
+			if v, _ := q.AsInt64(); v == 0 {
+				delete(item.Resources.Requests, corev1.ResourceStorage)
+				b := false
+				item.PVEnabled = &b
+				return
+			}
+			item.Resources.Requests[corev1.ResourceStorage] = q
+			// explicitly enabled `persistentVolumeEnabled` if provided storage size settings
+			b := true
+			item.PVEnabled = &b
 		}); err != nil {
 			return err
 		}