From fc19b805d58a0038223eacfd3b4a059569637fd7 Mon Sep 17 00:00:00 2001 From: Jamie Phillips Date: Wed, 28 Jul 2021 16:04:19 -0400 Subject: [PATCH] Added logic to strip any existing hyphens before processing the args. (#3662) Updated the logic to handle if extra args are passed with existing hyphens in the arg. The test was updated to add the additional case of having pre-existing hyphens. The method name was also refactored based on previous feedback. --- pkg/agent/run.go | 2 +- pkg/daemons/agent/agent.go | 4 ++-- pkg/daemons/config/types.go | 9 +++++--- pkg/daemons/config/types_test.go | 38 +++++++++++++++++++++++++++++--- pkg/daemons/control/server.go | 8 +++---- 5 files changed, 48 insertions(+), 13 deletions(-) diff --git a/pkg/agent/run.go b/pkg/agent/run.go index f16a1cc30d..554555a5ed 100644 --- a/pkg/agent/run.go +++ b/pkg/agent/run.go @@ -139,7 +139,7 @@ func getConntrackConfig(nodeConfig *daemonconfig.Node) (*kubeproxyconfig.KubePro } cmd := app2.NewProxyCommand() - if err := cmd.ParseFlags(daemonconfig.GetArgsList(map[string]string{}, nodeConfig.AgentConfig.ExtraKubeProxyArgs)); err != nil { + if err := cmd.ParseFlags(daemonconfig.GetArgs(map[string]string{}, nodeConfig.AgentConfig.ExtraKubeProxyArgs)); err != nil { return nil, err } maxPerCore, err := cmd.Flags().GetInt32("conntrack-max-per-core") diff --git a/pkg/daemons/agent/agent.go b/pkg/daemons/agent/agent.go index dc8097cfbd..ed52999907 100644 --- a/pkg/daemons/agent/agent.go +++ b/pkg/daemons/agent/agent.go @@ -43,7 +43,7 @@ func Agent(ctx context.Context, nodeConfig *daemonconfig.Node, proxy proxy.Proxy func startKubeProxy(cfg *daemonconfig.Agent) error { argsMap := kubeProxyArgs(cfg) - args := daemonconfig.GetArgsList(argsMap, cfg.ExtraKubeProxyArgs) + args := daemonconfig.GetArgs(argsMap, cfg.ExtraKubeProxyArgs) logrus.Infof("Running kube-proxy %s", daemonconfig.ArgString(args)) return executor.KubeProxy(args) } @@ -51,7 +51,7 @@ func startKubeProxy(cfg *daemonconfig.Agent) error { func startKubelet(cfg *daemonconfig.Agent) error { argsMap := kubeletArgs(cfg) - args := daemonconfig.GetArgsList(argsMap, cfg.ExtraKubeletArgs) + args := daemonconfig.GetArgs(argsMap, cfg.ExtraKubeletArgs) logrus.Infof("Running kubelet %s", daemonconfig.ArgString(args)) return executor.Kubelet(args) diff --git a/pkg/daemons/config/types.go b/pkg/daemons/config/types.go index c4c1cc5ebe..71db71cc0e 100644 --- a/pkg/daemons/config/types.go +++ b/pkg/daemons/config/types.go @@ -257,10 +257,13 @@ func (a ArgString) String() string { return b.String() } -func GetArgsList(argsMap map[string]string, extraArgs []string) []string { +// GetArgs appends extra arguments to existing arguments overriding any default options. +func GetArgs(argsMap map[string]string, extraArgs []string) []string { + const hyphens = "--" + // add extra args to args map to override any default option for _, arg := range extraArgs { - splitArg := strings.SplitN(arg, "=", 2) + splitArg := strings.SplitN(strings.TrimPrefix(arg, hyphens), "=", 2) if len(splitArg) < 2 { argsMap[splitArg[0]] = "true" continue @@ -269,7 +272,7 @@ func GetArgsList(argsMap map[string]string, extraArgs []string) []string { } var args []string for arg, value := range argsMap { - cmd := fmt.Sprintf("--%s=%s", arg, value) + cmd := fmt.Sprintf("%s%s=%s", hyphens, strings.TrimPrefix(arg, hyphens), value) args = append(args, cmd) } sort.Strings(args) diff --git a/pkg/daemons/config/types_test.go b/pkg/daemons/config/types_test.go index 1b3ce9d1d9..f94d6093b7 100644 --- a/pkg/daemons/config/types_test.go +++ b/pkg/daemons/config/types_test.go @@ -5,7 +5,7 @@ import ( "testing" ) -func Test_UnitGetArgsList(t *testing.T) { +func Test_UnitGetArgs(t *testing.T) { type args struct { argsMap map[string]string extraArgs []string @@ -35,6 +35,38 @@ func Test_UnitGetArgsList(t *testing.T) { }, }, + want: []string{ + "--aaa=A", + "--bbb=BB", + "--ccc=C", + "--ddd=DD", + "--eee=e", + "--fff=f", + "--ggg=g", + "--hhh=h", + "--iii=II", + }, + }, + { + name: "Args with existing hyphens Test", + args: args{ + argsMap: map[string]string{ + "aaa": "A", + "bbb": "B", + "ccc": "C", + "ddd": "d", + "eee": "e", + "fff": "f", + "ggg": "g", + "hhh": "h", + }, + extraArgs: []string{ + "--bbb=BB", + "--ddd=DD", + "--iii=II", + }, + }, + want: []string{ "--aaa=A", "--bbb=BB", @@ -50,8 +82,8 @@ func Test_UnitGetArgsList(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := GetArgsList(tt.args.argsMap, tt.args.extraArgs); !reflect.DeepEqual(got, tt.want) { - t.Errorf("GetArgsList() = %+v\nWant = %+v", got, tt.want) + if got := GetArgs(tt.args.argsMap, tt.args.extraArgs); !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetArgs() = %+v\nWant = %+v", got, tt.want) } }) } diff --git a/pkg/daemons/control/server.go b/pkg/daemons/control/server.go index 29844189cf..4a226c32d7 100644 --- a/pkg/daemons/control/server.go +++ b/pkg/daemons/control/server.go @@ -121,7 +121,7 @@ func controllerManager(cfg *config.Control, runtime *config.ControlRuntime) erro argsMap["controllers"] = "*,-service,-route,-cloud-node-lifecycle" } - args := config.GetArgsList(argsMap, cfg.ExtraControllerArgs) + args := config.GetArgs(argsMap, cfg.ExtraControllerArgs) logrus.Infof("Running kube-controller-manager %s", config.ArgString(args)) return executor.ControllerManager(runtime.APIServerReady, args) @@ -139,7 +139,7 @@ func scheduler(cfg *config.Control, runtime *config.ControlRuntime) error { if cfg.NoLeaderElect { argsMap["leader-elect"] = "false" } - args := config.GetArgsList(argsMap, cfg.ExtraSchedulerAPIArgs) + args := config.GetArgs(argsMap, cfg.ExtraSchedulerAPIArgs) logrus.Infof("Running kube-scheduler %s", config.ArgString(args)) return executor.Scheduler(runtime.APIServerReady, args) @@ -192,7 +192,7 @@ func apiServer(ctx context.Context, cfg *config.Control, runtime *config.Control if cfg.EncryptSecrets { argsMap["encryption-provider-config"] = runtime.EncryptionConfig } - args := config.GetArgsList(argsMap, cfg.ExtraAPIArgs) + args := config.GetArgs(argsMap, cfg.ExtraAPIArgs) logrus.Infof("Running kube-apiserver %s", config.ArgString(args)) @@ -302,7 +302,7 @@ func cloudControllerManager(ctx context.Context, cfg *config.Control, runtime *c if cfg.NoLeaderElect { argsMap["leader-elect"] = "false" } - args := config.GetArgsList(argsMap, cfg.ExtraCloudControllerArgs) + args := config.GetArgs(argsMap, cfg.ExtraCloudControllerArgs) logrus.Infof("Running cloud-controller-manager %s", config.ArgString(args))