From cbebb6145000676fdf9a58c19ca1f180f0b80f8b Mon Sep 17 00:00:00 2001 From: Michael Taufen Date: Wed, 15 Nov 2017 10:17:06 -0800 Subject: [PATCH] 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 | 8 +- hack/make-rules/test.sh | 1 + test/e2e_node/dynamic_kubelet_config_test.go | 25 ++- .../jenkins/jenkins-ci-ubuntu.properties | 1 + test/e2e_node/jenkins/jenkins-ci.properties | 1 + .../e2e_node/jenkins/jenkins-flaky.properties | 2 +- test/e2e_node/jenkins/jenkins-pull.properties | 2 +- .../jenkins/jenkins-serial-ubuntu.properties | 2 +- .../jenkins/jenkins-serial.properties | 2 +- test/e2e_node/services/BUILD | 15 ++ test/e2e_node/services/kubelet.go | 148 ++++++++++++++++-- test/e2e_node/services/kubelet_test.go | 146 +++++++++++++++++ 12 files changed, 335 insertions(+), 18 deletions(-) create mode 100644 test/e2e_node/services/kubelet_test.go diff --git a/cmd/kubelet/kubelet.go b/cmd/kubelet/kubelet.go index 33ed06f3f81..0075801a06b 100644 --- a/cmd/kubelet/kubelet.go +++ b/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, diff --git a/hack/make-rules/test.sh b/hack/make-rules/test.sh index 54feeea8ea8..adf0697f972 100755 --- a/hack/make-rules/test.sh +++ b/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 diff --git a/test/e2e_node/dynamic_kubelet_config_test.go b/test/e2e_node/dynamic_kubelet_config_test.go index dd83b3d1bc1..cfa40d03fc7 100644 --- a/test/e2e_node/dynamic_kubelet_config_test.go +++ b/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}, } diff --git a/test/e2e_node/jenkins/jenkins-ci-ubuntu.properties b/test/e2e_node/jenkins/jenkins-ci-ubuntu.properties index 1e0c76fa14d..9a60bfbeedd 100644 --- a/test/e2e_node/jenkins/jenkins-ci-ubuntu.properties +++ b/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. diff --git a/test/e2e_node/jenkins/jenkins-ci.properties b/test/e2e_node/jenkins/jenkins-ci.properties index 9c563b6a050..4f9cb307457 100644 --- a/test/e2e_node/jenkins/jenkins-ci.properties +++ b/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 diff --git a/test/e2e_node/jenkins/jenkins-flaky.properties b/test/e2e_node/jenkins/jenkins-flaky.properties index 824c1309dcf..196e532393e 100644 --- a/test/e2e_node/jenkins/jenkins-flaky.properties +++ b/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 diff --git a/test/e2e_node/jenkins/jenkins-pull.properties b/test/e2e_node/jenkins/jenkins-pull.properties index 884e45884f1..d3f00084b1e 100644 --- a/test/e2e_node/jenkins/jenkins-pull.properties +++ b/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=/' - diff --git a/test/e2e_node/jenkins/jenkins-serial-ubuntu.properties b/test/e2e_node/jenkins/jenkins-serial-ubuntu.properties index 5333bb8b037..2790f47c1ae 100644 --- a/test/e2e_node/jenkins/jenkins-serial-ubuntu.properties +++ b/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 diff --git a/test/e2e_node/jenkins/jenkins-serial.properties b/test/e2e_node/jenkins/jenkins-serial.properties index 31bded6deb2..c59d502fa2a 100644 --- a/test/e2e_node/jenkins/jenkins-serial.properties +++ b/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 diff --git a/test/e2e_node/services/BUILD b/test/e2e_node/services/BUILD index b0852a5828a..4aef23a78a9 100644 --- a/test/e2e_node/services/BUILD +++ b/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"], +) diff --git a/test/e2e_node/services/kubelet.go b/test/e2e_node/services/kubelet.go index 0e655349075..05d39bde56c 100644 --- a/test/e2e_node/services/kubelet.go +++ b/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) { diff --git a/test/e2e_node/services/kubelet_test.go b/test/e2e_node/services/kubelet_test.go new file mode 100644 index 00000000000..164640867ac --- /dev/null +++ b/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") + } + }) + } +}