From 36987e9dce808e2db4c43c3f3f5eea4d2bde84ff Mon Sep 17 00:00:00 2001 From: hurf Date: Sun, 16 Aug 2015 17:33:35 +0800 Subject: [PATCH 1/2] Move formatLabels function to labels package As the TODO above the function instructed, move formatLables function to labels package. --- pkg/kubectl/describe.go | 24 ++++++++++++------------ pkg/kubectl/kubectl.go | 10 ---------- pkg/kubectl/resource_printer.go | 15 ++++++++------- pkg/labels/labels.go | 9 +++++++++ 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/pkg/kubectl/describe.go b/pkg/kubectl/describe.go index bc8d07d6dd2..37f520d28b3 100644 --- a/pkg/kubectl/describe.go +++ b/pkg/kubectl/describe.go @@ -155,7 +155,7 @@ func (d *NamespaceDescriber) Describe(namespace, name string) (string, error) { func describeNamespace(namespace *api.Namespace, resourceQuotaList *api.ResourceQuotaList, limitRangeList *api.LimitRangeList) (string, error) { return tabbedString(func(out io.Writer) error { fmt.Fprintf(out, "Name:\t%s\n", namespace.Name) - fmt.Fprintf(out, "Labels:\t%s\n", formatLabels(namespace.Labels)) + fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(namespace.Labels)) fmt.Fprintf(out, "Status:\t%s\n", string(namespace.Status.Phase)) if resourceQuotaList != nil { fmt.Fprintf(out, "\n") @@ -416,7 +416,7 @@ func describePod(pod *api.Pod, rcs []api.ReplicationController, events *api.Even fmt.Fprintf(out, "Namespace:\t%s\n", pod.Namespace) fmt.Fprintf(out, "Image(s):\t%s\n", makeImageList(&pod.Spec)) fmt.Fprintf(out, "Node:\t%s\n", pod.Spec.NodeName+"/"+pod.Status.HostIP) - fmt.Fprintf(out, "Labels:\t%s\n", formatLabels(pod.Labels)) + fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(pod.Labels)) if pod.DeletionTimestamp != nil { fmt.Fprintf(out, "Status:\tTerminating (expires %s)\n", pod.DeletionTimestamp.Time.Format(time.RFC1123Z)) fmt.Fprintf(out, "Termination Grace Period:\t%ds\n", pod.DeletionGracePeriodSeconds) @@ -584,7 +584,7 @@ func (d *PersistentVolumeDescriber) Describe(namespace, name string) (string, er return tabbedString(func(out io.Writer) error { fmt.Fprintf(out, "Name:\t%s\n", pv.Name) - fmt.Fprintf(out, "Labels:\t%s\n", formatLabels(pv.Labels)) + fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(pv.Labels)) fmt.Fprintf(out, "Status:\t%s\n", pv.Status.Phase) if pv.Spec.ClaimRef != nil { fmt.Fprintf(out, "Claim:\t%s\n", pv.Spec.ClaimRef.Namespace+"/"+pv.Spec.ClaimRef.Name) @@ -611,7 +611,7 @@ func (d *PersistentVolumeClaimDescriber) Describe(namespace, name string) (strin return "", err } - labels := formatLabels(pvc.Labels) + labels := labels.FormatLabels(pvc.Labels) storage := pvc.Spec.Resources.Requests[api.ResourceStorage] capacity := "" accessModes := "" @@ -756,8 +756,8 @@ func describeReplicationController(controller *api.ReplicationController, events } else { fmt.Fprintf(out, "Image(s):\t%s\n", "") } - fmt.Fprintf(out, "Selector:\t%s\n", formatLabels(controller.Spec.Selector)) - fmt.Fprintf(out, "Labels:\t%s\n", formatLabels(controller.Labels)) + fmt.Fprintf(out, "Selector:\t%s\n", labels.FormatLabels(controller.Spec.Selector)) + fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(controller.Labels)) fmt.Fprintf(out, "Replicas:\t%d current / %d desired\n", controller.Status.Replicas, controller.Spec.Replicas) fmt.Fprintf(out, "Pods Status:\t%d Running / %d Waiting / %d Succeeded / %d Failed\n", running, waiting, succeeded, failed) if controller.Spec.Template != nil { @@ -790,8 +790,8 @@ func describeSecret(secret *api.Secret) (string, error) { return tabbedString(func(out io.Writer) error { fmt.Fprintf(out, "Name:\t%s\n", secret.Name) fmt.Fprintf(out, "Namespace:\t%s\n", secret.Namespace) - fmt.Fprintf(out, "Labels:\t%s\n", formatLabels(secret.Labels)) - fmt.Fprintf(out, "Annotations:\t%s\n", formatLabels(secret.Annotations)) + fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(secret.Labels)) + fmt.Fprintf(out, "Annotations:\t%s\n", labels.FormatLabels(secret.Annotations)) fmt.Fprintf(out, "\nType:\t%s\n", secret.Type) @@ -851,8 +851,8 @@ func describeService(service *api.Service, endpoints *api.Endpoints, events *api return tabbedString(func(out io.Writer) error { fmt.Fprintf(out, "Name:\t%s\n", service.Name) fmt.Fprintf(out, "Namespace:\t%s\n", service.Namespace) - fmt.Fprintf(out, "Labels:\t%s\n", formatLabels(service.Labels)) - fmt.Fprintf(out, "Selector:\t%s\n", formatLabels(service.Spec.Selector)) + fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(service.Labels)) + fmt.Fprintf(out, "Selector:\t%s\n", labels.FormatLabels(service.Spec.Selector)) fmt.Fprintf(out, "Type:\t%s\n", service.Spec.Type) fmt.Fprintf(out, "IP:\t%s\n", service.Spec.ClusterIP) if len(service.Status.LoadBalancer.Ingress) > 0 { @@ -914,7 +914,7 @@ func describeServiceAccount(serviceAccount *api.ServiceAccount, tokens []api.Sec return tabbedString(func(out io.Writer) error { fmt.Fprintf(out, "Name:\t%s\n", serviceAccount.Name) fmt.Fprintf(out, "Namespace:\t%s\n", serviceAccount.Namespace) - fmt.Fprintf(out, "Labels:\t%s\n", formatLabels(serviceAccount.Labels)) + fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(serviceAccount.Labels)) fmt.Fprintln(out) var ( @@ -1000,7 +1000,7 @@ func (d *NodeDescriber) Describe(namespace, name string) (string, error) { func describeNode(node *api.Node, pods []*api.Pod, events *api.EventList) (string, error) { return tabbedString(func(out io.Writer) error { fmt.Fprintf(out, "Name:\t%s\n", node.Name) - fmt.Fprintf(out, "Labels:\t%s\n", formatLabels(node.Labels)) + fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(node.Labels)) fmt.Fprintf(out, "CreationTimestamp:\t%s\n", node.CreationTimestamp.Time.Format(time.RFC1123Z)) if len(node.Status.Conditions) > 0 { fmt.Fprint(out, "Conditions:\n Type\tStatus\tLastHeartbeatTime\tLastTransitionTime\tReason\tMessage\n") diff --git a/pkg/kubectl/kubectl.go b/pkg/kubectl/kubectl.go index 537e9d52e40..8e2d16718e3 100644 --- a/pkg/kubectl/kubectl.go +++ b/pkg/kubectl/kubectl.go @@ -23,7 +23,6 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/meta" - "k8s.io/kubernetes/pkg/labels" ) const kubectlAnnotationPrefix = "kubectl.kubernetes.io/" @@ -32,15 +31,6 @@ type NamespaceInfo struct { Namespace string } -// TODO Move to labels package. -func formatLabels(labelMap map[string]string) string { - l := labels.Set(labelMap).String() - if l == "" { - l = "" - } - return l -} - func listOfImages(spec *api.PodSpec) []string { var images []string for _, container := range spec.Containers { diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index 07337797305..943f0932e6a 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -34,6 +34,7 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/conversion" "k8s.io/kubernetes/pkg/expapi" + "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/util/jsonpath" @@ -500,7 +501,7 @@ func printPodTemplate(pod *api.PodTemplate, w io.Writer, withNamespace bool, wid name, firstContainer.Name, firstContainer.Image, - formatLabels(pod.Template.Labels), + labels.FormatLabels(pod.Template.Labels), ); err != nil { return err } @@ -553,7 +554,7 @@ func printReplicationController(controller *api.ReplicationController, w io.Writ name, firstContainer.Name, firstContainer.Image, - formatLabels(controller.Spec.Selector), + labels.FormatLabels(controller.Spec.Selector), controller.Spec.Replicas, translateTimestamp(controller.CreationTimestamp), ); err != nil { @@ -643,7 +644,7 @@ func printService(svc *api.Service, w io.Writer, withNamespace bool, wide bool, internalIP, externalIP, makePortString(svc.Spec.Ports), - formatLabels(svc.Spec.Selector), + labels.FormatLabels(svc.Spec.Selector), translateTimestamp(svc.CreationTimestamp), ); err != nil { return err @@ -693,7 +694,7 @@ func printNamespace(item *api.Namespace, w io.Writer, withNamespace bool, wide b return fmt.Errorf("namespace is not namespaced") } - if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s", item.Name, formatLabels(item.Labels), item.Status.Phase, translateTimestamp(item.CreationTimestamp)); err != nil { + if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s", item.Name, labels.FormatLabels(item.Labels), item.Status.Phase, translateTimestamp(item.CreationTimestamp)); err != nil { return err } _, err := fmt.Fprint(w, appendLabels(item.Labels, columnLabels)) @@ -788,7 +789,7 @@ func printNode(node *api.Node, w io.Writer, withNamespace bool, wide bool, showA status = append(status, "SchedulingDisabled") } - if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s", node.Name, formatLabels(node.Labels), strings.Join(status, ","), translateTimestamp(node.CreationTimestamp)); err != nil { + if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s", node.Name, labels.FormatLabels(node.Labels), strings.Join(status, ","), translateTimestamp(node.CreationTimestamp)); err != nil { return err } _, err := fmt.Fprint(w, appendLabels(node.Labels, columnLabels)) @@ -824,7 +825,7 @@ func printPersistentVolume(pv *api.PersistentVolume, w io.Writer, withNamespace if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s", name, - formatLabels(pv.Labels), + labels.FormatLabels(pv.Labels), aSize, modesStr, pv.Status.Phase, claimRefUID, @@ -856,7 +857,7 @@ func printPersistentVolumeClaim(pvc *api.PersistentVolumeClaim, w io.Writer, wit } } - labels := formatLabels(pvc.Labels) + labels := labels.FormatLabels(pvc.Labels) phase := pvc.Status.Phase storage := pvc.Spec.Resources.Requests[api.ResourceStorage] capacity := "" diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index a1d0f65ae7f..73324ba2bc3 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -60,3 +60,12 @@ func (ls Set) Get(label string) string { func (ls Set) AsSelector() Selector { return SelectorFromSet(ls) } + +// FormatLables convert label map into plain string +func FormatLabels(labelMap map[string]string) string { + l := Set(labelMap).String() + if l == "" { + l = "" + } + return l +} From ddf090d82485f80e9075c80af2906a8d883caff3 Mon Sep 17 00:00:00 2001 From: hurf Date: Sun, 16 Aug 2015 20:22:35 +0800 Subject: [PATCH 2/2] Change default output of 'label' command Using simple, human understandable output instead of the form of 'kubectl get' command. --- pkg/kubectl/cmd/label.go | 18 ++++++++++++------ pkg/kubectl/cmd/label_test.go | 27 ++++++--------------------- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/pkg/kubectl/cmd/label.go b/pkg/kubectl/cmd/label.go index 0dc7d74aa89..db7260cb7df 100644 --- a/pkg/kubectl/cmd/label.go +++ b/pkg/kubectl/cmd/label.go @@ -173,7 +173,7 @@ func RunLabel(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []stri return err } - labels, remove, err := parseLabels(labelArgs) + lbls, remove, err := parseLabels(labelArgs) if err != nil { return cmdutil.UsageError(cmd, err.Error()) } @@ -204,7 +204,7 @@ func RunLabel(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []stri return err } obj, err := cmdutil.UpdateObject(info, func(obj runtime.Object) error { - err := labelFunc(obj, overwrite, resourceVersion, labels, remove) + err := labelFunc(obj, overwrite, resourceVersion, lbls, remove) if err != nil { return err } @@ -214,10 +214,16 @@ func RunLabel(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []stri return err } - printer, err := f.PrinterForMapping(cmd, info.Mapping, false) - if err != nil { - return err + outputFormat := cmdutil.GetFlagString(cmd, "output") + if outputFormat == "" { + cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "labeled") + } else { + printer, err := f.PrinterForMapping(cmd, info.Mapping, false) + if err != nil { + return err + } + return printer.PrintObj(obj, out) } - return printer.PrintObj(obj, out) + return nil }) } diff --git a/pkg/kubectl/cmd/label_test.go b/pkg/kubectl/cmd/label_test.go index c8345df9f91..82eaf69ca23 100644 --- a/pkg/kubectl/cmd/label_test.go +++ b/pkg/kubectl/cmd/label_test.go @@ -325,10 +325,7 @@ func TestLabelErrors(t *testing.T) { func TestLabelForResourceFromFile(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) { @@ -355,7 +352,6 @@ func TestLabelForResourceFromFile(t *testing.T) { } }), } - tf.Namespace = "test" tf.ClientConfig = &client.Config{Version: testapi.Version()} @@ -363,25 +359,18 @@ func TestLabelForResourceFromFile(t *testing.T) { cmd := NewCmdLabel(f, buf) cmd.Flags().Set("filename", "../../../examples/cassandra/cassandra.yaml") - cmd.SetOutput(buf) err := RunLabel(f, buf, cmd, []string{"a=b"}) - if err != nil { t.Fatalf("unexpected error: %v", err) } - if tf.Printer.(*testPrinter).Objects == nil { - t.Errorf("unexpected 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())) + if !strings.Contains(buf.String(), "labeled") { + t.Errorf("did not set labels: %s", buf.String()) } } 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) { @@ -412,19 +401,15 @@ func TestLabelMultipleObjects(t *testing.T) { } tf.Namespace = "test" tf.ClientConfig = &client.Config{Version: testapi.Version()} + buf := bytes.NewBuffer([]byte{}) - cmd := NewCmdLabel(f, 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())) + if strings.Count(buf.String(), "labeled") != len(pods.Items) { + t.Errorf("not all labels are set: %s", buf.String()) } }