Merge pull request #34873 from k82cn/k8s_34633

pkg/kubectl/cmd/config/BUILD

@@ -61,6 +61,7 @@ go_test(
         "//pkg/api:go_default_library",
         "//pkg/client/unversioned/clientcmd:go_default_library",
         "//pkg/client/unversioned/clientcmd/api:go_default_library",
+        "//pkg/kubectl/cmd/util:go_default_library",
         "//pkg/util/diff:go_default_library",
         "//pkg/util/flag:go_default_library",
     ],

pkg/kubectl/cmd/config/config_test.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
 	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
+	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/util/diff"
 )
 
@@ -107,13 +108,30 @@ func TestSetCurrentContext(t *testing.T) {
 
 func TestSetNonExistentContext(t *testing.T) {
 	expectedConfig := newRedFederalCowHammerConfig()
+
 	test := configCommandTest{
-		args:            []string{"use-context", "non-existent-config"},
-		startingConfig:  expectedConfig,
-		expectedConfig:  expectedConfig,
-		expectedOutputs: []string{`no context exists with the name: "non-existent-config"`},
+		args:           []string{"use-context", "non-existent-config"},
+		startingConfig: expectedConfig,
+		expectedConfig: expectedConfig,
 	}
-	test.run(t)
+
+	func() {
+		defer func() {
+			// Restore cmdutil behavior.
+			cmdutil.DefaultBehaviorOnFatal()
+		}()
+
+		// Check exit code.
+		cmdutil.BehaviorOnFatal(func(e string, code int) {
+			if code != 1 {
+				t.Errorf("The exit code is %d, expected 1", code)
+			}
+			expectedOutputs := []string{`no context exists with the name: "non-existent-config"`}
+			test.checkOutput(e, expectedOutputs, t)
+		})
+
+		test.run(t)
+	}()
 }
 
 func TestSetIntoExistingStruct(t *testing.T) {
@@ -283,13 +301,28 @@ func TestEmbedClientKey(t *testing.T) {
 func TestEmbedNoKeyOrCertDisallowed(t *testing.T) {
 	expectedConfig := newRedFederalCowHammerConfig()
 	test := configCommandTest{
-		args:            []string{"set-credentials", "another-user", "--" + clientcmd.FlagEmbedCerts + "=true"},
-		startingConfig:  newRedFederalCowHammerConfig(),
-		expectedConfig:  expectedConfig,
-		expectedOutputs: []string{"--client-certificate", "--client-key", "embed"},
+		args:           []string{"set-credentials", "another-user", "--" + clientcmd.FlagEmbedCerts + "=true"},
+		startingConfig: newRedFederalCowHammerConfig(),
+		expectedConfig: expectedConfig,
 	}
 
-	test.run(t)
+	func() {
+		defer func() {
+			// Restore cmdutil behavior.
+			cmdutil.DefaultBehaviorOnFatal()
+		}()
+
+		// Check exit code.
+		cmdutil.BehaviorOnFatal(func(e string, code int) {
+			if code != 1 {
+				t.Errorf("The exit code is %d, expected 1", code)
+			}
+			expectedOutputs := []string{"--client-certificate", "--client-key", "embed"}
+			test.checkOutput(e, expectedOutputs, t)
+		})
+
+		test.run(t)
+	}()
 }
 
 func TestEmptyTokenAndCertAllowed(t *testing.T) {
@@ -327,13 +360,29 @@ func TestTokenAndCertAllowed(t *testing.T) {
 func TestTokenAndBasicDisallowed(t *testing.T) {
 	expectedConfig := newRedFederalCowHammerConfig()
 	test := configCommandTest{
-		args:            []string{"set-credentials", "another-user", "--" + clientcmd.FlagUsername + "=myuser", "--" + clientcmd.FlagBearerToken + "=token"},
-		startingConfig:  newRedFederalCowHammerConfig(),
-		expectedConfig:  expectedConfig,
-		expectedOutputs: []string{"--token", "--username"},
+		args:           []string{"set-credentials", "another-user", "--" + clientcmd.FlagUsername + "=myuser", "--" + clientcmd.FlagBearerToken + "=token"},
+		startingConfig: newRedFederalCowHammerConfig(),
+		expectedConfig: expectedConfig,
 	}
 
-	test.run(t)
+	func() {
+		defer func() {
+			// Restore cmdutil behavior.
+			cmdutil.DefaultBehaviorOnFatal()
+		}()
+
+		// Check exit code.
+		cmdutil.BehaviorOnFatal(func(e string, code int) {
+			if code != 1 {
+				t.Errorf("The exit code is %d, expected 1", code)
+			}
+
+			expectedOutputs := []string{"--token", "--username"}
+			test.checkOutput(e, expectedOutputs, t)
+		})
+
+		test.run(t)
+	}()
 }
 
 func TestBasicClearsToken(t *testing.T) {
@@ -445,7 +494,21 @@ func TestSetBytesBad(t *testing.T) {
 		expectedConfig: startingConfig,
 	}
 
-	test.run(t)
+	func() {
+		defer func() {
+			// Restore cmdutil behavior.
+			cmdutil.DefaultBehaviorOnFatal()
+		}()
+
+		// Check exit code.
+		cmdutil.BehaviorOnFatal(func(e string, code int) {
+			if code != 1 {
+				t.Errorf("The exit code is %d, expected 1", code)
+			}
+		})
+
+		test.run(t)
+	}()
 }
 
 func TestSetBytes(t *testing.T) {
@@ -604,24 +667,56 @@ func TestCADataClearsCA(t *testing.T) {
 func TestEmbedNoCADisallowed(t *testing.T) {
 	expectedConfig := newRedFederalCowHammerConfig()
 	test := configCommandTest{
-		args:            []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagEmbedCerts + "=true"},
-		startingConfig:  newRedFederalCowHammerConfig(),
-		expectedConfig:  expectedConfig,
-		expectedOutputs: []string{"--certificate-authority", "embed"},
+		args:           []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagEmbedCerts + "=true"},
+		startingConfig: newRedFederalCowHammerConfig(),
+		expectedConfig: expectedConfig,
 	}
 
-	test.run(t)
+	func() {
+		defer func() {
+			// Restore cmdutil behavior.
+			cmdutil.DefaultBehaviorOnFatal()
+		}()
+
+		// Check exit code.
+		cmdutil.BehaviorOnFatal(func(e string, code int) {
+			if code != 1 {
+				t.Errorf("The exit code is %d, expected 1", code)
+			}
+
+			expectedOutputs := []string{"--certificate-authority", "embed"}
+			test.checkOutput(e, expectedOutputs, t)
+		})
+
+		test.run(t)
+	}()
 }
 
 func TestCAAndInsecureDisallowed(t *testing.T) {
 	test := configCommandTest{
-		args:            []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagCAFile + "=cafile", "--" + clientcmd.FlagInsecure + "=true"},
-		startingConfig:  newRedFederalCowHammerConfig(),
-		expectedConfig:  newRedFederalCowHammerConfig(),
-		expectedOutputs: []string{"certificate", "insecure"},
+		args:           []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagCAFile + "=cafile", "--" + clientcmd.FlagInsecure + "=true"},
+		startingConfig: newRedFederalCowHammerConfig(),
+		expectedConfig: newRedFederalCowHammerConfig(),
 	}
 
-	test.run(t)
+	func() {
+		defer func() {
+			// Restore cmdutil behavior.
+			cmdutil.DefaultBehaviorOnFatal()
+		}()
+
+		// Check exit code.
+		cmdutil.BehaviorOnFatal(func(e string, code int) {
+			if code != 1 {
+				t.Errorf("The exit code is %d, expected 1", code)
+			}
+
+			expectedOutputs := []string{"certificate", "insecure"}
+			test.checkOutput(e, expectedOutputs, t)
+		})
+
+		test.run(t)
+	}()
 }
 
 func TestMergeExistingAuth(t *testing.T) {
@@ -787,6 +882,14 @@ type configCommandTest struct {
 	expectedOutputs []string
 }
 
+func (test configCommandTest) checkOutput(out string, expectedOutputs []string, t *testing.T) {
+	for _, expectedOutput := range expectedOutputs {
+		if !strings.Contains(out, expectedOutput) {
+			t.Errorf("expected '%s' in output, got '%s'", expectedOutput, out)
+		}
+	}
+}
+
 func (test configCommandTest) run(t *testing.T) string {
 	out, actualConfig := testConfigCommand(test.args, test.startingConfig, t)
 
@@ -799,11 +902,7 @@ func (test configCommandTest) run(t *testing.T) string {
 		t.Errorf("expected: %#v\n actual:   %#v", test.expectedConfig, actualConfig)
 	}
 
-	for _, expectedOutput := range test.expectedOutputs {
-		if !strings.Contains(out, expectedOutput) {
-			t.Errorf("expected '%s' in output, got '%s'", expectedOutput, out)
-		}
-	}
+	test.checkOutput(out, test.expectedOutputs, t)
 
 	return out
 }

pkg/kubectl/cmd/config/create_authinfo.go

@@ -109,12 +109,8 @@ func newCmdConfigSetAuthInfo(out io.Writer, options *createAuthInfoOptions) *cob
 				return
 			}
 
-			err := options.run()
-			if err != nil {
-				fmt.Fprintf(out, "%v\n", err)
-			} else {
-				fmt.Fprintf(out, "user %q set.\n", options.name)
-			}
+			cmdutil.CheckErr(options.run())
+			fmt.Fprintf(out, "User %q set.\n", options.name)
 		},
 	}
 

pkg/kubectl/cmd/config/create_cluster.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
 	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
+	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/util/flag"
 )
 
@@ -71,12 +72,8 @@ func NewCmdConfigSetCluster(out io.Writer, configAccess clientcmd.ConfigAccess)
 				return
 			}
 
-			err := options.run()
-			if err != nil {
-				fmt.Fprintf(out, "%v\n", err)
-			} else {
-				fmt.Fprintf(out, "cluster %q set.\n", options.name)
-			}
+			cmdutil.CheckErr(options.run())
+			fmt.Fprintf(out, "Cluster %q set.\n", options.name)
 		},
 	}
 

pkg/kubectl/cmd/config/create_context.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
 	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
+	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/util/flag"
 )
 
@@ -61,12 +62,8 @@ func NewCmdConfigSetContext(out io.Writer, configAccess clientcmd.ConfigAccess)
 				return
 			}
 
-			err := options.run()
-			if err != nil {
-				fmt.Fprintf(out, "%v\n", err)
-			} else {
-				fmt.Fprintf(out, "context %q set.\n", options.name)
-			}
+			cmdutil.CheckErr(options.run())
+			fmt.Fprintf(out, "Context %q set.\n", options.name)
 		},
 	}
 

pkg/kubectl/cmd/config/set.go

@@ -28,6 +28,7 @@ import (
 
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
+	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/util/flag"
 )
 
@@ -62,12 +63,8 @@ func NewCmdConfigSet(out io.Writer, configAccess clientcmd.ConfigAccess) *cobra.
 				return
 			}
 
-			err := options.run()
-			if err != nil {
-				fmt.Fprintf(out, "%v\n", err)
-			} else {
-				fmt.Fprintf(out, "property %q set.\n", options.propertyName)
-			}
+			cmdutil.CheckErr(options.run())
+			fmt.Fprintf(out, "Property %q set.\n", options.propertyName)
 		},
 	}
 

pkg/kubectl/cmd/config/unset.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
+	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 )
 
 type unsetOptions struct {
@@ -50,12 +51,8 @@ func NewCmdConfigUnset(out io.Writer, configAccess clientcmd.ConfigAccess) *cobr
 				return
 			}
 
-			err := options.run()
-			if err != nil {
-				fmt.Fprintf(out, "%v\n", err)
-			} else {
-				fmt.Fprintf(out, "property %q unset.\n", options.propertyName)
-			}
+			cmdutil.CheckErr(options.run())
+			fmt.Fprintf(out, "Property %q unset.\n", options.propertyName)
 		},
 	}
 

pkg/kubectl/cmd/config/use_context.go

@@ -25,6 +25,7 @@ import (
 
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
 	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
+	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 )
 
 type useContextOptions struct {
@@ -44,12 +45,8 @@ func NewCmdConfigUseContext(out io.Writer, configAccess clientcmd.ConfigAccess)
 				return
 			}
 
-			err := options.run()
-			if err != nil {
-				fmt.Fprintf(out, "%v\n", err)
-			} else {
-				fmt.Fprintf(out, "switched to context %q.\n", options.contextName)
-			}
+			cmdutil.CheckErr(options.run())
+			fmt.Fprintf(out, "Switched to context %q.\n", options.contextName)
 		},
 	}