Kubelet flags take precedence over config from files/ConfigMaps

Changes the Kubelet configuration flag precedence order so that flags
take precedence over config from files/ConfigMaps.

See issue #56171 for more details.

Also modifies e2e node test suite to transform all relevant Kubelet
flags into a config file before starting tests when the
KubeletConfigFile feature gate is true, and turns on the
KubeletConfigFile gate for all e2e node tests. This allows the alpha
dynamic Kubelet config feature to continue to work in tests after
the precedence change.

cmd/kubelet/kubelet.go

@@ -53,7 +53,7 @@ func main() {
 	}
 	options.AddKubeletConfigFlags(pflag.CommandLine, defaultConfig)
 
-	// parse the command line flags into the respective objects
+	// initialize pflag and parse the initial command line flags into the respective objects
 	flag.InitFlags()
 
 	// initialize logging and defer flush
@@ -80,6 +80,12 @@ func main() {
 		die(err)
 	}
 
+	// re-parse the command-line flags on top of the returned configuration
+	// we layer flags over file-based and remote configuration to
+	// preserve backwards compatibility across binary upgrades
+	// see issue #56171 for more details
+	pflag.Parse()
+
 	// construct a KubeletServer from kubeletFlags and kubeletConfig
 	kubeletServer := &options.KubeletServer{
 		KubeletFlags:         *kubeletFlags,

hack/make-rules/test.sh

@@ -69,6 +69,7 @@ kube::test::find_dirs() {
         -o -path './vendor/k8s.io/client-go/*' \
         -o -path './vendor/k8s.io/apiserver/*' \
         -o -path './test/e2e_node/system/*' \
+        -o -path './test/e2e_node/services/*' \
       -name '*_test.go' -print0 | xargs -0n1 dirname | sed "s|^\./|${KUBE_GO_PACKAGE}/|" | LC_ALL=C sort -u
 
     # run tests for client-go

test/e2e_node/dynamic_kubelet_config_test.go

@@ -25,6 +25,7 @@ import (
 
 	apiv1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig"
 	"k8s.io/kubernetes/pkg/kubelet/kubeletconfig/status"
 
@@ -46,6 +47,8 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 	f := framework.NewDefaultFramework("dynamic-kubelet-configuration-test")
 	var originalKC *kubeletconfig.KubeletConfiguration
 	var originalConfigMap *apiv1.ConfigMap
+	// local messages/reasons, depending on whether we expect the default or init config
+	var curLocalMessage, lkgLocalMessage, curLocalOKReason string
 
 	// Dummy context to prevent framework's AfterEach from cleaning up before this test's AfterEach can run
 	Context("", func() {
@@ -58,6 +61,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 				originalConfigMap, err = f.ClientSet.CoreV1().ConfigMaps("kube-system").Create(originalConfigMap)
 				framework.ExpectNoError(err)
 			}
+
 			// make sure Dynamic Kubelet Configuration feature is enabled on the Kubelet we are about to test
 			enabled, err := isKubeletConfigEnabled(f)
 			framework.ExpectNoError(err)
@@ -66,6 +70,19 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 					"Pass --feature-gates=DynamicKubeletConfig=true to the Kubelet to enable this feature.\n" +
 					"For `make test-e2e-node`, you can set `TEST_ARGS='--feature-gates=DynamicKubeletConfig=true'`."))
 			}
+
+			// expect different local messages/reasons depending on how Kuebelet is configured
+			if v, ok := originalKC.FeatureGates[string(features.KubeletConfigFile)]; !ok || !v {
+				// KubeletConfigFile key not found or set to false. It's still an alpha feature, so it is turned off by default.
+				curLocalMessage = status.CurDefaultMessage
+				lkgLocalMessage = status.LkgDefaultMessage
+				curLocalOKReason = status.CurDefaultOKReason
+			} else {
+				// KubeletConfigFile key was found and set to true.
+				curLocalMessage = status.CurInitMessage
+				lkgLocalMessage = status.LkgInitMessage
+				curLocalOKReason = status.CurInitOKReason
+			}
 		})
 
 		AfterEach(func() {
@@ -119,8 +136,8 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 					{desc: "Node.Spec.ConfigSource is nil",
 						configSource: nil,
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionTrue,
-							Message: status.CurDefaultMessage,
-							Reason:  status.CurDefaultOKReason},
+							Message: curLocalMessage,
+							Reason:  curLocalOKReason},
 						expectConfig: nil},
 
 					// Node.Spec.ConfigSource has all nil subfields
@@ -170,7 +187,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Namespace: failParseConfigMap.Namespace,
 							Name:      failParseConfigMap.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionFalse,
-							Message: status.LkgDefaultMessage,
+							Message: lkgLocalMessage,
 							Reason:  fmt.Sprintf(status.CurFailParseReasonFmt, failParseConfigMap.UID)},
 						expectConfig: nil},
 
@@ -181,7 +198,7 @@ var _ = framework.KubeDescribe("DynamicKubeletConfiguration [Feature:DynamicKube
 							Namespace: failValidateConfigMap.Namespace,
 							Name:      failValidateConfigMap.Name}},
 						expectConfigOK: &apiv1.NodeCondition{Type: apiv1.NodeConfigOK, Status: apiv1.ConditionFalse,
-							Message: status.LkgDefaultMessage,
+							Message: lkgLocalMessage,
 							Reason:  fmt.Sprintf(status.CurFailValidateReasonFmt, failValidateConfigMap.UID)},
 						expectConfig: nil},
 				}

test/e2e_node/jenkins/jenkins-ci-ubuntu.properties

@@ -6,6 +6,7 @@ GCE_ZONE=us-central1-f
 GCE_PROJECT=k8s-jkns-ubuntu-node
 CLEANUP=true
 GINKGO_FLAGS='--skip="\[Flaky\]|\[Serial\]"'
+TEST_ARGS='--feature-gates=KubeletConfigFile=true'
 KUBELET_ARGS='--cgroups-per-qos=true --cgroup-root=/'
 TIMEOUT=1h
 # Use the system spec defined in test/e2e_node/system/specs/gke.yaml.

test/e2e_node/jenkins/jenkins-ci.properties

@@ -4,5 +4,6 @@ GCE_ZONE=us-central1-f
 GCE_PROJECT=k8s-jkns-ci-node-e2e
 CLEANUP=true
 GINKGO_FLAGS='--skip="\[Flaky\]|\[Serial\]"'
+TEST_ARGS='--feature-gates=KubeletConfigFile=true'
 KUBELET_ARGS='--cgroups-per-qos=true --cgroup-root=/'
 TIMEOUT=1h

test/e2e_node/jenkins/jenkins-flaky.properties

@@ -4,7 +4,7 @@ GCE_ZONE=us-central1-f
 GCE_PROJECT=k8s-jkns-ci-node-e2e
 CLEANUP=true
 GINKGO_FLAGS='--focus="\[Flaky\]"'
-TEST_ARGS='--feature-gates=DynamicKubeletConfig=true,LocalStorageCapacityIsolation=true,PodPriority=true'
+TEST_ARGS='--feature-gates=KubeletConfigFile=true,DynamicKubeletConfig=true,LocalStorageCapacityIsolation=true,PodPriority=true'
 KUBELET_ARGS='--cgroups-per-qos=true --cgroup-root=/'
 PARALLELISM=1
 TIMEOUT=3h

test/e2e_node/jenkins/jenkins-pull.properties

@@ -4,5 +4,5 @@ GCE_ZONE=us-central1-f
 GCE_PROJECT=k8s-jkns-pr-node-e2e
 CLEANUP=true
 GINKGO_FLAGS='--skip="\[Flaky\]|\[Slow\]|\[Serial\]" --flakeAttempts=2'
+TEST_ARGS='--feature-gates=KubeletConfigFile=true'
 KUBELET_ARGS='--cgroups-per-qos=true --cgroup-root=/'
-

test/e2e_node/jenkins/jenkins-serial-ubuntu.properties

@@ -6,7 +6,7 @@ GCE_ZONE=us-central1-f
 GCE_PROJECT=k8s-jkns-ubuntu-node-serial
 CLEANUP=true
 GINKGO_FLAGS='--focus="\[Serial\]" --skip="\[Flaky\]|\[Benchmark\]"'
-TEST_ARGS='--feature-gates=DynamicKubeletConfig=true'
+TEST_ARGS='--feature-gates=KubeletConfigFile=true,DynamicKubeletConfig=true'
 KUBELET_ARGS='--cgroups-per-qos=true --cgroup-root=/'
 PARALLELISM=1
 TIMEOUT=3h

test/e2e_node/jenkins/jenkins-serial.properties

@@ -4,7 +4,7 @@ GCE_ZONE=us-west1-b
 GCE_PROJECT=k8s-jkns-ci-node-e2e
 CLEANUP=true
 GINKGO_FLAGS='--focus="\[Serial\]" --skip="\[Flaky\]|\[Benchmark\]"'
-TEST_ARGS='--feature-gates=DynamicKubeletConfig=true'
+TEST_ARGS='--feature-gates=KubeletConfigFile=true,DynamicKubeletConfig=true'
 KUBELET_ARGS='--cgroups-per-qos=true --cgroup-root=/'
 PARALLELISM=1
 TIMEOUT=3h

test/e2e_node/services/BUILD

@@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
+    "go_test",
 )
 
 go_library(
@@ -22,9 +23,13 @@ go_library(
     deps = [
         "//cmd/kube-apiserver/app:go_default_library",
         "//cmd/kube-apiserver/app/options:go_default_library",
+        "//cmd/kubelet/app/options:go_default_library",
         "//pkg/api/legacyscheme:go_default_library",
         "//pkg/controller/namespace:go_default_library",
         "//pkg/features:go_default_library",
+        "//pkg/kubelet/apis/kubeletconfig:go_default_library",
+        "//pkg/kubelet/apis/kubeletconfig/scheme:go_default_library",
+        "//pkg/kubelet/apis/kubeletconfig/v1alpha1:go_default_library",
         "//test/e2e/framework:go_default_library",
         "//test/e2e_node/builder:go_default_library",
         "//vendor/github.com/coreos/etcd/etcdserver:go_default_library",
@@ -33,7 +38,9 @@ go_library(
         "//vendor/github.com/coreos/etcd/pkg/types:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/github.com/kardianos/osext:go_default_library",
+        "//vendor/github.com/spf13/pflag:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//vendor/k8s.io/client-go/dynamic:go_default_library",
         "//vendor/k8s.io/client-go/informers:go_default_library",
@@ -54,3 +61,11 @@ filegroup(
     srcs = [":package-srcs"],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["kubelet_test.go"],
+    importpath = "k8s.io/kubernetes/test/e2e_node/services",
+    library = ":go_default_library",
+    deps = ["//vendor/github.com/spf13/pflag:go_default_library"],
+)

test/e2e_node/services/kubelet.go

@@ -27,9 +27,15 @@ import (
 	"strings"
 
 	"github.com/golang/glog"
+	"github.com/spf13/pflag"
 
+	"k8s.io/apimachinery/pkg/runtime"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/kubernetes/cmd/kubelet/app/options"
 	"k8s.io/kubernetes/pkg/features"
+	"k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig"
+	"k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/scheme"
+	"k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/v1alpha1"
 	"k8s.io/kubernetes/test/e2e/framework"
 	"k8s.io/kubernetes/test/e2e_node/builder"
 )
@@ -118,6 +124,7 @@ func (e *E2EServices) startKubelet() (*server, error) {
 	var isSystemd bool
 	// Apply default kubelet flags.
 	cmdArgs := []string{}
+	kubeArgs := []string{}
 	if systemdRun, err := exec.LookPath("systemd-run"); err == nil {
 		// On systemd services, detection of a service / unit works reliably while
 		// detection of a process started from an ssh session does not work.
@@ -132,13 +139,13 @@ func (e *E2EServices) startKubelet() (*server, error) {
 			Name:              "kubelet.log",
 			JournalctlCommand: []string{"-u", unitName},
 		}
-		cmdArgs = append(cmdArgs,
+		kubeArgs = append(kubeArgs,
 			"--kubelet-cgroups=/kubelet.slice",
 			"--cgroup-root=/",
 		)
 	} else {
 		cmdArgs = append(cmdArgs, builder.GetKubeletServerBin())
-		cmdArgs = append(cmdArgs,
+		kubeArgs = append(kubeArgs,
 			// TODO(random-liu): Get rid of this docker specific thing.
 			"--runtime-cgroups=/docker-daemon",
 			"--kubelet-cgroups=/kubelet",
@@ -146,7 +153,7 @@ func (e *E2EServices) startKubelet() (*server, error) {
 			"--system-cgroups=/system",
 		)
 	}
-	cmdArgs = append(cmdArgs,
+	kubeArgs = append(kubeArgs,
 		"--kubeconfig", kubeconfigPath,
 		"--address", "0.0.0.0",
 		"--port", kubeletPort,
@@ -175,11 +182,11 @@ func (e *E2EServices) startKubelet() (*server, error) {
 
 	if utilfeature.DefaultFeatureGate.Enabled(features.DynamicKubeletConfig) {
 		// Enable dynamic config if the feature gate is enabled
-		dynamicConfigDir, err := getDynamicConfigDir()
+		dir, err := getDynamicConfigDirectory()
 		if err != nil {
 			return nil, err
 		}
-		cmdArgs = append(cmdArgs, "--dynamic-config-dir", dynamicConfigDir)
+		kubeArgs = append(kubeArgs, "--dynamic-config-dir", dir)
 	}
 
 	// Enable kubenet by default.
@@ -193,18 +200,40 @@ func (e *E2EServices) startKubelet() (*server, error) {
 		return nil, err
 	}
 
-	cmdArgs = append(cmdArgs,
+	kubeArgs = append(kubeArgs,
 		"--network-plugin=kubenet",
 		"--cni-bin-dir", cniBinDir,
 		"--cni-conf-dir", cniConfDir)
 
 	// Keep hostname override for convenience.
 	if framework.TestContext.NodeName != "" { // If node name is specified, set hostname override.
-		cmdArgs = append(cmdArgs, "--hostname-override", framework.TestContext.NodeName)
+		kubeArgs = append(kubeArgs, "--hostname-override", framework.TestContext.NodeName)
 	}
 
 	// Override the default kubelet flags.
-	cmdArgs = append(cmdArgs, kubeletArgs...)
+	kubeArgs = append(kubeArgs, kubeletArgs...)
+
+	// If the config file feature gate is enabled, generate the file and remove the flags it applies to
+	if utilfeature.DefaultFeatureGate.Enabled(features.KubeletConfigFile) {
+		kc, other, err := splitKubeletConfigArgs(kubeArgs)
+		if err != nil {
+			return nil, err
+		}
+		// replace kubeArgs with the new command line, which has had the KubeletConfiguration flags removed
+		kubeArgs = other
+		path, err := writeKubeletConfigFile(kc)
+		if err != nil {
+			return nil, err
+		}
+		// ensure the test context feature gates (typically DynamicKubeletConfig and KubeletConfigFile)
+		// are set on the command line
+		kubeArgs = append(kubeArgs, "--feature-gates", framework.TestContext.FeatureGates)
+		// add the flag to load config from a file
+		kubeArgs = append(kubeArgs, "--init-config-dir", filepath.Dir(path))
+	}
+
+	// combine the kubelet parameters with the command
+	cmdArgs = append(cmdArgs, kubeArgs...)
 
 	// Adjust the args if we are running kubelet with systemd.
 	if isSystemd {
@@ -224,6 +253,94 @@ func (e *E2EServices) startKubelet() (*server, error) {
 	return server, server.start()
 }
 
+// splitKubeletConfigArgs parses args onto a KubeletConfiguration object and also returns the unknown args
+func splitKubeletConfigArgs(args []string) (*kubeletconfig.KubeletConfiguration, []string, error) {
+	kc, err := options.NewKubeletConfiguration()
+	if err != nil {
+		return nil, nil, err
+	}
+	fs := pflag.NewFlagSet("kubeletconfig", pflag.ContinueOnError)
+	options.AddKubeletConfigFlags(fs, kc)
+	known, other := splitKnownArgs(fs, args)
+	if err := fs.Parse(known); err != nil {
+		return nil, nil, err
+	}
+	return kc, other, nil
+}
+
+// splitKnownArgs splits argument list into those known by the flagset, and those not known
+// only tests for longhand args, e.g. prefixed with `--`
+// TODO(mtaufen): I don't think the kubelet has any shorthand args, but if it does we will need to modify this.
+func splitKnownArgs(fs *pflag.FlagSet, args []string) ([]string, []string) {
+	known := []string{}
+	other := []string{}
+	lastFlag := len(args)
+	for i := len(args) - 1; i >= 0; i-- {
+		if strings.HasPrefix(args[i], "--") {
+			if fs.Lookup(strings.TrimPrefix(args[i], "--")) == nil {
+				// flag is unknown, add flag and params to other
+				// prepend to maintain order
+				other = append(append([]string(nil), args[i:lastFlag]...), other...)
+				// cut from known
+			} else {
+				// flag is known, add flag and params to known
+				// prepend to maintain order
+				known = append(append([]string(nil), args[i:lastFlag]...), known...)
+			}
+			// mark the last location where we saw a flag
+			lastFlag = i
+		}
+	}
+	return known, other
+}
+
+// writeKubeletConfigFile writes the kubelet config file based on the args and returns the filename
+func writeKubeletConfigFile(internal *kubeletconfig.KubeletConfiguration) (string, error) {
+	// extract the KubeletConfiguration and convert to versioned
+	versioned := &v1alpha1.KubeletConfiguration{}
+	scheme, _, err := scheme.NewSchemeAndCodecs()
+	if err != nil {
+		return "", err
+	}
+	if err := scheme.Convert(internal, versioned, nil); err != nil {
+		return "", err
+	}
+	// encode
+	encoder, err := newKubeletConfigJSONEncoder()
+	if err != nil {
+		return "", err
+	}
+	data, err := runtime.Encode(encoder, versioned)
+	if err != nil {
+		return "", err
+	}
+	// create the init conifg directory
+	dir, err := createKubeletInitConfigDirectory()
+	if err != nil {
+		return "", err
+	}
+	// write init config file
+	path := filepath.Join(dir, "kubelet")
+	if err := ioutil.WriteFile(path, data, 0755); err != nil {
+		return "", err
+	}
+	return path, nil
+}
+
+func newKubeletConfigJSONEncoder() (runtime.Encoder, error) {
+	_, kubeletCodecs, err := scheme.NewSchemeAndCodecs()
+	if err != nil {
+		return nil, err
+	}
+
+	mediaType := "application/json"
+	info, ok := runtime.SerializerInfoForMediaType(kubeletCodecs.SupportedMediaTypes(), mediaType)
+	if !ok {
+		return nil, fmt.Errorf("unsupported media type %q", mediaType)
+	}
+	return kubeletCodecs.EncoderForVersion(info.Serializer, v1alpha1.SchemeGroupVersion), nil
+}
+
 // createPodManifestDirectory creates pod manifest directory.
 func createPodManifestDirectory() (string, error) {
 	cwd, err := os.Getwd()
@@ -313,7 +430,7 @@ func getCNIConfDirectory() (string, error) {
 }
 
 // getDynamicConfigDir returns the directory for dynamic Kubelet configuration
-func getDynamicConfigDir() (string, error) {
+func getDynamicConfigDirectory() (string, error) {
 	cwd, err := os.Getwd()
 	if err != nil {
 		return "", err
@@ -321,6 +438,19 @@ func getDynamicConfigDir() (string, error) {
 	return filepath.Join(cwd, "dynamic-kubelet-config"), nil
 }
 
+// createKubeletInitConfigDirectory creates and returns the name of the directory for dynamic Kubelet configuration
+func createKubeletInitConfigDirectory() (string, error) {
+	cwd, err := os.Getwd()
+	if err != nil {
+		return "", err
+	}
+	path := filepath.Join(cwd, "init-kubelet-config")
+	if err := os.MkdirAll(path, 0755); err != nil {
+		return "", err
+	}
+	return path, nil
+}
+
 // adjustArgsForSystemd escape special characters in kubelet arguments for systemd. Systemd
 // may try to do auto expansion without escaping.
 func adjustArgsForSystemd(args []string) {

test/e2e_node/services/kubelet_test.go

@@ -0,0 +1,146 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package services
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/spf13/pflag"
+)
+
+func TestSplitKnownArgs(t *testing.T) {
+	cases := []struct {
+		desc        string
+		args        []string
+		fs          *pflag.FlagSet
+		expectKnown []string
+		expectOther []string
+	}{
+		{
+			"splits three args:a",
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+			func() *pflag.FlagSet {
+				fs := pflag.NewFlagSet("", pflag.ContinueOnError)
+				var a string
+				fs.StringVar(&a, "a", a, "")
+				return fs
+			}(),
+			[]string{"--a", "a"},
+			[]string{"--b", "b", "--c", "c"},
+		},
+		{
+			"splits three args:b",
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+			func() *pflag.FlagSet {
+				fs := pflag.NewFlagSet("", pflag.ContinueOnError)
+				var a string
+				fs.StringVar(&a, "b", a, "")
+				return fs
+			}(),
+			[]string{"--b", "b"},
+			[]string{"--a", "a", "--c", "c"},
+		},
+		{
+			"splits three args:c",
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+			func() *pflag.FlagSet {
+				fs := pflag.NewFlagSet("", pflag.ContinueOnError)
+				var a string
+				fs.StringVar(&a, "c", a, "")
+				return fs
+			}(),
+			[]string{"--c", "c"},
+			[]string{"--a", "a", "--b", "b"},
+		},
+		{
+			"splits three args:ab",
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+			func() *pflag.FlagSet {
+				fs := pflag.NewFlagSet("", pflag.ContinueOnError)
+				var a string
+				fs.StringVar(&a, "a", a, "")
+				fs.StringVar(&a, "b", a, "")
+				return fs
+			}(),
+			[]string{"--a", "a", "--b", "b"},
+			[]string{"--c", "c"},
+		},
+		{
+			"splits three args:bc",
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+			func() *pflag.FlagSet {
+				fs := pflag.NewFlagSet("", pflag.ContinueOnError)
+				var a string
+				fs.StringVar(&a, "b", a, "")
+				fs.StringVar(&a, "c", a, "")
+				return fs
+			}(),
+			[]string{"--b", "b", "--c", "c"},
+			[]string{"--a", "a"},
+		},
+		{
+			"splits three args:ac",
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+			func() *pflag.FlagSet {
+				fs := pflag.NewFlagSet("", pflag.ContinueOnError)
+				var a string
+				fs.StringVar(&a, "a", a, "")
+				fs.StringVar(&a, "c", a, "")
+				return fs
+			}(),
+			[]string{"--a", "a", "--c", "c"},
+			[]string{"--b", "b"},
+		},
+		{
+			"splits three args:abc",
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+			func() *pflag.FlagSet {
+				fs := pflag.NewFlagSet("", pflag.ContinueOnError)
+				var a string
+				fs.StringVar(&a, "a", a, "")
+				fs.StringVar(&a, "b", a, "")
+				fs.StringVar(&a, "c", a, "")
+				return fs
+			}(),
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+			[]string{},
+		},
+		{
+			"splits three args:none",
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+			pflag.NewFlagSet("", pflag.ContinueOnError),
+			[]string{},
+			[]string{"--a", "a", "--b", "b", "--c", "c"},
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			origArgs := append([]string(nil), c.args...)
+			known, other := splitKnownArgs(c.fs, c.args)
+			if !reflect.DeepEqual(c.expectKnown, known) {
+				t.Errorf("expect known args to be %v, got %v", c.expectKnown, known)
+			}
+			if !reflect.DeepEqual(c.expectOther, other) {
+				t.Errorf("expect other args to be %v, got %v", c.expectOther, other)
+			}
+			if !reflect.DeepEqual(origArgs, c.args) {
+				t.Errorf("args was mutated")
+			}
+		})
+	}
+}