diff --git a/pkg/kubectl/cmd/config/BUILD b/pkg/kubectl/cmd/config/BUILD index 27ab3f8229b..ac782c54331 100644 --- a/pkg/kubectl/cmd/config/BUILD +++ b/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", ], diff --git a/pkg/kubectl/cmd/config/config_test.go b/pkg/kubectl/cmd/config/config_test.go index e52f85e8834..a37b18fa71f 100644 --- a/pkg/kubectl/cmd/config/config_test.go +++ b/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 } diff --git a/pkg/kubectl/cmd/config/create_authinfo.go b/pkg/kubectl/cmd/config/create_authinfo.go index 8c2b2107bd0..8629353b36e 100644 --- a/pkg/kubectl/cmd/config/create_authinfo.go +++ b/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) }, } diff --git a/pkg/kubectl/cmd/config/create_cluster.go b/pkg/kubectl/cmd/config/create_cluster.go index e5a30f1362d..9c697bc2be4 100644 --- a/pkg/kubectl/cmd/config/create_cluster.go +++ b/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) }, } diff --git a/pkg/kubectl/cmd/config/create_context.go b/pkg/kubectl/cmd/config/create_context.go index 9e3c1dc2e23..e4184b9e931 100644 --- a/pkg/kubectl/cmd/config/create_context.go +++ b/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) }, } diff --git a/pkg/kubectl/cmd/config/set.go b/pkg/kubectl/cmd/config/set.go index 233092bf542..faf02308389 100644 --- a/pkg/kubectl/cmd/config/set.go +++ b/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) }, } diff --git a/pkg/kubectl/cmd/config/unset.go b/pkg/kubectl/cmd/config/unset.go index 9b685f638d7..48a56f8b46e 100644 --- a/pkg/kubectl/cmd/config/unset.go +++ b/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) }, } diff --git a/pkg/kubectl/cmd/config/use_context.go b/pkg/kubectl/cmd/config/use_context.go index f73037aa0a7..b80890de874 100644 --- a/pkg/kubectl/cmd/config/use_context.go +++ b/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) }, }