From d29560d89aca9727b3d5f7353d3c1b16c4918ffd Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Thu, 25 May 2017 12:16:55 -0700 Subject: [PATCH 1/8] kubectl: rename Run() -> RunRun() to clarify purpose Run() is too overloaded in the codebase already. The other commands have a pattern of RunExpose, RunScale, and so on. Since the command name is "run", the associated function should be called RunRun. --- pkg/kubectl/cmd/run.go | 4 ++-- pkg/kubectl/cmd/run_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/kubectl/cmd/run.go b/pkg/kubectl/cmd/run.go index be13117ea25..1fb6588fbad 100644 --- a/pkg/kubectl/cmd/run.go +++ b/pkg/kubectl/cmd/run.go @@ -95,7 +95,7 @@ func NewCmdRun(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer) *co Example: runExample, Run: func(cmd *cobra.Command, args []string) { argsLenAtDash := cmd.ArgsLenAtDash() - err := Run(f, cmdIn, cmdOut, cmdErr, cmd, args, argsLenAtDash) + err := RunRun(f, cmdIn, cmdOut, cmdErr, cmd, args, argsLenAtDash) cmdutil.CheckErr(err) }, } @@ -136,7 +136,7 @@ func addRunFlags(cmd *cobra.Command) { cmd.Flags().String("schedule", "", i18n.T("A schedule in the Cron format the job should be run with.")) } -func Run(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *cobra.Command, args []string, argsLenAtDash int) error { +func RunRun(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *cobra.Command, args []string, argsLenAtDash int) error { // Let kubectl run follow rules for `--`, see #13004 issue if len(args) == 0 || argsLenAtDash == 0 { return cmdutil.UsageError(cmd, "NAME is required for run") diff --git a/pkg/kubectl/cmd/run_test.go b/pkg/kubectl/cmd/run_test.go index 97c3328f49b..ed3b7860a9e 100644 --- a/pkg/kubectl/cmd/run_test.go +++ b/pkg/kubectl/cmd/run_test.go @@ -181,7 +181,7 @@ func TestRunArgsFollowDashRules(t *testing.T) { cmd := NewCmdRun(f, os.Stdin, os.Stdout, os.Stderr) cmd.Flags().Set("image", "nginx") cmd.Flags().Set("generator", "run/v1") - err := Run(f, os.Stdin, os.Stdout, os.Stderr, cmd, test.args, test.argsLenAtDash) + err := RunRun(f, os.Stdin, os.Stdout, os.Stderr, cmd, test.args, test.argsLenAtDash) if test.expectError && err == nil { t.Errorf("unexpected non-error (%s)", test.name) } @@ -438,7 +438,7 @@ func TestRunValidations(t *testing.T) { for flagName, flagValue := range test.flags { cmd.Flags().Set(flagName, flagValue) } - err := Run(f, inBuf, outBuf, errBuf, cmd, test.args, cmd.ArgsLenAtDash()) + err := RunRun(f, inBuf, outBuf, errBuf, cmd, test.args, cmd.ArgsLenAtDash()) if err != nil && len(test.expectedErr) > 0 { if !strings.Contains(err.Error(), test.expectedErr) { t.Errorf("unexpected error: %v", err) From f9913c4948038b9b51f2342134d546c6bb74e7a3 Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Thu, 25 May 2017 12:23:22 -0700 Subject: [PATCH 2/8] kubectl: rewrite docstrings in several files Fixing inaccuracies and clarifying in the case of ambiguities. --- pkg/kubectl/cmd/cp.go | 15 ++++++++------- pkg/kubectl/cmd/create.go | 4 ++-- pkg/kubectl/cmd/create_deployment.go | 4 +++- pkg/kubectl/doc.go | 7 ++++--- pkg/kubectl/generate.go | 4 +++- pkg/kubectl/kubectl.go | 5 ++++- pkg/kubectl/proxy_server.go | 3 ++- pkg/kubectl/stop.go | 9 +++++---- 8 files changed, 31 insertions(+), 20 deletions(-) diff --git a/pkg/kubectl/cmd/cp.go b/pkg/kubectl/cmd/cp.go index a0bac76c39f..d5e6c18cd8d 100644 --- a/pkg/kubectl/cmd/cp.go +++ b/pkg/kubectl/cmd/cp.go @@ -35,14 +35,15 @@ import ( var ( cp_example = templates.Examples(i18n.T(` - # !!!Important Note!!! - # Requires that the 'tar' binary is present in your container - # image. If 'tar' is not present, 'kubectl cp' will fail. + # !!!Important Note!!! + # Requires that the 'tar' binary is present in your container + # image. If 'tar' is not present, 'kubectl cp' will fail. - # Copy /tmp/foo_dir local directory to /tmp/bar_dir in a remote pod in the default namespace + + # Copy /tmp/foo_dir local directory to /tmp/bar_dir in a remote pod in the default namespace kubectl cp /tmp/foo_dir :/tmp/bar_dir - # Copy /tmp/foo local file to /tmp/bar in a remote pod in a specific container + # Copy /tmp/foo local file to /tmp/bar in a remote pod in a specific container kubectl cp /tmp/foo :/tmp/bar -c # Copy /tmp/foo local file to /tmp/bar in a remote pod in namespace @@ -52,8 +53,8 @@ var ( kubectl cp /:/tmp/foo /tmp/bar`)) cpUsageStr = dedent.Dedent(` - expected 'cp [-c container]'. - is: + expected 'cp [-c container]'. + is: [namespace/]pod-name:/file/path for a remote file /file/path for a local file`) ) diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go index bfec15bc8c6..8112a6b7fe2 100644 --- a/pkg/kubectl/cmd/create.go +++ b/pkg/kubectl/cmd/create.go @@ -40,7 +40,7 @@ type CreateOptions struct { var ( create_long = templates.LongDesc(i18n.T(` - Create a resource by filename or stdin. + Create a resource from a file or from stdin. JSON and YAML formats are accepted.`)) @@ -60,7 +60,7 @@ func NewCmdCreate(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "create -f FILENAME", - Short: i18n.T("Create a resource by filename or stdin"), + Short: i18n.T("Create a resource from a file or from stdin."), Long: create_long, Example: create_example, Run: func(cmd *cobra.Command, args []string) { diff --git a/pkg/kubectl/cmd/create_deployment.go b/pkg/kubectl/cmd/create_deployment.go index b3393788e67..b7033d41f0e 100644 --- a/pkg/kubectl/cmd/create_deployment.go +++ b/pkg/kubectl/cmd/create_deployment.go @@ -38,7 +38,9 @@ var ( kubectl create deployment my-dep --image=busybox`)) ) -// NewCmdCreateDeployment is a macro command to create a new deployment +// NewCmdCreateDeployment is a macro command to create a new deployment. +// This command is better known to users as `kubectl create deployment`. +// Note that this command overlaps significantly with the `kubectl run` command. func NewCmdCreateDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "deployment NAME --image=image [--dry-run]", diff --git a/pkg/kubectl/doc.go b/pkg/kubectl/doc.go index df45d44691b..52afc49f19f 100644 --- a/pkg/kubectl/doc.go +++ b/pkg/kubectl/doc.go @@ -14,7 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package kubectl is a set of libraries that are used by the kubectl command line tool. -// They are separated out into a library to support unit testing. Most functionality should -// be included in this package, and the main kubectl should really just be an entry point. +// Package kubectl provides the functions used by the kubectl command line tool +// under k8s.io/kubernetes/cmd. The functions are kept in this package to better +// support unit testing. The main() method for kubectl is only an entry point +// and should contain no functionality. package kubectl // import "k8s.io/kubernetes/pkg/kubectl" diff --git a/pkg/kubectl/generate.go b/pkg/kubectl/generate.go index 07c437cdd47..083f15584a9 100644 --- a/pkg/kubectl/generate.go +++ b/pkg/kubectl/generate.go @@ -35,7 +35,9 @@ type GeneratorParam struct { Required bool } -// Generator is an interface for things that can generate API objects from input parameters. +// Generator is an interface for things that can generate API objects from input +// parameters. One example is the "expose" generator that is capable of exposing +// new replication controllers and services, among other things. type Generator interface { // Generate creates an API object given a set of parameters Generate(params map[string]interface{}) (runtime.Object, error) diff --git a/pkg/kubectl/kubectl.go b/pkg/kubectl/kubectl.go index d22953adbfa..2cf9425eb8e 100644 --- a/pkg/kubectl/kubectl.go +++ b/pkg/kubectl/kubectl.go @@ -212,7 +212,10 @@ func parseFileSource(source string) (keyName, filePath string, err error) { } } -// parseLiteralSource parses the source key=val pair +// parseLiteralSource parses the source key=val pair into its component pieces. +// This functionality is distinguished from strings.SplitN(source, "=", 2) since +// it returns an error in the case of empty keys, values, or a missing equals +// sign. func parseLiteralSource(source string) (keyName, value string, err error) { // leading equal is invalid if strings.Index(source, "=") == 0 { diff --git a/pkg/kubectl/proxy_server.go b/pkg/kubectl/proxy_server.go index 8ae65004759..ce60727b4b8 100644 --- a/pkg/kubectl/proxy_server.go +++ b/pkg/kubectl/proxy_server.go @@ -108,7 +108,8 @@ func (f *FilterServer) accept(method, path, host string) bool { return false } -// Make a copy of f which passes requests along to the new delegate. +// HandlerFor makes a shallow copy of f which passes its requests along to the +// new delegate. func (f *FilterServer) HandlerFor(delegate http.Handler) *FilterServer { f2 := *f f2.delegate = delegate diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index 1a8db59b5f9..4fcb9415fc8 100644 --- a/pkg/kubectl/stop.go +++ b/pkg/kubectl/stop.go @@ -46,11 +46,12 @@ const ( Timeout = time.Minute * 5 ) -// A Reaper handles terminating an object as gracefully as possible. -// timeout is how long we'll wait for the termination to be successful -// gracePeriod is time given to an API object for it to delete itself cleanly, -// e.g., pod shutdown. It may or may not be supported by the API object. +// A Reaper terminates an object as gracefully as possible. type Reaper interface { + // Stop a given object within a namespace. timeout is how long we'll + // wait for the termination to be successful. gracePeriod is time given + // to an API object for it to delete itself cleanly (e.g., pod + // shutdown). It may or may not be supported by the API object. Stop(namespace, name string, timeout time.Duration, gracePeriod *metav1.DeleteOptions) error } From 066dbb72062d00f17c42e0bcb805794530e9542c Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Thu, 25 May 2017 12:30:19 -0700 Subject: [PATCH 3/8] cmd: make createDeployment a private function --- pkg/kubectl/cmd/create_deployment.go | 5 ++--- pkg/kubectl/cmd/create_deployment_test.go | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/kubectl/cmd/create_deployment.go b/pkg/kubectl/cmd/create_deployment.go index b7033d41f0e..b8fe848218c 100644 --- a/pkg/kubectl/cmd/create_deployment.go +++ b/pkg/kubectl/cmd/create_deployment.go @@ -49,7 +49,7 @@ func NewCmdCreateDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer) *cobra. Long: deploymentLong, Example: deploymentExample, Run: func(cmd *cobra.Command, args []string) { - err := CreateDeployment(f, cmdOut, cmdErr, cmd, args) + err := createDeployment(f, cmdOut, cmdErr, cmd, args) cmdutil.CheckErr(err) }, } @@ -62,8 +62,7 @@ func NewCmdCreateDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer) *cobra. return cmd } -// CreateDeployment implements the behavior to run the create deployment command -func CreateDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer, cmd *cobra.Command, args []string) error { +func createDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer, cmd *cobra.Command, args []string) error { name, err := NameFromCommandArgs(cmd, args) if err != nil { return err diff --git a/pkg/kubectl/cmd/create_deployment_test.go b/pkg/kubectl/cmd/create_deployment_test.go index 9dd6720b785..33fefd6a839 100644 --- a/pkg/kubectl/cmd/create_deployment_test.go +++ b/pkg/kubectl/cmd/create_deployment_test.go @@ -80,6 +80,6 @@ func TestCreateDeploymentNoImage(t *testing.T) { cmd := NewCmdCreateDeployment(f, buf, buf) cmd.Flags().Set("dry-run", "true") cmd.Flags().Set("output", "name") - err := CreateDeployment(f, buf, buf, cmd, []string{depName}) + err := createDeployment(f, buf, buf, cmd, []string{depName}) assert.Error(t, err, "at least one image must be specified") } From 01ae6edc6c7b06f10f2f56f1c45c9ef5a942de9c Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Thu, 25 May 2017 12:44:43 -0700 Subject: [PATCH 4/8] cmd: refactor common err expr into helper function The same redundant fmt.Sprintf() and string literal was duplicated throughout many of the files in kubectl/cmd. Replace with a helper function. --- pkg/kubectl/cmd/create_clusterrolebinding.go | 3 +-- pkg/kubectl/cmd/create_configmap.go | 3 +-- pkg/kubectl/cmd/create_deployment.go | 2 +- pkg/kubectl/cmd/create_namespace.go | 3 +-- pkg/kubectl/cmd/create_pdb.go | 3 +-- pkg/kubectl/cmd/create_quota.go | 3 +-- pkg/kubectl/cmd/create_rolebinding.go | 3 +-- pkg/kubectl/cmd/create_secret.go | 7 +++---- pkg/kubectl/cmd/create_service.go | 13 ++++++++----- pkg/kubectl/cmd/create_serviceaccount.go | 3 +-- 10 files changed, 19 insertions(+), 24 deletions(-) diff --git a/pkg/kubectl/cmd/create_clusterrolebinding.go b/pkg/kubectl/cmd/create_clusterrolebinding.go index 6b08dcdbbc7..228d279ef26 100644 --- a/pkg/kubectl/cmd/create_clusterrolebinding.go +++ b/pkg/kubectl/cmd/create_clusterrolebinding.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "fmt" "io" "github.com/spf13/cobra" @@ -77,7 +76,7 @@ func CreateClusterRoleBinding(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Co ServiceAccounts: cmdutil.GetFlagStringArray(cmd, "serviceaccount"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, diff --git a/pkg/kubectl/cmd/create_configmap.go b/pkg/kubectl/cmd/create_configmap.go index e56ca0ae656..b6544e14610 100644 --- a/pkg/kubectl/cmd/create_configmap.go +++ b/pkg/kubectl/cmd/create_configmap.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "fmt" "io" "github.com/spf13/cobra" @@ -97,7 +96,7 @@ func CreateConfigMap(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Command, ar EnvFileSource: cmdutil.GetFlagString(cmd, "from-env-file"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, diff --git a/pkg/kubectl/cmd/create_deployment.go b/pkg/kubectl/cmd/create_deployment.go index b8fe848218c..367960198c9 100644 --- a/pkg/kubectl/cmd/create_deployment.go +++ b/pkg/kubectl/cmd/create_deployment.go @@ -92,7 +92,7 @@ func createDeployment(f cmdutil.Factory, cmdOut, cmdErr io.Writer, cmd *cobra.Co case cmdutil.DeploymentBasicV1Beta1GeneratorName: generator = &kubectl.DeploymentBasicGeneratorV1{Name: name, Images: cmdutil.GetFlagStringSlice(cmd, "image")} default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, diff --git a/pkg/kubectl/cmd/create_namespace.go b/pkg/kubectl/cmd/create_namespace.go index edd86e849e0..4f5a363d26c 100644 --- a/pkg/kubectl/cmd/create_namespace.go +++ b/pkg/kubectl/cmd/create_namespace.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "fmt" "io" "github.com/spf13/cobra" @@ -69,7 +68,7 @@ func CreateNamespace(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Command, ar case cmdutil.NamespaceV1GeneratorName: generator = &kubectl.NamespaceGeneratorV1{Name: name} default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, diff --git a/pkg/kubectl/cmd/create_pdb.go b/pkg/kubectl/cmd/create_pdb.go index dd282a4647f..95b7b4ebd52 100644 --- a/pkg/kubectl/cmd/create_pdb.go +++ b/pkg/kubectl/cmd/create_pdb.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "fmt" "io" "github.com/spf13/cobra" @@ -89,7 +88,7 @@ func CreatePodDisruptionBudget(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.C Selector: cmdutil.GetFlagString(cmd, "selector"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, diff --git a/pkg/kubectl/cmd/create_quota.go b/pkg/kubectl/cmd/create_quota.go index e26f4fcdfc0..055ee5a0eb7 100644 --- a/pkg/kubectl/cmd/create_quota.go +++ b/pkg/kubectl/cmd/create_quota.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "fmt" "io" "github.com/spf13/cobra" @@ -78,7 +77,7 @@ func CreateQuota(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Command, args [ Scopes: cmdutil.GetFlagString(cmd, "scopes"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, diff --git a/pkg/kubectl/cmd/create_rolebinding.go b/pkg/kubectl/cmd/create_rolebinding.go index eab697a3597..047a41cc059 100644 --- a/pkg/kubectl/cmd/create_rolebinding.go +++ b/pkg/kubectl/cmd/create_rolebinding.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "fmt" "io" "github.com/spf13/cobra" @@ -78,7 +77,7 @@ func CreateRoleBinding(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Command, ServiceAccounts: cmdutil.GetFlagStringArray(cmd, "serviceaccount"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, diff --git a/pkg/kubectl/cmd/create_secret.go b/pkg/kubectl/cmd/create_secret.go index dff9950695b..10c9d51eb1a 100644 --- a/pkg/kubectl/cmd/create_secret.go +++ b/pkg/kubectl/cmd/create_secret.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "fmt" "io" "github.com/spf13/cobra" @@ -110,7 +109,7 @@ func CreateSecretGeneric(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Command EnvFileSource: cmdutil.GetFlagString(cmd, "from-env-file"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, @@ -191,7 +190,7 @@ func CreateSecretDockerRegistry(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra. Server: cmdutil.GetFlagString(cmd, "docker-server"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, @@ -254,7 +253,7 @@ func CreateSecretTLS(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Command, ar Cert: cmdutil.GetFlagString(cmd, "cert"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, diff --git a/pkg/kubectl/cmd/create_service.go b/pkg/kubectl/cmd/create_service.go index c1423e009a9..4995427fee4 100644 --- a/pkg/kubectl/cmd/create_service.go +++ b/pkg/kubectl/cmd/create_service.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "fmt" "io" "github.com/spf13/cobra" @@ -83,6 +82,10 @@ func NewCmdCreateServiceClusterIP(f cmdutil.Factory, cmdOut io.Writer) *cobra.Co return cmd } +func errUnsupportedGenerator(cmd *cobra.Command, generatorName string) error { + return cmdutil.UsageError(cmd, "Generator %s not supported. ", generatorName) +} + // CreateServiceClusterIP implements the behavior to run the create service clusterIP command func CreateServiceClusterIP(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Command, args []string) error { name, err := NameFromCommandArgs(cmd, args) @@ -99,7 +102,7 @@ func CreateServiceClusterIP(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Comm ClusterIP: cmdutil.GetFlagString(cmd, "clusterip"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, @@ -156,7 +159,7 @@ func CreateServiceNodePort(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Comma NodePort: cmdutil.GetFlagInt(cmd, "node-port"), } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, @@ -211,7 +214,7 @@ func CreateServiceLoadBalancer(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.C ClusterIP: "", } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, @@ -272,7 +275,7 @@ func CreateExternalNameService(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.C ClusterIP: "", } default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, diff --git a/pkg/kubectl/cmd/create_serviceaccount.go b/pkg/kubectl/cmd/create_serviceaccount.go index e4fd48f882c..72578797e16 100644 --- a/pkg/kubectl/cmd/create_serviceaccount.go +++ b/pkg/kubectl/cmd/create_serviceaccount.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "fmt" "io" "github.com/spf13/cobra" @@ -69,7 +68,7 @@ func CreateServiceAccount(f cmdutil.Factory, cmdOut io.Writer, cmd *cobra.Comman case cmdutil.ServiceAccountV1GeneratorName: generator = &kubectl.ServiceAccountGeneratorV1{Name: name} default: - return cmdutil.UsageError(cmd, fmt.Sprintf("Generator: %s not supported.", generatorName)) + return errUnsupportedGenerator(cmd, generatorName) } return RunCreateSubcommand(f, cmd, cmdOut, &CreateSubcommandOptions{ Name: name, From ef9ae612403373d45003d05882a09c2b828f8ce3 Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Thu, 25 May 2017 13:37:56 -0700 Subject: [PATCH 5/8] kubectl: simplify code with help of linter --- pkg/kubectl/apply.go | 6 +----- pkg/kubectl/cmd/apply_set_last_applied.go | 11 ++--------- pkg/kubectl/cmd/convert.go | 6 +----- pkg/kubectl/cmd/cp.go | 5 ++--- pkg/kubectl/cmd/create.go | 8 ++++---- pkg/kubectl/cmd/delete.go | 2 +- pkg/kubectl/cmd/edit_test.go | 4 ++-- pkg/kubectl/cmd/expose.go | 14 +++++++------- pkg/kubectl/cmd/get.go | 3 +-- pkg/kubectl/cmd/get_test.go | 4 +--- pkg/kubectl/cmd/label.go | 2 +- pkg/kubectl/cmd/taint.go | 4 ++-- pkg/kubectl/cmd/version.go | 6 +----- pkg/kubectl/rolling_updater.go | 19 ++++++++----------- pkg/kubectl/run.go | 6 ------ pkg/kubectl/stop.go | 5 ----- 16 files changed, 34 insertions(+), 71 deletions(-) diff --git a/pkg/kubectl/apply.go b/pkg/kubectl/apply.go index b4b49b5826f..ba748889529 100644 --- a/pkg/kubectl/apply.go +++ b/pkg/kubectl/apply.go @@ -61,11 +61,7 @@ func SetOriginalConfiguration(info *resource.Info, original []byte) error { } annots[api.LastAppliedConfigAnnotation] = string(original) - if err := info.Mapping.MetadataAccessor.SetAnnotations(info.Object, annots); err != nil { - return err - } - - return nil + return info.Mapping.MetadataAccessor.SetAnnotations(info.Object, annots) } // GetModifiedConfiguration retrieves the modified configuration of the object. diff --git a/pkg/kubectl/cmd/apply_set_last_applied.go b/pkg/kubectl/cmd/apply_set_last_applied.go index c86e90c6e5e..188e54b09b7 100644 --- a/pkg/kubectl/cmd/apply_set_last_applied.go +++ b/pkg/kubectl/cmd/apply_set_last_applied.go @@ -118,11 +118,7 @@ func (o *SetLastAppliedOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) } o.Namespace, o.EnforceNamespace, err = f.DefaultNamespace() - if err != nil { - return err - } - - return nil + return err } func (o *SetLastAppliedOptions) Validate(f cmdutil.Factory, cmd *cobra.Command) error { @@ -179,10 +175,7 @@ func (o *SetLastAppliedOptions) Validate(f cmdutil.Factory, cmd *cobra.Command) return nil }) - if err != nil { - return err - } - return nil + return err } func (o *SetLastAppliedOptions) RunSetLastApplied(f cmdutil.Factory, cmd *cobra.Command) error { diff --git a/pkg/kubectl/cmd/convert.go b/pkg/kubectl/cmd/convert.go index b6737573107..c96e9420bd8 100644 --- a/pkg/kubectl/cmd/convert.go +++ b/pkg/kubectl/cmd/convert.go @@ -158,11 +158,7 @@ func (o *ConvertOptions) Complete(f cmdutil.Factory, out io.Writer, cmd *cobra.C } o.encoder = f.JSONEncoder() o.printer, err = f.PrinterForCommand(cmd, o.local, nil, printers.PrintOptions{}) - if err != nil { - return err - } - - return nil + return err } // RunConvert implements the generic Convert command diff --git a/pkg/kubectl/cmd/cp.go b/pkg/kubectl/cmd/cp.go index d5e6c18cd8d..fd9eaeacd29 100644 --- a/pkg/kubectl/cmd/cp.go +++ b/pkg/kubectl/cmd/cp.go @@ -34,12 +34,11 @@ import ( ) var ( - cp_example = templates.Examples(i18n.T(` + cpExample = templates.Examples(i18n.T(` # !!!Important Note!!! # Requires that the 'tar' binary is present in your container # image. If 'tar' is not present, 'kubectl cp' will fail. - # Copy /tmp/foo_dir local directory to /tmp/bar_dir in a remote pod in the default namespace kubectl cp /tmp/foo_dir :/tmp/bar_dir @@ -65,7 +64,7 @@ func NewCmdCp(f cmdutil.Factory, cmdOut, cmdErr io.Writer) *cobra.Command { Use: "cp ", Short: i18n.T("Copy files and directories to and from containers."), Long: "Copy files and directories to and from containers.", - Example: cp_example, + Example: cpExample, Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(runCopy(f, cmd, cmdOut, cmdErr, args)) }, diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go index 8112a6b7fe2..7d942131b32 100644 --- a/pkg/kubectl/cmd/create.go +++ b/pkg/kubectl/cmd/create.go @@ -39,12 +39,12 @@ type CreateOptions struct { } var ( - create_long = templates.LongDesc(i18n.T(` + createLong = templates.LongDesc(i18n.T(` Create a resource from a file or from stdin. JSON and YAML formats are accepted.`)) - create_example = templates.Examples(i18n.T(` + createExample = templates.Examples(i18n.T(` # Create a pod using the data in pod.json. kubectl create -f ./pod.json @@ -61,8 +61,8 @@ func NewCmdCreate(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "create -f FILENAME", Short: i18n.T("Create a resource from a file or from stdin."), - Long: create_long, - Example: create_example, + Long: createLong, + Example: createExample, Run: func(cmd *cobra.Command, args []string) { if cmdutil.IsFilenameEmpty(options.FilenameOptions.Filenames) { defaultRunFunc := cmdutil.DefaultSubCommandRun(errOut) diff --git a/pkg/kubectl/cmd/delete.go b/pkg/kubectl/cmd/delete.go index e43ff2889ec..8e9e178fe62 100644 --- a/pkg/kubectl/cmd/delete.go +++ b/pkg/kubectl/cmd/delete.go @@ -327,7 +327,7 @@ func deleteResource(info *resource.Info, out io.Writer, shortOutput bool, mapper return nil } -// objectDeletionWaitInterval is the interval to wait between checks for deletion. Exposed for testing. +// objectDeletionWaitInterval is the interval to wait between checks for deletion. var objectDeletionWaitInterval = time.Second // waitForObjectDeletion refreshes the object, waiting until it is deleted, a timeout is reached, or diff --git a/pkg/kubectl/cmd/edit_test.go b/pkg/kubectl/cmd/edit_test.go index 55516bb0beb..1155efa6ef2 100644 --- a/pkg/kubectl/cmd/edit_test.go +++ b/pkg/kubectl/cmd/edit_test.go @@ -116,7 +116,7 @@ func TestEdit(t *testing.T) { if step.StepType != "edit" { t.Fatalf("%s, step %d: expected edit step, got %s %s", name, i, req.Method, req.URL.Path) } - if bytes.Compare(body, expectedInput) != 0 { + if !bytes.Equal(body, expectedInput) { if updateInputFixtures { // Convenience to allow recapturing the input and persisting it here ioutil.WriteFile(inputFile, body, os.FileMode(0644)) @@ -139,7 +139,7 @@ func TestEdit(t *testing.T) { req.Method, req.URL.Path, req.Header.Get("Content-Type"), ) } - if bytes.Compare(body, expectedInput) != 0 { + if !bytes.Equal(body, expectedInput) { if updateInputFixtures { // Convenience to allow recapturing the input and persisting it here ioutil.WriteFile(inputFile, body, os.FileMode(0644)) diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index 2314044094b..20580d8b399 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/pkg/kubectl/cmd/expose.go @@ -34,9 +34,9 @@ import ( ) var ( - expose_resources = `pod (po), service (svc), replicationcontroller (rc), deployment (deploy), replicaset (rs)` + exposeResources = `pod (po), service (svc), replicationcontroller (rc), deployment (deploy), replicaset (rs)` - expose_long = templates.LongDesc(` + exposeLong = templates.LongDesc(` Expose a resource as a new Kubernetes service. Looks up a deployment, service, replica set, replication controller or pod by name and uses the selector @@ -48,9 +48,9 @@ var ( Possible resources include (case insensitive): - ` + expose_resources) + ` + exposeResources) - expose_example = templates.Examples(i18n.T(` + exposeExample = templates.Examples(i18n.T(` # Create a service for a replicated nginx, which serves on port 80 and connects to the containers on port 8000. kubectl expose rc nginx --port=80 --target-port=8000 @@ -77,7 +77,7 @@ func NewCmdExposeService(f cmdutil.Factory, out io.Writer) *cobra.Command { options := &resource.FilenameOptions{} validArgs, argAliases := []string{}, []string{} - resources := regexp.MustCompile(`\s*,`).Split(expose_resources, -1) + resources := regexp.MustCompile(`\s*,`).Split(exposeResources, -1) for _, r := range resources { validArgs = append(validArgs, strings.Fields(r)[0]) argAliases = kubectl.ResourceAliases(validArgs) @@ -86,8 +86,8 @@ func NewCmdExposeService(f cmdutil.Factory, out io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "expose (-f FILENAME | TYPE NAME) [--port=port] [--protocol=TCP|UDP] [--target-port=number-or-name] [--name=name] [--external-ip=external-ip-of-service] [--type=type]", Short: i18n.T("Take a replication controller, service, deployment or pod and expose it as a new Kubernetes Service"), - Long: expose_long, - Example: expose_example, + Long: exposeLong, + Example: exposeExample, Run: func(cmd *cobra.Command, args []string) { err := RunExpose(f, out, cmd, args, options) cmdutil.CheckErr(err) diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go index f7ee34826f6..833b86da611 100644 --- a/pkg/kubectl/cmd/get.go +++ b/pkg/kubectl/cmd/get.go @@ -289,8 +289,7 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [ first = false return false, nil } - err := printer.PrintObj(e.Object, out) - return false, err + return false, printer.PrintObj(e.Object, out) }) return err }) diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index 4fa573cfcde..e2c80d5f629 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -506,9 +506,7 @@ func extractResourceList(objs []runtime.Object) ([]runtime.Object, error) { if err != nil { return nil, err } - for _, item := range items { - finalObjs = append(finalObjs, item) - } + finalObjs = append(finalObjs, items...) } return finalObjs, nil } diff --git a/pkg/kubectl/cmd/label.go b/pkg/kubectl/cmd/label.go index d9568e6c7fb..33f2a1e9c33 100644 --- a/pkg/kubectl/cmd/label.go +++ b/pkg/kubectl/cmd/label.go @@ -308,7 +308,7 @@ func parseLabels(spec []string) (map[string]string, []string, error) { labels := map[string]string{} var remove []string for _, labelSpec := range spec { - if strings.Index(labelSpec, "=") != -1 { + if strings.Contains(labelSpec, "=") { parts := strings.Split(labelSpec, "=") if len(parts) != 2 { return nil, nil, fmt.Errorf("invalid label spec: %v", labelSpec) diff --git a/pkg/kubectl/cmd/taint.go b/pkg/kubectl/cmd/taint.go index 837a0219623..2b2cb42933b 100644 --- a/pkg/kubectl/cmd/taint.go +++ b/pkg/kubectl/cmd/taint.go @@ -178,7 +178,7 @@ func parseTaints(spec []string) ([]v1.Taint, []v1.Taint, error) { uniqueTaints := map[v1.TaintEffect]sets.String{} for _, taintSpec := range spec { - if strings.Index(taintSpec, "=") != -1 && strings.Index(taintSpec, ":") != -1 { + if strings.Contains(taintSpec, "=") && strings.Contains(taintSpec, ":") { newTaint, err := utiltaints.ParseTaint(taintSpec) if err != nil { return nil, nil, err @@ -197,7 +197,7 @@ func parseTaints(spec []string) ([]v1.Taint, []v1.Taint, error) { } else if strings.HasSuffix(taintSpec, "-") { taintKey := taintSpec[:len(taintSpec)-1] var effect v1.TaintEffect - if strings.Index(taintKey, ":") != -1 { + if strings.Contains(taintKey, ":") { parts := strings.Split(taintKey, ":") taintKey = parts[0] effect = v1.TaintEffect(parts[1]) diff --git a/pkg/kubectl/cmd/version.go b/pkg/kubectl/cmd/version.go index 56ecf51e60b..b939a99e53a 100644 --- a/pkg/kubectl/cmd/version.go +++ b/pkg/kubectl/cmd/version.go @@ -107,11 +107,7 @@ func RunVersion(f cmdutil.Factory, out io.Writer, cmd *cobra.Command) error { } - if serverErr != nil { - return serverErr - } - - return nil + return serverErr } func retrieveServerVersion(f cmdutil.Factory) (*apimachineryversion.Info, error) { diff --git a/pkg/kubectl/rolling_updater.go b/pkg/kubectl/rolling_updater.go index 7adb8f37a1a..3c50e3921ed 100644 --- a/pkg/kubectl/rolling_updater.go +++ b/pkg/kubectl/rolling_updater.go @@ -565,10 +565,7 @@ func Rename(c coreclient.ReplicationControllersGetter, rc *api.ReplicationContro } // Then create the same RC with the new name. _, err = c.ReplicationControllers(rc.Namespace).Create(rc) - if err != nil { - return err - } - return nil + return err } func LoadExistingNextReplicationController(c coreclient.ReplicationControllersGetter, namespace, newName string) (*api.ReplicationController, error) { @@ -684,14 +681,14 @@ func UpdateExistingReplicationController(rcClient coreclient.ReplicationControll if _, found := oldRc.Spec.Selector[deploymentKey]; !found { SetNextControllerAnnotation(oldRc, newName) return AddDeploymentKeyToReplicationController(oldRc, rcClient, podClient, deploymentKey, deploymentValue, namespace, out) - } else { - // If we didn't need to update the controller for the deployment key, we still need to write - // the "next" controller. - applyUpdate := func(rc *api.ReplicationController) { - SetNextControllerAnnotation(rc, newName) - } - return updateRcWithRetries(rcClient, namespace, oldRc, applyUpdate) } + + // If we didn't need to update the controller for the deployment key, we still need to write + // the "next" controller. + applyUpdate := func(rc *api.ReplicationController) { + SetNextControllerAnnotation(rc, newName) + } + return updateRcWithRetries(rcClient, namespace, oldRc, applyUpdate) } func AddDeploymentKeyToReplicationController(oldRc *api.ReplicationController, rcClient coreclient.ReplicationControllersGetter, podClient coreclient.PodsGetter, deploymentKey, deploymentValue, namespace string, out io.Writer) (*api.ReplicationController, error) { diff --git a/pkg/kubectl/run.go b/pkg/kubectl/run.go index 2b00f68e33d..2ec50978d5d 100644 --- a/pkg/kubectl/run.go +++ b/pkg/kubectl/run.go @@ -868,9 +868,3 @@ func parseEnvs(envArray []string) ([]v1.EnvVar, error) { } return envs, nil } - -func newBool(val bool) *bool { - p := new(bool) - *p = val - return p -} diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index 4fcb9415fc8..c75fe03f749 100644 --- a/pkg/kubectl/stop.go +++ b/pkg/kubectl/stop.go @@ -136,11 +136,6 @@ type StatefulSetReaper struct { pollInterval, timeout time.Duration } -type objInterface interface { - Delete(name string) error - Get(name string) (metav1.Object, error) -} - // getOverlappingControllers finds rcs that this controller overlaps, as well as rcs overlapping this controller. func getOverlappingControllers(rcClient coreclient.ReplicationControllerInterface, rc *api.ReplicationController) ([]api.ReplicationController, error) { rcs, err := rcClient.List(metav1.ListOptions{}) From 7b54199fd5efcec65ba76076455fd5b6fe4e865b Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Thu, 25 May 2017 13:38:23 -0700 Subject: [PATCH 6/8] kubectl: note a bug with a comment This doesn't seem to be affecting anything and I'm not sure what the correct behavior needs to be here. I'll highlight this in the code review and hopefully work out a correct solution with the help of the reviewers. --- pkg/kubectl/stop.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index c75fe03f749..86510ac02af 100644 --- a/pkg/kubectl/stop.go +++ b/pkg/kubectl/stop.go @@ -332,6 +332,8 @@ func (reaper *StatefulSetReaper) Stop(namespace, name string, timeout time.Durat } if timeout == 0 { numReplicas := ss.Spec.Replicas + + // BUG: this timeout is never used. timeout = Timeout + time.Duration(10*numReplicas)*time.Second } retry := NewRetryParams(reaper.pollInterval, reaper.timeout) From 63e9c67db8cc73a28e37073bd87587884da0eba2 Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Thu, 25 May 2017 14:23:00 -0700 Subject: [PATCH 7/8] kubectl: refactor addFromEnvFile, write tests --- pkg/kubectl/BUILD | 1 + pkg/kubectl/env_file.go | 80 ++++++++++++++++++--------- pkg/kubectl/env_file_test.go | 103 +++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 27 deletions(-) create mode 100644 pkg/kubectl/env_file_test.go diff --git a/pkg/kubectl/BUILD b/pkg/kubectl/BUILD index b269c52d7de..9e953c04dde 100644 --- a/pkg/kubectl/BUILD +++ b/pkg/kubectl/BUILD @@ -14,6 +14,7 @@ go_test( "cluster_test.go", "configmap_test.go", "deployment_test.go", + "env_file_test.go", "generate_test.go", "kubectl_test.go", "namespace_test.go", diff --git a/pkg/kubectl/env_file.go b/pkg/kubectl/env_file.go index bbdcdc657ab..ce24c710441 100644 --- a/pkg/kubectl/env_file.go +++ b/pkg/kubectl/env_file.go @@ -28,6 +28,47 @@ import ( "k8s.io/apimachinery/pkg/util/validation" ) +var utf8bom = []byte{0xEF, 0xBB, 0xBF} + +// proccessEnvFileLine returns a blank key if the line is empty or a comment. +// The value will be retrieved from the environment if necessary. +func proccessEnvFileLine(line []byte, filePath string, + currentLine int) (key, value string, err error) { + + if !utf8.Valid(line) { + return ``, ``, fmt.Errorf("env file %s contains invalid utf8 bytes at line %d: %v", + filePath, currentLine+1, line) + } + + // We trim UTF8 BOM from the first line of the file but no others + if currentLine == 0 { + line = bytes.TrimPrefix(line, utf8bom) + } + + // trim the line from all leading whitespace first + line = bytes.TrimLeftFunc(line, unicode.IsSpace) + + // If the line is empty or a comment, we return a blank key/value pair. + if len(line) == 0 || line[0] == '#' { + return ``, ``, nil + } + + data := strings.SplitN(string(line), "=", 2) + key = data[0] + if errs := validation.IsCIdentifier(key); len(errs) != 0 { + return ``, ``, fmt.Errorf("%q is not a valid key name: %s", key, strings.Join(errs, ";")) + } + + if len(data) == 2 { + value = data[1] + } else { + // No value (no `=` in the line) is a signal to obtain the value + // from the environment. + value = os.Getenv(key) + } + return +} + // addFromEnvFile processes an env file allows a generic addTo to handle the // collection of key value pairs or returns an error. func addFromEnvFile(filePath string, addTo func(key, value string) error) error { @@ -39,38 +80,23 @@ func addFromEnvFile(filePath string, addTo func(key, value string) error) error scanner := bufio.NewScanner(f) currentLine := 0 - utf8bom := []byte{0xEF, 0xBB, 0xBF} for scanner.Scan() { + // Proccess the current line, retrieving a key/value pair if + // possible. scannedBytes := scanner.Bytes() - if !utf8.Valid(scannedBytes) { - return fmt.Errorf("env file %s contains invalid utf8 bytes at line %d: %v", filePath, currentLine+1, scannedBytes) + key, value, err := proccessEnvFileLine(scannedBytes, filePath, currentLine) + if err != nil { + return err } - // We trim UTF8 BOM - if currentLine == 0 { - scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom) - } - // trim the line from all leading whitespace first - line := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace) currentLine++ - // line is not empty, and not starting with '#' - if len(line) > 0 && !strings.HasPrefix(line, "#") { - data := strings.SplitN(line, "=", 2) - key := data[0] - if errs := validation.IsCIdentifier(key); len(errs) != 0 { - return fmt.Errorf("%q is not a valid key name: %s", key, strings.Join(errs, ";")) - } - value := "" - if len(data) > 1 { - // pass the value through, no trimming - value = data[1] - } else { - // a pass-through variable is given - value = os.Getenv(key) - } - if err = addTo(key, value); err != nil { - return err - } + if len(key) == 0 { + // no key means line was empty or a comment + continue + } + + if err = addTo(key, value); err != nil { + return err } } return nil diff --git a/pkg/kubectl/env_file_test.go b/pkg/kubectl/env_file_test.go new file mode 100644 index 00000000000..10e37238de2 --- /dev/null +++ b/pkg/kubectl/env_file_test.go @@ -0,0 +1,103 @@ +/* +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 kubectl + +import ( + "os" + "strings" + "testing" +) + +// Test the cases of proccessEnvFileLine that can be run without touching the +// environment. +func Test_processEnvFileLine(t *testing.T) { + testCases := []struct { + name string + line []byte + currentLine int + expectedKey string + expectedValue string + expectedErr string + }{ + {"the utf8bom is trimmed on the first line", + append(utf8bom, 'a', '=', 'c'), 0, "a", "c", ""}, + + {"the utf8bom is NOT trimmed on the second line", + append(utf8bom, 'a', '=', 'c'), 1, "", "", "not a valid key name"}, + + {"no key is returned on a comment line", + []byte{' ', '#', 'c'}, 0, "", "", ""}, + + {"no key is returned on a blank line", + []byte{' ', ' ', '\t'}, 0, "", "", ""}, + + {"key is returned even with no value", + []byte{' ', 'x', '='}, 0, "x", "", ""}, + } + for _, c := range testCases { + key, value, err := proccessEnvFileLine(c.line, `filename`, c.currentLine) + t.Logf("Testing that %s.", c.name) + if c.expectedKey != key { + t.Errorf("\texpected key %q, received %q", c.expectedKey, key) + } + if c.expectedValue != value { + t.Errorf("\texpected value %q, received %q", c.expectedValue, value) + } + if len(c.expectedErr) == 0 { + if err != nil { + t.Errorf("\tunexpected err %v", err) + } + } else { + if !strings.Contains(err.Error(), c.expectedErr) { + t.Errorf("\terr %v doesn't match expected %q", err, c.expectedErr) + } + } + } +} + +// proccessEnvFileLine needs to fetch the value from the environment if no +// equals sign is provided. +// For example: +// +// my_key1=alpha +// my_key2=beta +// my_key3 +// +// In this file, my_key3 must be fetched from the environment. +// Test this capability. +func Test_processEnvFileLine_readEnvironment(t *testing.T) { + const realKey = "k8s_test_env_file_key" + const realValue = `my_value` + + // Just in case, these two lines ensure the environment is restored to + // its original state. + original := os.Getenv(realKey) + defer func() { os.Setenv(realKey, original) }() + + os.Setenv(realKey, `my_value`) + + key, value, err := proccessEnvFileLine([]byte(realKey), `filename`, 3) + if err != nil { + t.Fatal(err) + } + if key != realKey { + t.Errorf(`expected key %q, received %q`, realKey, key) + } + if value != realValue { + t.Errorf(`expected value %q, received %q`, realValue, value) + } +} From ac793982b0a91291a9cd3163526d99edddf0dc10 Mon Sep 17 00:00:00 2001 From: Alexander Campbell Date: Tue, 30 May 2017 10:15:39 -0700 Subject: [PATCH 8/8] kubectl: fix inaccurate usage messages for --windows-line-endings Part of the problem is that these are duplicated between the different commands. I'm planning to consolidate these further. --- pkg/kubectl/cmd/apply_edit_last_applied.go | 5 +++-- pkg/kubectl/cmd/create.go | 5 +++-- pkg/kubectl/cmd/edit.go | 6 ++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/kubectl/cmd/apply_edit_last_applied.go b/pkg/kubectl/cmd/apply_edit_last_applied.go index fcf4201d005..519737e0d2e 100644 --- a/pkg/kubectl/cmd/apply_edit_last_applied.go +++ b/pkg/kubectl/cmd/apply_edit_last_applied.go @@ -18,7 +18,7 @@ package cmd import ( "io" - gruntime "runtime" + "runtime" "github.com/spf13/cobra" @@ -96,7 +96,8 @@ func NewCmdApplyEditLastApplied(f cmdutil.Factory, out, errOut io.Writer) *cobra usage := "to use to edit the resource" cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) cmd.Flags().StringVarP(&options.Output, "output", "o", "yaml", "Output format. One of: yaml|json.") - cmd.Flags().BoolVar(&options.WindowsLineEndings, "windows-line-endings", gruntime.GOOS == "windows", "Use Windows line-endings (default Unix line-endings)") + cmd.Flags().BoolVar(&options.WindowsLineEndings, "windows-line-endings", runtime.GOOS == "windows", + "Defaults to the line ending native to your platform.") cmdutil.AddRecordVarFlag(cmd, &options.Record) return cmd diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go index 7d942131b32..6e0c6c4e428 100644 --- a/pkg/kubectl/cmd/create.go +++ b/pkg/kubectl/cmd/create.go @@ -19,7 +19,7 @@ package cmd import ( "fmt" "io" - gruntime "runtime" + "runtime" "github.com/spf13/cobra" @@ -80,7 +80,8 @@ func NewCmdCreate(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command { cmdutil.AddValidateFlags(cmd) cmdutil.AddPrinterFlags(cmd) cmd.Flags().BoolVar(&options.EditBeforeCreate, "edit", false, "Edit the API resource before creating") - cmd.Flags().Bool("windows-line-endings", gruntime.GOOS == "windows", "Only relevant if --edit=true. Use Windows line-endings (default Unix line-endings)") + cmd.Flags().Bool("windows-line-endings", runtime.GOOS == "windows", + "Only relevant if --edit=true. Defaults to the line ending native to your platform.") cmdutil.AddApplyAnnotationFlags(cmd) cmdutil.AddRecordFlag(cmd) cmdutil.AddDryRunFlag(cmd) diff --git a/pkg/kubectl/cmd/edit.go b/pkg/kubectl/cmd/edit.go index fecb026fa30..64270828e1d 100644 --- a/pkg/kubectl/cmd/edit.go +++ b/pkg/kubectl/cmd/edit.go @@ -19,7 +19,7 @@ package cmd import ( "fmt" "io" - gruntime "runtime" + "runtime" "github.com/spf13/cobra" @@ -108,7 +108,9 @@ func NewCmdEdit(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command { cmdutil.AddValidateOptionFlags(cmd, &options.ValidateOptions) cmd.Flags().StringVarP(&options.Output, "output", "o", "yaml", "Output format. One of: yaml|json.") - cmd.Flags().BoolVar(&options.WindowsLineEndings, "windows-line-endings", gruntime.GOOS == "windows", "Use Windows line-endings (default Unix line-endings)") + cmd.Flags().BoolVar(&options.WindowsLineEndings, "windows-line-endings", runtime.GOOS == "windows", + "Defaults to the line ending native to your platform.") + cmdutil.AddApplyAnnotationVarFlags(cmd, &options.ApplyAnnotation) cmdutil.AddRecordVarFlag(cmd, &options.Record) cmdutil.AddInclude3rdPartyVarFlags(cmd, &options.Include3rdParty)