kubectl label should support resource builder

Allow applying labels to all resources, by existing selector, and
soon allow multiple selection.

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]
 ```
 

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
 

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'
 

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"
 )

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")
 	}
 }

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"

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"

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"

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) {
+func updateObject(info *resource.Info, updateFn func(runtime.Object) (runtime.Object, error)) (runtime.Object, error) {
-	helper := resource.NewHelper(client, mapping)
+	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,37 +150,66 @@ 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 {
+	resources, labelArgs := []string{}, []string{}
-		return util.UsageError(cmd, "<resource> <name> is required")
+	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.")
 	}
-	res := args[:2]
+	if len(resources) < 1 {
+		return util.UsageError(cmd, "one or more resources must be specified as <resource> <name> or <resource>/<name>")
+	}
+	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()
+	labels, remove, err := parseLabels(labelArgs)
-	// 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)
 	if err != nil {
 		return err
 	}
 
-	labels, remove, err := parseLabels(args[2:])
+	mapper, typer := f.Object()
-	if err != nil {
+	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")
+	// only apply resource version locking on a single resource
-	resourceVersion := util.GetFlagString(cmd, "resource-version")
+	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) {
+	// 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
@@ -193,11 +220,10 @@ func RunLabel(f *Factory, out io.Writer, cmd *cobra.Command, args []string) erro
 			return err
 		}
 
-	printer, err := f.PrinterForMapping(cmd, mapping)
+		printer, err := f.PrinterForMapping(cmd, info.Mapping)
 		if err != nil {
 			return err
 		}
-
+		return printer.PrintObj(obj, out)
-	printer.PrintObj(obj, out)
+	})
-	return nil
 }

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()))
+	}
+}

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"