Merge pull request #135400 from neolit123/1.36-dont-sort-extra-args

kubeadm: do not sort extraArgs alpha-numerically

cmd/kubeadm/app/apis/kubeadm/types.go

@@ -167,6 +167,7 @@ type ControlPlaneComponent struct {
 	// An argument name in this list is the flag name as it appears on the
 	// command line except without leading dash(es). Extra arguments will override existing
 	// default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	ExtraArgs []Arg
 
 	// ExtraVolumes is an extra set of host volumes, mounted to the control plane component.
@@ -249,6 +250,7 @@ type NodeRegistrationOptions struct {
 	// Flags have higher priority when parsing. These values are local and specific to the node kubeadm is executing on.
 	// An argument name in this list is the flag name as it appears on the command line except without leading dash(es).
 	// Extra arguments will override existing default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	KubeletExtraArgs []Arg
 
 	// IgnorePreflightErrors provides a slice of pre-flight errors to be ignored when the current node is registered, e.g. 'IsPrivilegedUser,Swap'.
@@ -301,6 +303,7 @@ type LocalEtcd struct {
 	// An argument name in this list is the flag name as it appears on the
 	// command line except without leading dash(es). Extra arguments will override existing
 	// default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	ExtraArgs []Arg
 
 	// ExtraEnvs is an extra set of environment variables to pass to the control plane component.

cmd/kubeadm/app/apis/kubeadm/v1beta4/types.go

@@ -172,6 +172,7 @@ type ControlPlaneComponent struct {
 	// An argument name in this list is the flag name as it appears on the
 	// command line except without leading dash(es). Extra arguments will override existing
 	// default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	// +optional
 	ExtraArgs []Arg `json:"extraArgs,omitempty"`
 
@@ -262,6 +263,7 @@ type NodeRegistrationOptions struct {
 	// Flags have higher priority when parsing. These values are local and specific to the node kubeadm is executing on.
 	// An argument name in this list is the flag name as it appears on the command line except without leading dash(es).
 	// Extra arguments will override existing default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	// +optional
 	KubeletExtraArgs []Arg `json:"kubeletExtraArgs,omitempty"`
 
@@ -324,6 +326,7 @@ type LocalEtcd struct {
 	// An argument name in this list is the flag name as it appears on the
 	// command line except without leading dash(es). Extra arguments will override existing
 	// default arguments. Duplicate extra arguments are allowed.
+	// The default arguments are sorted alpha-numerically but the extra arguments are not.
 	// +optional
 	ExtraArgs []Arg `json:"extraArgs,omitempty"`
 

cmd/kubeadm/app/util/arguments.go

@@ -29,37 +29,34 @@ import (
 )
 
 // ArgumentsToCommand takes two Arg slices, one with the base arguments and one
-// with optional override arguments. In the return list override arguments will precede base
-// arguments. If an argument is present in the overrides, it will cause
+// with optional override arguments. In the return list, base arguments will precede
+// override arguments. If an argument is present in the overrides, it will cause
 // all instances of the same argument in the base list to be discarded, leaving
 // only the instances of this argument in the overrides to be applied.
-func ArgumentsToCommand(base []kubeadmapi.Arg, overrides []kubeadmapi.Arg) []string {
-	var command []string
-	// Copy the overrides arguments into a new slice.
-	args := make([]kubeadmapi.Arg, len(overrides))
-	copy(args, overrides)
+func ArgumentsToCommand(base, overrides []kubeadmapi.Arg) []string {
+	// Sort only the base.
+	sortArgsSlice(&base)
 
-	// overrideArgs is a set of args which will replace the args defined in the base
+	// Collect all overrides in a set.
 	overrideArgs := sets.New[string]()
 	for _, arg := range overrides {
 		overrideArgs.Insert(arg.Name)
 	}
 
+	// Append only the base args that do not have overrides.
+	args := make([]kubeadmapi.Arg, 0, len(base)+len(overrides))
 	for _, arg := range base {
 		if !overrideArgs.Has(arg.Name) {
 			args = append(args, arg)
 		}
 	}
 
-	sort.Slice(args, func(i, j int) bool {
-		if args[i].Name == args[j].Name {
-			return args[i].Value < args[j].Value
-		}
-		return args[i].Name < args[j].Name
-	})
+	// Append the overrides.
+	args = append(args, overrides...)
 
-	for _, arg := range args {
-		command = append(command, fmt.Sprintf("--%s=%s", arg.Name, arg.Value))
+	command := make([]string, len(args))
+	for i, arg := range args {
+		command[i] = fmt.Sprintf("--%s=%s", arg.Name, arg.Value)
 	}
 
 	return command
@@ -85,12 +82,8 @@ func ArgumentsFromCommand(command []string) []kubeadmapi.Arg {
 		args = append(args, kubeadmapi.Arg{Name: key, Value: val})
 	}
 
-	sort.Slice(args, func(i, j int) bool {
-		if args[i].Name == args[j].Name {
-			return args[i].Value < args[j].Value
-		}
-		return args[i].Name < args[j].Name
-	})
+	sortArgsSlice(&args)
+
 	return args
 }
 
@@ -117,3 +110,14 @@ func parseArgument(arg string) (string, string, error) {
 
 	return keyvalSlice[0], keyvalSlice[1], nil
 }
+
+// sortArgsSlice sorts a slice of Args alpha-numerically.
+func sortArgsSlice(argsPtr *[]kubeadmapi.Arg) {
+	args := *argsPtr
+	sort.Slice(args, func(i, j int) bool {
+		if args[i].Name == args[j].Name {
+			return args[i].Value < args[j].Value
+		}
+		return args[i].Name < args[j].Name
+	})
+}

cmd/kubeadm/app/util/arguments_test.go

@@ -41,8 +41,8 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"},
 			},
 			expected: []string{
-				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--allow-privileged=true",
+				"--admission-control=NamespaceLifecycle,LimitRanger",
 			},
 		},
 		{
@@ -56,9 +56,9 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "tls-sni-cert-key", Value: "/some/new/path/subpath"},
 			},
 			expected: []string{
+				"--token-auth-file=/token",
 				"--tls-sni-cert-key=/some/new/path",
 				"--tls-sni-cert-key=/some/new/path/subpath",
-				"--token-auth-file=/token",
 			},
 		},
 		{
@@ -72,8 +72,8 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "tls-sni-cert-key", Value: "/some/new/path"},
 			},
 			expected: []string{
-				"--tls-sni-cert-key=/some/new/path",
 				"--token-auth-file=/token",
+				"--tls-sni-cert-key=/some/new/path",
 			},
 		},
 		{
@@ -85,8 +85,8 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"},
 			},
 			expected: []string{
-				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--allow-privileged=true",
+				"--admission-control=NamespaceLifecycle,LimitRanger",
 			},
 		},
 		{
@@ -99,9 +99,9 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "admission-control", Value: "NamespaceLifecycle,LimitRanger"},
 			},
 			expected: []string{
-				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--allow-privileged=true",
 				"--something-that-allows-empty-string=",
+				"--admission-control=NamespaceLifecycle,LimitRanger",
 			},
 		},
 		{
@@ -115,11 +115,31 @@ func TestArgumentsToCommand(t *testing.T) {
 				{Name: "something-that-allows-empty-string", Value: ""},
 			},
 			expected: []string{
-				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--allow-privileged=true",
+				"--admission-control=NamespaceLifecycle,LimitRanger",
 				"--something-that-allows-empty-string=",
 			},
 		},
+		{
+			name: "base are sorted and overrides are not",
+			base: []kubeadmapi.Arg{
+				{Name: "b", Value: "true"},
+				{Name: "c", Value: "true"},
+				{Name: "a", Value: "true"},
+			},
+			overrides: []kubeadmapi.Arg{
+				{Name: "e", Value: "true"},
+				{Name: "b", Value: "true"},
+				{Name: "d", Value: "true"},
+			},
+			expected: []string{
+				"--a=true",
+				"--c=true",
+				"--e=true",
+				"--b=true",
+				"--d=true",
+			},
+		},
 	}
 
 	for _, rt := range tests {
@@ -189,6 +209,21 @@ func TestArgumentsFromCommand(t *testing.T) {
 				{Name: "tls-sni-cert-key", Value: "/some/path/subpath"},
 			},
 		},
+		{
+			name: "args are sorted",
+			args: []string{
+				"--c=foo",
+				"--a=foo",
+				"--b=foo",
+				"--b=bar",
+			},
+			expected: []kubeadmapi.Arg{
+				{Name: "a", Value: "foo"},
+				{Name: "b", Value: "bar"},
+				{Name: "b", Value: "foo"},
+				{Name: "c", Value: "foo"},
+			},
+		},
 	}
 
 	for _, rt := range tests {