Merge pull request #43558 from xilabao/fix-kubeadm-args

Automatic merge from submit-queue

kubeadm: If `--config` is set, don't allow any other option as it won't have effect

If use config in kubeadm init, cann't get other values from other arguments. 
`kubeadm init --config=../kubeadm.config --token 447ad3.96cda76e3206fca0 --apiserver-bind-port 6445`

So I think we need to allow get values from command and is prior than cofig file.

cmd/kubeadm/app/apis/kubeadm/validation/BUILD

@@ -15,6 +15,7 @@ go_test(
     tags = ["automanaged"],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
+        "//vendor/github.com/spf13/pflag:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
     ],
 )
@@ -30,6 +31,7 @@ go_library(
         "//pkg/api/validation:go_default_library",
         "//pkg/kubeapiserver/authorizer/modes:go_default_library",
         "//pkg/registry/core/service/ipallocator:go_default_library",
+        "//vendor/github.com/spf13/pflag:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
     ],

cmd/kubeadm/app/apis/kubeadm/validation/validation.go

@@ -24,6 +24,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/spf13/pflag"
+
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
@@ -249,3 +251,10 @@ func ValidateCloudProvider(provider string, fldPath *field.Path) field.ErrorList
 	allErrs = append(allErrs, field.Invalid(fldPath, provider, "cloudprovider not supported"))
 	return allErrs
 }
+
+func ValidateMixedArguments(flag *pflag.FlagSet) error {
+	if flag.Changed("config") && flag.NFlag() != 1 {
+		return fmt.Errorf("can not mix '--config' with other arguments")
+	}
+	return nil
+}

cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go

@@ -19,6 +19,8 @@ package validation
 import (
 	"testing"
 
+	"github.com/spf13/pflag"
+
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 )
@@ -250,3 +252,42 @@ func TestValidateNodeConfiguration(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateMixedArguments(t *testing.T) {
+	var tests = []struct {
+		args     []string
+		expected bool
+	}{
+		{[]string{"--foo=bar"}, true},
+		{[]string{"--config=hello"}, true},
+		{[]string{"--foo=bar", "--config=hello"}, false},
+	}
+
+	var cfgPath string
+	var skipPreFlight bool
+
+	for _, rt := range tests {
+		f := pflag.NewFlagSet("test", pflag.ContinueOnError)
+		if f.Parsed() {
+			t.Error("f.Parse() = true before Parse")
+		}
+		f.String("foo", "", "string value")
+		f.StringVar(&cfgPath, "config", cfgPath, "Path to kubeadm config file")
+		f.BoolVar(
+			&skipPreFlight, "skip-preflight-checks", skipPreFlight,
+			"Skip preflight checks normally run before modifying the system",
+		)
+		if err := f.Parse(rt.args); err != nil {
+			t.Fatal(err)
+		}
+
+		actual := ValidateMixedArguments(f)
+		if (actual == nil) != rt.expected {
+			t.Errorf(
+				"failed ValidateMixedArguments:\n\texpected: %t\n\t  actual: %t",
+				rt.expected,
+				(actual == nil),
+			)
+		}
+	}
+}

cmd/kubeadm/app/cmd/init.go

@@ -84,7 +84,7 @@ func NewCmdInit(out io.Writer) *cobra.Command {
 
 			i, err := NewInit(cfgPath, internalcfg, skipPreFlight, skipTokenPrint)
 			kubeadmutil.CheckErr(err)
-			kubeadmutil.CheckErr(i.Validate())
+			kubeadmutil.CheckErr(i.Validate(cmd))
 			kubeadmutil.CheckErr(i.Run(out))
 		},
 	}
@@ -186,7 +186,10 @@ type Init struct {
 }
 
 // Validate validates configuration passed to "kubeadm init"
-func (i *Init) Validate() error {
+func (i *Init) Validate(cmd *cobra.Command) error {
+	if err := validation.ValidateMixedArguments(cmd.PersistentFlags()); err != nil {
+		return err
+	}
 	return validation.ValidateMasterConfiguration(i.cfg).ToAggregate()
 }
 

cmd/kubeadm/app/cmd/join.go

@@ -97,7 +97,7 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
 
 			j, err := NewJoin(cfgPath, args, internalcfg, skipPreFlight)
 			kubeadmutil.CheckErr(err)
-			kubeadmutil.CheckErr(j.Validate())
+			kubeadmutil.CheckErr(j.Validate(cmd))
 			kubeadmutil.CheckErr(j.Run(out))
 		},
 	}
@@ -161,7 +161,10 @@ func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, s
 	return &Join{cfg: cfg}, nil
 }
 
-func (j *Join) Validate() error {
+func (j *Join) Validate(cmd *cobra.Command) error {
+	if err := validation.ValidateMixedArguments(cmd.PersistentFlags()); err != nil {
+		return err
+	}
 	return validation.ValidateNodeConfiguration(j.cfg).ToAggregate()
 }
 

cmd/kubeadm/test/cmd/init_test.go

@@ -137,3 +137,31 @@ func TestCmdInitAPIPort(t *testing.T) {
 		kubeadmReset()
 	}
 }
+
+func TestCmdInitArgsMixed(t *testing.T) {
+	if *kubeadmCmdSkip {
+		t.Log("kubeadm cmd tests being skipped")
+		t.Skip()
+	}
+
+	var initTest = []struct {
+		args     string
+		expected bool
+	}{
+		{"--api-port=1000 --config=/etc/kubernets/kubeadm.config", false},
+	}
+
+	for _, rt := range initTest {
+		_, _, actual := RunCmd(*kubeadmPath, "init", rt.args, "--skip-preflight-checks")
+		if (actual == nil) != rt.expected {
+			t.Errorf(
+				"failed CmdInitArgsMixed running 'kubeadm init %s' with an error: %v\n\texpected: %t\n\t  actual: %t",
+				rt.args,
+				actual,
+				rt.expected,
+				(actual == nil),
+			)
+		}
+		kubeadmReset()
+	}
+}

cmd/kubeadm/test/cmd/join_test.go

@@ -191,3 +191,31 @@ func TestCmdJoinBadArgs(t *testing.T) {
 		kubeadmReset()
 	}
 }
+
+func TestCmdJoinArgsMixed(t *testing.T) {
+	if *kubeadmCmdSkip {
+		t.Log("kubeadm cmd tests being skipped")
+		t.Skip()
+	}
+
+	var initTest = []struct {
+		args     string
+		expected bool
+	}{
+		{"--discovery-token=abcdef.1234567890abcdef --config=/etc/kubernets/kubeadm.config", false},
+	}
+
+	for _, rt := range initTest {
+		_, _, actual := RunCmd(*kubeadmPath, "join", rt.args, "--skip-preflight-checks")
+		if (actual == nil) != rt.expected {
+			t.Errorf(
+				"failed CmdJoinArgsMixed running 'kubeadm join %s' with an error: %v\n\texpected: %t\n\t  actual: %t",
+				rt.args,
+				actual,
+				rt.expected,
+				(actual == nil),
+			)
+		}
+		kubeadmReset()
+	}
+}