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.

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

Improvements to CLI usability and maintainability #34502

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Godeps/Godeps.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 58 additions & 0 deletions Godeps/LICENSES

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

48 changes: 48 additions & 0 deletions cmd/clicheck/check_cli_conventions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
Copyright 2016 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 main

import (
"fmt"
"io/ioutil"
"os"

"k8s.io/kubernetes/pkg/kubectl/cmd"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
cmdsanity "k8s.io/kubernetes/pkg/kubectl/cmd/util/sanity"
)

var (
skip = []string{}
)

func main() {
errors := []error{}

kubectl := cmd.NewKubectlCommand(cmdutil.NewFactory(nil), os.Stdin, ioutil.Discard, ioutil.Discard)
result := cmdsanity.CheckCmdTree(kubectl, cmdsanity.AllCmdChecks, []string{})
errors = append(errors, result...)

if len(errors) > 0 {
for i, err := range errors {
fmt.Fprintf(os.Stderr, "%d. %s\n\n", i+1, err)
}
os.Exit(1)
}

fmt.Fprintln(os.Stdout, "Congrats, CLI looks good!")
}
27 changes: 14 additions & 13 deletions docs/devel/kubectl-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -301,24 +301,25 @@ Sample command skeleton:
// MineRecommendedName is the recommended command name for kubectl mine.
const MineRecommendedName = "mine"

// MineConfig contains all the options for running the mine cli command.
type MineConfig struct {
mineLatest bool
}

// Long command description and examples.
var (
mineLong = dedent.Dedent(`
mine which is described here
with lots of details.`)
mineLong = templates.LongDesc(`
mine which is described here
with lots of details.`)

mineExample = dedent.Dedent(`
# Run my command's first action
kubectl mine first_action
mineExample = templates.Examples(`
# Run my command's first action
kubectl mine first_action
# Run my command's second action on latest stuff
kubectl mine second_action --flag`)
# Run my command's second action on latest stuff
kubectl mine second_action --flag`)
)

// MineConfig contains all the options for running the mine cli command.
type MineConfig struct {
mineLatest bool
}

// NewCmdMine implements the kubectl mine command.
func NewCmdMine(parent, name string, f *cmdutil.Factory, out io.Writer) *cobra.Command {
opts := &MineConfig{}
Expand Down
1 change: 1 addition & 0 deletions hack/.linted_packages
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
cluster/addons/fluentd-elasticsearch/es-image
cluster/images/etcd/attachlease
cluster/images/etcd/rollback
cmd/clicheck
cmd/gendocs
cmd/genkubedocs
cmd/genman
Expand Down
40 changes: 40 additions & 0 deletions hack/verify-cli-conventions.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/bin/bash

# Copyright 2016 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.

set -o errexit
set -o nounset
set -o pipefail

KUBE_ROOT=$(dirname "${BASH_SOURCE}")/..
source "${KUBE_ROOT}/hack/lib/init.sh"

Choose a reason for hiding this comment

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

whether we should add this check to normal verify process? like check this when commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it does it already. verify runs everything that satisfies hack/verify-*.sh, which includes this one.

https://github.com/fabianofranz/kubernetes/blob/001e58e83a82f881b4a753f33d08908d885aaa2e/hack/make-rules/verify.sh#L90


kube::golang::setup_env

BINS=(
cmd/clicheck
)
make -C "${KUBE_ROOT}" WHAT="${BINS[*]}"

clicheck=$(kube::util::find-binary "clicheck")

if ! output=`$clicheck 2>&1`
then
echo "FAILURE: CLI is not following one or more required conventions:"
echo "$output"
exit 1
else
echo "SUCCESS: CLI is following all tested conventions."
fi
44 changes: 22 additions & 22 deletions pkg/kubectl/cmd/annotate.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ import (
"io"

"github.com/golang/glog"
"github.com/renstrom/dedent"
"github.com/spf13/cobra"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/meta"
"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/resource"
"k8s.io/kubernetes/pkg/runtime"
Expand Down Expand Up @@ -59,36 +59,36 @@ type AnnotateOptions struct {
}

var (
annotate_long = dedent.Dedent(`
annotate_long = templates.LongDesc(`
Update the annotations on one or more resources.

An annotation is a key/value pair that can hold larger (compared to a label), and possibly not human-readable, data.
It is intended to store non-identifying auxiliary data, especially data manipulated by tools and system extensions.
If --overwrite is true, then existing annotations can be overwritten, otherwise attempting to overwrite an annotation will result in an error.
If --resource-version is specified, then updates will use this resource version, otherwise the existing resource-version will be used.
* An annotation is a key/value pair that can hold larger (compared to a label), and possibly not human-readable, data.

Choose a reason for hiding this comment

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

why add * here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's list items. Not strictly required, but if we don't use list items here we have to either give each one its own paragraph, or make it all part of a single paragraph (no line wrapping). Markdown unwraps single line breaks.

* It is intended to store non-identifying auxiliary data, especially data manipulated by tools and system extensions.
* If --overwrite is true, then existing annotations can be overwritten, otherwise attempting to overwrite an annotation will result in an error.
* If --resource-version is specified, then updates will use this resource version, otherwise the existing resource-version will be used.

`) + valid_resources
` + valid_resources)

annotate_example = dedent.Dedent(`
# Update pod 'foo' with the annotation 'description' and the value 'my frontend'.
# If the same annotation is set multiple times, only the last value will be applied
kubectl annotate pods foo description='my frontend'
annotate_example = templates.Examples(`
# Update pod 'foo' with the annotation 'description' and the value 'my frontend'.
# If the same annotation is set multiple times, only the last value will be applied
kubectl annotate pods foo description='my frontend'

# Update a pod identified by type and name in "pod.json"
kubectl annotate -f pod.json description='my frontend'
# Update a pod identified by type and name in "pod.json"
kubectl annotate -f pod.json description='my frontend'

# Update pod 'foo' with the annotation 'description' and the value 'my frontend running nginx', overwriting any existing value.
kubectl annotate --overwrite pods foo description='my frontend running nginx'
# Update pod 'foo' with the annotation 'description' and the value 'my frontend running nginx', overwriting any existing value.
kubectl annotate --overwrite pods foo description='my frontend running nginx'

# Update all pods in the namespace
kubectl annotate pods --all description='my frontend running nginx'
# Update all pods in the namespace
kubectl annotate pods --all description='my frontend running nginx'

# Update pod 'foo' only if the resource is unchanged from version 1.
kubectl annotate pods foo description='my frontend running nginx' --resource-version=1
# Update pod 'foo' only if the resource is unchanged from version 1.
kubectl annotate pods foo description='my frontend running nginx' --resource-version=1

# Update pod 'foo' by removing an annotation named 'description' if it exists.
# Does not require the --overwrite flag.
kubectl annotate pods foo description-`)
# Update pod 'foo' by removing an annotation named 'description' if it exists.
# Does not require the --overwrite flag.
kubectl annotate pods foo description-`)
)

func NewCmdAnnotate(f cmdutil.Factory, out io.Writer) *cobra.Command {
Expand Down
8 changes: 4 additions & 4 deletions pkg/kubectl/cmd/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"time"

"github.com/jonboulle/clockwork"
"github.com/renstrom/dedent"
"github.com/spf13/cobra"

"k8s.io/kubernetes/pkg/api"
Expand All @@ -32,6 +31,7 @@ import (
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/resource"
"k8s.io/kubernetes/pkg/labels"
Expand All @@ -58,16 +58,16 @@ const (
)

var (
apply_long = dedent.Dedent(`
apply_long = templates.LongDesc(`
Apply a configuration to a resource by filename or stdin.
This resource will be created if it doesn't exist yet.
To use 'apply', always create the resource initially with either 'apply' or 'create --save-config'.

JSON and YAML formats are accepted.

Alpha Disclaimer: the --prune functionality is not yet complete. Do not use unless you are aware of what the current state is. See https://issues.k8s.io/34274.`)

apply_example = dedent.Dedent(`
apply_example = templates.Examples(`
# Apply the configuration in pod.json to a pod.
kubectl apply -f ./pod.json

Expand Down
4 changes: 2 additions & 2 deletions pkg/kubectl/cmd/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,21 @@ import (
"net/url"

"github.com/golang/glog"
"github.com/renstrom/dedent"
"github.com/spf13/cobra"

"k8s.io/kubernetes/pkg/api"
coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/unversioned"
"k8s.io/kubernetes/pkg/client/restclient"
"k8s.io/kubernetes/pkg/client/unversioned/remotecommand"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
remotecommandserver "k8s.io/kubernetes/pkg/kubelet/server/remotecommand"
utilerrors "k8s.io/kubernetes/pkg/util/errors"
"k8s.io/kubernetes/pkg/util/term"
)

var (
attach_example = dedent.Dedent(`
attach_example = templates.Examples(`
# Get output from running pod 123456-7890, using the first container by default
kubectl attach 123456-7890

Expand Down
7 changes: 3 additions & 4 deletions pkg/kubectl/cmd/autoscale.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ import (
"fmt"
"io"

"github.com/renstrom/dedent"

"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/resource"
utilerrors "k8s.io/kubernetes/pkg/util/errors"
Expand All @@ -31,13 +30,13 @@ import (
)

var (
autoscaleLong = dedent.Dedent(`
autoscaleLong = templates.LongDesc(`
Creates an autoscaler that automatically chooses and sets the number of pods that run in a kubernetes cluster.

Looks up a Deployment, ReplicaSet, or ReplicationController by name and creates an autoscaler that uses the given resource as a reference.
An autoscaler can automatically increase or decrease number of pods deployed within the system as needed.`)

autoscaleExample = dedent.Dedent(`
autoscaleExample = templates.Examples(`
# Auto scale a deployment "foo", with the number of pods between 2 and 10, target CPU utilization specified so a default autoscaling policy will be used:
kubectl autoscale deployment foo --min=2 --max=10

Expand Down
Loading