Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cleanup pkg/kubectl #46468

Merged
merged 8 commits into from
Jun 24, 2017
Merged

Cleanup pkg/kubectl #46468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d29560d
kubectl: rename Run() -> RunRun() to clarify purpose
alexandercampbell May 25, 2017
f9913c4
kubectl: rewrite docstrings in several files
alexandercampbell May 25, 2017
066dbb7
cmd: make createDeployment a private function
alexandercampbell May 25, 2017
01ae6ed
cmd: refactor common err expr into helper function
alexandercampbell May 25, 2017
ef9ae61
kubectl: simplify code with help of linter
alexandercampbell May 25, 2017
7b54199
kubectl: note a bug with a comment
alexandercampbell May 25, 2017
63e9c67
kubectl: refactor addFromEnvFile, write tests
alexandercampbell May 25, 2017
ac79398
kubectl: fix inaccurate usage messages for --windows-line-endings
alexandercampbell May 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/kubectl/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 1 addition & 5 deletions pkg/kubectl/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions pkg/kubectl/cmd/apply_edit_last_applied.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package cmd

import (
"io"
gruntime "runtime"
"runtime"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -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.")
Copy link
Member

@apelisse apelisse May 31, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree that this argument is mostly obvious, it feels weird that this reads the default, but not what behavior it actually controls.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow. Can you explain a little more?
I did not change any behavior here; I only deleted the part of the help message that was blatantly incorrect :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. You've removed "Use Windows line-endings". Now the description starts with "Default ..", which is correct, but it doesn't let you know what the parameter is actually about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The help message shows

--windows-line-endings=false: Defaults to the line ending native to your platform.

Which I feel is unambiguous.
Do you think it would be better to re-add the words "Use Windows line endings" to the beginning of the help message?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it's fine

cmdutil.AddRecordVarFlag(cmd, &options.Record)

return cmd
Expand Down
11 changes: 2 additions & 9 deletions pkg/kubectl/cmd/apply_set_last_applied.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 1 addition & 5 deletions pkg/kubectl/cmd/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 9 additions & 9 deletions pkg/kubectl/cmd/cp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,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.
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
# Copy /tmp/foo_dir local directory to /tmp/bar_dir in a remote pod in the default namespace
kubectl cp /tmp/foo_dir <some-pod>:/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 <some-pod>:/tmp/bar -c <specific-container>

# Copy /tmp/foo local file to /tmp/bar in a remote pod in namespace <some-namespace>
Expand All @@ -52,8 +52,8 @@ var (
kubectl cp <some-namespace>/<some-pod>:/tmp/foo /tmp/bar`))

cpUsageStr = dedent.Dedent(`
expected 'cp <file-spec-src> <file-spec-dest> [-c container]'.
<file-spec> is:
expected 'cp <file-spec-src> <file-spec-dest> [-c container]'.
<file-spec> is:
[namespace/]pod-name:/file/path for a remote file
/file/path for a local file`)
)
Expand All @@ -64,7 +64,7 @@ func NewCmdCp(f cmdutil.Factory, cmdOut, cmdErr io.Writer) *cobra.Command {
Use: "cp <file-spec-src> <file-spec-dest>",
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))
},
Expand Down
17 changes: 9 additions & 8 deletions pkg/kubectl/cmd/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package cmd
import (
"fmt"
"io"
gruntime "runtime"
"runtime"

"github.com/spf13/cobra"

Expand All @@ -39,12 +39,12 @@ type CreateOptions struct {
}

var (
create_long = templates.LongDesc(i18n.T(`
Create a resource by filename or stdin.
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

Expand All @@ -60,9 +60,9 @@ 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"),
Long: create_long,
Example: create_example,
Short: i18n.T("Create a resource from a file or from stdin."),
Long: createLong,
Example: createExample,
Run: func(cmd *cobra.Command, args []string) {
if cmdutil.IsFilenameEmpty(options.FilenameOptions.Filenames) {
defaultRunFunc := cmdutil.DefaultSubCommandRun(errOut)
Expand All @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions pkg/kubectl/cmd/create_clusterrolebinding.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cmd

import (
"fmt"
"io"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions pkg/kubectl/cmd/create_configmap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cmd

import (
"fmt"
"io"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions pkg/kubectl/cmd/create_deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]",
Expand All @@ -47,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)
},
}
Expand All @@ -60,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
Expand Down Expand Up @@ -91,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,
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubectl/cmd/create_deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
3 changes: 1 addition & 2 deletions pkg/kubectl/cmd/create_namespace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cmd

import (
"fmt"
"io"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions pkg/kubectl/cmd/create_pdb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cmd

import (
"fmt"
"io"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions pkg/kubectl/cmd/create_quota.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cmd

import (
"fmt"
"io"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions pkg/kubectl/cmd/create_rolebinding.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cmd

import (
"fmt"
"io"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 3 additions & 4 deletions pkg/kubectl/cmd/create_secret.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package cmd

import (
"fmt"
"io"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down