Deduplicate k0s args in comparison (#1157)

* Deduplicate k0s args in comparison

Signed-off-by: Alexey Makhov <[email protected]>
Signed-off-by: makhov <[email protected]>

* Deduplicate k0s args in comparison

Signed-off-by: Alexey Makhov <[email protected]>
Signed-off-by: makhov <[email protected]>

---------

Signed-off-by: Alexey Makhov <[email protected]>
Signed-off-by: makhov <[email protected]>

Author: makhov

internal/controller/controlplane/helper.go

@@ -292,24 +292,41 @@ func (c *K0sController) hasControllerConfigChanged(bootstrapConfigs map[string]b
 		return false
 	}
 
+	kcpK0sConfigSpecCopy := kcp.Spec.K0sConfigSpec.DeepCopy()
+	bootstrapConfigCopy := bootstrapConfig.DeepCopy()
+	kcpK0sConfigSpecCopy.Args = uniqueArgs(kcpK0sConfigSpecCopy.Args)
+
 	// remove data that should not be taken into account to check if the configuration has changed.
-	normalizeK0sConfigSpec(kcp, bootstrapConfig)
+	normalizeK0sConfigSpec(kcp, bootstrapConfigCopy)
+	bootstrapConfigSpecCopy := bootstrapConfigCopy.Spec.K0sConfigSpec.DeepCopy()
 
 	// k0s config will be reconciled using dynamic config, so leave it out of the comparison
-	bootstrapAPIConfig, _, _ := unstructured.NestedMap(bootstrapConfig.Spec.K0sConfigSpec.K0s.Object, "spec", "api")
-	kcpAPIConfig, _, _ := unstructured.NestedMap(kcp.Spec.K0sConfigSpec.K0s.Object, "spec", "api")
-	bootstrapStorageConfig, _, _ := unstructured.NestedMap(bootstrapConfig.Spec.K0sConfigSpec.K0s.Object, "spec", "storage")
-	kcpStorageConfig, _, _ := unstructured.NestedMap(kcp.Spec.K0sConfigSpec.K0s.Object, "spec", "storage")
+	bootstrapAPIConfig, _, _ := unstructured.NestedMap(bootstrapConfigSpecCopy.K0s.Object, "spec", "api")
+	kcpAPIConfig, _, _ := unstructured.NestedMap(kcpK0sConfigSpecCopy.K0s.Object, "spec", "api")
+	bootstrapStorageConfig, _, _ := unstructured.NestedMap(bootstrapConfigSpecCopy.K0s.Object, "spec", "storage")
+	kcpStorageConfig, _, _ := unstructured.NestedMap(kcpK0sConfigSpecCopy.K0s.Object, "spec", "storage")
+
+	// Handle nil cases consistently - convert nil to empty map for comparison
+	if bootstrapStorageConfig == nil {
+		bootstrapStorageConfig = make(map[string]interface{})
+	}
+	if kcpStorageConfig == nil {
+		kcpStorageConfig = make(map[string]interface{})
+	}
+
 	// Bootstrap controller did set etcd name to the K0sControllerConfig, so we need to compare it with the name set in the K0sControlPlane
-	kcpStorageConfigEtcdWithName, _, _ := unstructured.NestedMap(kcp.Spec.K0sConfigSpec.K0s.Object, "spec", "storage")
+	kcpStorageConfigEtcdWithName, _, _ := unstructured.NestedMap(kcpK0sConfigSpecCopy.K0s.Object, "spec", "storage")
 	if kcpStorageConfigEtcdWithName == nil {
 		kcpStorageConfigEtcdWithName = make(map[string]interface{})
 	}
 	_ = unstructured.SetNestedField(kcpStorageConfigEtcdWithName, machine.Name, "etcd", "extraArgs", "name")
-	bootstrapConfig.Spec.K0sConfigSpec.K0s = kcp.Spec.K0sConfigSpec.K0s
+
+	bootstrapConfigCopy.Spec.K0sConfigSpec.K0s = kcpK0sConfigSpecCopy.K0s
+
 	// leave out the tunneling spec for the bootstrap config
-	bootstrapConfig.Spec.K0sConfigSpec.Tunneling = kcp.Spec.K0sConfigSpec.Tunneling
-	return !reflect.DeepEqual(kcp.Spec.K0sConfigSpec, *bootstrapConfig.Spec.K0sConfigSpec) ||
+	bootstrapConfigCopy.Spec.K0sConfigSpec.Tunneling = kcpK0sConfigSpecCopy.Tunneling
+
+	return !reflect.DeepEqual(kcpK0sConfigSpecCopy, bootstrapConfigCopy.Spec.K0sConfigSpec) ||
 		!reflect.DeepEqual(kcpAPIConfig, bootstrapAPIConfig) ||
 		(!reflect.DeepEqual(kcpStorageConfig, bootstrapStorageConfig) && !reflect.DeepEqual(kcpStorageConfigEtcdWithName, bootstrapStorageConfig))
 }
@@ -587,7 +604,7 @@ func minVersion(machines collections.Machines) (string, error) {
 // when comparing if the k0s configuration has changed.
 // TODO: This method should be replaced with a more robust mechanism to prevent unexpected updates from
 // the bootstrap controller.
-func normalizeK0sConfigSpec(kcp *cpv1beta1.K0sControlPlane, bootstrapConfig bootstrapv1.K0sControllerConfig) {
+func normalizeK0sConfigSpec(kcp *cpv1beta1.K0sControlPlane, bootstrapConfig *bootstrapv1.K0sControllerConfig) {
 	isK0sConfigYAMLSet := false
 	for _, arg := range kcp.Spec.K0sConfigSpec.Args {
 		if arg == "/etc/k0s.yaml" {
@@ -609,12 +626,6 @@ func normalizeK0sConfigSpec(kcp *cpv1beta1.K0sControlPlane, bootstrapConfig boot
 }
 
 func uniqueArgs(args []string) []string {
-	// DO NOT REMOVE THIS CHECK
-	// If the slice is empty, we return the slice as is to avoid any modifications.
-	// In callers, we may compare slices and in some cases it may end up in comparing empty and nil slices.
-	if len(args) == 0 {
-		return args
-	}
 	uniqueArgsSlice := []string{}
 	uniqueArgsMap := make(map[string]struct{})
 	for _, arg := range args {