From 68c46e7f52ec3b5497aebb589c99b2ebe92300ec Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 25 Mar 2015 21:52:12 -0400 Subject: [PATCH] `kubectl label` should support resource builder Allow applying labels to all resources, by existing selector, and soon allow multiple selection. --- docs/kubectl-label.md | 7 +- docs/man/man1/kubectl-label.1 | 13 +++- hack/test-cmd.sh | 4 +- pkg/kubectl/cmd/cmd_test.go | 3 +- pkg/kubectl/cmd/create_test.go | 5 +- pkg/kubectl/cmd/delete_test.go | 2 +- pkg/kubectl/cmd/describe_test.go | 2 +- pkg/kubectl/cmd/get_test.go | 2 +- pkg/kubectl/cmd/label.go | 112 +++++++++++++++++++------------ pkg/kubectl/cmd/label_test.go | 107 +++++++++++++++++++++++++++++ pkg/kubectl/cmd/update_test.go | 2 +- 11 files changed, 203 insertions(+), 56 deletions(-) diff --git a/docs/kubectl-label.md b/docs/kubectl-label.md index 41f2d3e6d7f..0c7824aedf9 100644 --- a/docs/kubectl-label.md +++ b/docs/kubectl-label.md @@ -23,6 +23,9 @@ $ kubectl label pods foo unhealthy=true // Update pod 'foo' with the label 'status' and the value 'unhealthy', overwriting any existing value. $ kubectl label --overwrite pods foo status=unhealthy +// Update all pods in the namespace +$ kubectl label pods --all status=unhealthy + // Update pod 'foo' only if the resource is unchanged from version 1. $ kubectl label pods foo status=unhealthy --resource-version=1 @@ -34,12 +37,14 @@ $ kubectl label pods foo bar- ### Options ``` + --all=false: select all resources in the namespace of the specified resource types -h, --help=false: help for label --no-headers=false: When using the default output, don't print headers. -o, --output="": Output format. One of: json|yaml|template|templatefile. --output-version="": Output the formatted object with the given version (default api-version). --overwrite=false: If true, allow labels to be overwritten, otherwise reject label updates that overwrite existing labels. - --resource-version="": If non-empty, the labels update will only succeed if this is the current resource-version for the object. + --resource-version="": If non-empty, the labels update will only succeed if this is the current resource-version for the object. Only valid when specifying a single resource. + -l, --selector="": Selector (label query) to filter on -t, --template="": Template string or path to template file to use when -o=template or -o=templatefile. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview] ``` diff --git a/docs/man/man1/kubectl-label.1 b/docs/man/man1/kubectl-label.1 index 704ac370dc7..192f087d183 100644 --- a/docs/man/man1/kubectl-label.1 +++ b/docs/man/man1/kubectl-label.1 @@ -21,6 +21,10 @@ If \-\-resource\-version is specified, then updates will use this resource versi .SH OPTIONS +.PP +\fB\-\-all\fP=false + select all resources in the namespace of the specified resource types + .PP \fB\-h\fP, \fB\-\-help\fP=false help for label @@ -43,7 +47,11 @@ If \-\-resource\-version is specified, then updates will use this resource versi .PP \fB\-\-resource\-version\fP="" - If non\-empty, the labels update will only succeed if this is the current resource\-version for the object. + If non\-empty, the labels update will only succeed if this is the current resource\-version for the object. Only valid when specifying a single resource. + +.PP +\fB\-l\fP, \fB\-\-selector\fP="" + Selector (label query) to filter on .PP \fB\-t\fP, \fB\-\-template\fP="" @@ -164,6 +172,9 @@ $ kubectl label pods foo unhealthy=true // Update pod 'foo' with the label 'status' and the value 'unhealthy', overwriting any existing value. $ kubectl label \-\-overwrite pods foo status=unhealthy +// Update all pods in the namespace +$ kubectl label pods \-\-all status=unhealthy + // Update pod 'foo' only if the resource is unchanged from version 1. $ kubectl label pods foo status=unhealthy \-\-resource\-version=1 diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 0126a4f91ca..ab29dcc0497 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -307,11 +307,11 @@ for version in "${kube_api_versions[@]}"; do # Post-condition: name is still valid-pod kube::test::get_object_assert 'pod valid-pod' "{{.${labels_field}.name}}" 'valid-pod' - ### --overwrite must be used to overwrite existing label + ### --overwrite must be used to overwrite existing label, can be applied to all resources # Pre-condition: name is valid-pod kube::test::get_object_assert 'pod valid-pod' "{{.${labels_field}.name}}" 'valid-pod' # Command - kubectl label --overwrite pods valid-pod name=valid-pod-super-sayan "${kube_flags[@]}" + kubectl label --overwrite pods --all name=valid-pod-super-sayan "${kube_flags[@]}" # Post-condition: name is valid-pod-super-sayan kube::test::get_object_assert 'pod valid-pod' "{{.${labels_field}.name}}" 'valid-pod-super-sayan' diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index 410ec490142..be62b94bc3a 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/pkg/kubectl/cmd/cmd_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cmd_test +package cmd import ( "bytes" @@ -30,7 +30,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl" - . "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/cmd" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/resource" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" ) diff --git a/pkg/kubectl/cmd/create_test.go b/pkg/kubectl/cmd/create_test.go index 05a5e662315..70d0735f011 100644 --- a/pkg/kubectl/cmd/create_test.go +++ b/pkg/kubectl/cmd/create_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cmd_test +package cmd import ( "bytes" @@ -22,7 +22,6 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" - "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/cmd" ) func TestExtraArgsFail(t *testing.T) { @@ -30,7 +29,7 @@ func TestExtraArgsFail(t *testing.T) { f, _, _ := NewAPIFactory() c := f.NewCmdCreate(buf) - if cmd.ValidateArgs(c, []string{"rc"}) == nil { + if ValidateArgs(c, []string{"rc"}) == nil { t.Errorf("unexpected non-error") } } diff --git a/pkg/kubectl/cmd/delete_test.go b/pkg/kubectl/cmd/delete_test.go index 28309050e60..092085c08ef 100644 --- a/pkg/kubectl/cmd/delete_test.go +++ b/pkg/kubectl/cmd/delete_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cmd_test +package cmd import ( "bytes" diff --git a/pkg/kubectl/cmd/describe_test.go b/pkg/kubectl/cmd/describe_test.go index 994a38e0f80..f736073f764 100644 --- a/pkg/kubectl/cmd/describe_test.go +++ b/pkg/kubectl/cmd/describe_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cmd_test +package cmd import ( "bytes" diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index d3bb8925930..c868868c870 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cmd_test +package cmd import ( "bytes" diff --git a/pkg/kubectl/cmd/label.go b/pkg/kubectl/cmd/label.go index b55747be89e..e02bada9bd6 100644 --- a/pkg/kubectl/cmd/label.go +++ b/pkg/kubectl/cmd/label.go @@ -22,7 +22,6 @@ import ( "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/cmd/util" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/resource" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" @@ -40,6 +39,9 @@ $ kubectl label pods foo unhealthy=true // Update pod 'foo' with the label 'status' and the value 'unhealthy', overwriting any existing value. $ kubectl label --overwrite pods foo status=unhealthy +// Update all pods in the namespace +$ kubectl label pods --all status=unhealthy + // Update pod 'foo' only if the resource is unchanged from version 1. $ kubectl label pods foo status=unhealthy --resource-version=1 @@ -61,29 +63,25 @@ func (f *Factory) NewCmdLabel(out io.Writer) *cobra.Command { } util.AddPrinterFlags(cmd) cmd.Flags().Bool("overwrite", false, "If true, allow labels to be overwritten, otherwise reject label updates that overwrite existing labels.") - cmd.Flags().String("resource-version", "", "If non-empty, the labels update will only succeed if this is the current resource-version for the object.") + cmd.Flags().StringP("selector", "l", "", "Selector (label query) to filter on") + cmd.Flags().Bool("all", false, "select all resources in the namespace of the specified resource types") + cmd.Flags().String("resource-version", "", "If non-empty, the labels update will only succeed if this is the current resource-version for the object. Only valid when specifying a single resource.") return cmd } -func updateObject(client resource.RESTClient, mapping *meta.RESTMapping, namespace, name string, updateFn func(runtime.Object) (runtime.Object, error)) (runtime.Object, error) { - helper := resource.NewHelper(client, mapping) +func updateObject(info *resource.Info, updateFn func(runtime.Object) (runtime.Object, error)) (runtime.Object, error) { + helper := resource.NewHelper(info.Client, info.Mapping) - obj, err := helper.Get(namespace, name) + obj, err := updateFn(info.Object) if err != nil { return nil, err } - - obj, err = updateFn(obj) - if err != nil { - return nil, err - } - data, err := helper.Codec.Encode(obj) if err != nil { return nil, err } - _, err = helper.Update(namespace, name, true, data) + _, err = helper.Update(info.Namespace, info.Name, true, data) if err != nil { return nil, err } @@ -152,52 +150,80 @@ func labelFunc(obj runtime.Object, overwrite bool, resourceVersion string, label } func RunLabel(f *Factory, out io.Writer, cmd *cobra.Command, args []string) error { - if len(args) < 2 { - return util.UsageError(cmd, " is required") + resources, labelArgs := []string{}, []string{} + first := true + for _, s := range args { + isLabel := strings.Contains(s, "=") || strings.HasSuffix(s, "-") + switch { + case first && isLabel: + first = false + fallthrough + case !first && isLabel: + labelArgs = append(labelArgs, s) + case first && !isLabel: + resources = append(resources, s) + case !first && !isLabel: + return util.UsageError(cmd, "all resources must be specified before label changes: %s", s) + } } - if len(args) < 3 { - return util.UsageError(cmd, "at least one label update is required.") + if len(resources) < 1 { + return util.UsageError(cmd, "one or more resources must be specified as or /") } - res := args[:2] + if len(labelArgs) < 1 { + return util.UsageError(cmd, "at least one label update is required") + } + + selector := util.GetFlagString(cmd, "selector") + all := util.GetFlagBool(cmd, "all") + overwrite := util.GetFlagBool(cmd, "overwrite") + resourceVersion := util.GetFlagString(cmd, "resource-version") + cmdNamespace, err := f.DefaultNamespace() if err != nil { return err } - mapper, _ := f.Object() - // TODO: use resource.Builder instead - mapping, namespace, name, err := util.ResourceFromArgs(cmd, res, mapper, cmdNamespace) - if err != nil { - return err - } - client, err := f.RESTClient(mapping) + labels, remove, err := parseLabels(labelArgs) if err != nil { return err } - labels, remove, err := parseLabels(args[2:]) - if err != nil { + mapper, typer := f.Object() + b := resource.NewBuilder(mapper, typer, f.ClientMapperForCommand(cmd)). + ContinueOnError(). + NamespaceParam(cmdNamespace).DefaultNamespace(). + SelectorParam(selector). + ResourceTypeOrNameArgs(all, resources...). + Flatten(). + Latest() + + one := false + r := b.Do().IntoSingular(&one) + if err := r.Err(); err != nil { return err } - overwrite := util.GetFlagBool(cmd, "overwrite") - resourceVersion := util.GetFlagString(cmd, "resource-version") + // only apply resource version locking on a single resource + if !one && len(resourceVersion) > 0 { + return util.UsageError(cmd, "--resource-version may only be used with a single resource") + } - obj, err := updateObject(client, mapping, namespace, name, func(obj runtime.Object) (runtime.Object, error) { - outObj, err := labelFunc(obj, overwrite, resourceVersion, labels, remove) + // TODO: support bulk generic output a la Get + return r.Visit(func(info *resource.Info) error { + obj, err := updateObject(info, func(obj runtime.Object) (runtime.Object, error) { + outObj, err := labelFunc(obj, overwrite, resourceVersion, labels, remove) + if err != nil { + return nil, err + } + return outObj, nil + }) if err != nil { - return nil, err + return err } - return outObj, nil + + printer, err := f.PrinterForMapping(cmd, info.Mapping) + if err != nil { + return err + } + return printer.PrintObj(obj, out) }) - if err != nil { - return err - } - - printer, err := f.PrinterForMapping(cmd, mapping) - if err != nil { - return err - } - - printer.PrintObj(obj, out) - return nil } diff --git a/pkg/kubectl/cmd/label_test.go b/pkg/kubectl/cmd/label_test.go index ea109b47ae2..1933ba17280 100644 --- a/pkg/kubectl/cmd/label_test.go +++ b/pkg/kubectl/cmd/label_test.go @@ -17,10 +17,14 @@ limitations under the License. package cmd import ( + "bytes" + "net/http" "reflect" + "strings" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" ) @@ -258,3 +262,106 @@ func TestLabelFunc(t *testing.T) { } } } + +func TestLabelErrors(t *testing.T) { + testCases := map[string]struct { + args []string + flags map[string]string + errFn func(error) bool + }{ + "no args": { + args: []string{}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "one or more resources must be specified") }, + }, + "not enough labels": { + args: []string{"pods"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "at least one label update is required") }, + }, + "no resources": { + args: []string{"pods-"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "one or more resources must be specified") }, + }, + "no resources 2": { + args: []string{"pods=bar"}, + errFn: func(err error) bool { return strings.Contains(err.Error(), "one or more resources must be specified") }, + }, + } + + for k, testCase := range testCases { + f, tf, _ := NewAPIFactory() + tf.Printer = &testPrinter{} + tf.Namespace = "test" + tf.ClientConfig = &client.Config{Version: "v1beta1"} + + buf := bytes.NewBuffer([]byte{}) + cmd := f.NewCmdLabel(buf) + cmd.SetOutput(buf) + + for k, v := range testCase.flags { + cmd.Flags().Set(k, v) + } + err := RunLabel(f, buf, cmd, testCase.args) + if !testCase.errFn(err) { + t.Errorf("%s: unexpected error: %v", k, err) + continue + } + if tf.Printer.(*testPrinter).Objects != nil { + t.Errorf("unexpected print to default printer") + } + if buf.Len() > 0 { + t.Errorf("buffer should be empty: %s", string(buf.Bytes())) + } + } +} + +func TestLabelMultipleObjects(t *testing.T) { + pods, _, _ := testData() + + f, tf, codec := NewAPIFactory() + tf.Printer = &testPrinter{} + tf.Client = &client.FakeRESTClient{ + Codec: codec, + Client: client.HTTPClientFunc(func(req *http.Request) (*http.Response, error) { + switch req.Method { + case "GET": + switch req.URL.Path { + case "/namespaces/test/pods": + return &http.Response{StatusCode: 200, Body: objBody(codec, pods)}, nil + default: + t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) + return nil, nil + } + case "PUT": + switch req.URL.Path { + case "/namespaces/test/pods/foo": + return &http.Response{StatusCode: 200, Body: objBody(codec, &pods.Items[0])}, nil + case "/namespaces/test/pods/bar": + return &http.Response{StatusCode: 200, Body: objBody(codec, &pods.Items[1])}, nil + default: + t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) + return nil, nil + } + default: + t.Fatalf("unexpected request: %s %#v\n%#v", req.Method, req.URL, req) + return nil, nil + } + }), + } + tf.Namespace = "test" + tf.ClientConfig = &client.Config{Version: "v1beta1"} + buf := bytes.NewBuffer([]byte{}) + + cmd := f.NewCmdLabel(buf) + + cmd.Flags().Set("all", "true") + if err := RunLabel(f, buf, cmd, []string{"pods", "a=b"}); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if tf.Printer.(*testPrinter).Objects == nil { + t.Errorf("unexpected non print to default printer") + } + if !reflect.DeepEqual(tf.Printer.(*testPrinter).Objects[0].(*api.Pod).Labels, map[string]string{"a": "b"}) { + t.Errorf("did not set labels: %#v", string(buf.Bytes())) + } +} diff --git a/pkg/kubectl/cmd/update_test.go b/pkg/kubectl/cmd/update_test.go index 0986b6b057b..cb14c00b043 100644 --- a/pkg/kubectl/cmd/update_test.go +++ b/pkg/kubectl/cmd/update_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cmd_test +package cmd import ( "bytes"