From 40f76a95cbd3f04757ec267e8f4bb316925367ef Mon Sep 17 00:00:00 2001 From: Rudi Chiarito Date: Thu, 2 Jun 2016 19:09:51 -0400 Subject: [PATCH] AWS: kubectl get service should print hostnames for LoadBalancer services Fixes #21526 Also test wide outputs. We only guarantee the first IP to be fully printed if multiple ingresses are present. For AWS, which has no ingress IPs, but only hostnames, the ELB hostname will be truncated, unless -o=wide is specified. --- pkg/kubectl/describe.go | 2 +- pkg/kubectl/resource_printer.go | 22 +++++++++---- pkg/kubectl/resource_printer_test.go | 49 +++++++++++++++------------- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/pkg/kubectl/describe.go b/pkg/kubectl/describe.go index c4703723ae4..83a2a106ead 100644 --- a/pkg/kubectl/describe.go +++ b/pkg/kubectl/describe.go @@ -1294,7 +1294,7 @@ func (i *IngressDescriber) describeIngress(ing *extensions.Ingress, describerSet return tabbedString(func(out io.Writer) error { fmt.Fprintf(out, "Name:\t%v\n", ing.Name) fmt.Fprintf(out, "Namespace:\t%v\n", ing.Namespace) - fmt.Fprintf(out, "Address:\t%v\n", loadBalancerStatusStringer(ing.Status.LoadBalancer)) + fmt.Fprintf(out, "Address:\t%v\n", loadBalancerStatusStringer(ing.Status.LoadBalancer, true)) def := ing.Spec.Backend ns := ing.Namespace if def == nil { diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index 61fd8d490d6..d995cdef855 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -53,6 +53,7 @@ const ( tabwriterPadding = 3 tabwriterPadChar = ' ' tabwriterFlags = 0 + loadBalancerWidth = 16 ) // GetPrinter takes a format type, an optional format argument. It will return true @@ -930,19 +931,26 @@ func printJobList(list *batch.JobList, w io.Writer, options PrintOptions) error return nil } -// loadBalancerStatusStringer behaves just like a string interface and converts the given status to a string. -func loadBalancerStatusStringer(s api.LoadBalancerStatus) string { +// loadBalancerStatusStringer behaves mostly like a string interface and converts the given status to a string. +// `wide` indicates whether the returned value is meant for --o=wide output. If not, it's clipped to 16 bytes. +func loadBalancerStatusStringer(s api.LoadBalancerStatus, wide bool) string { ingress := s.Ingress result := []string{} for i := range ingress { if ingress[i].IP != "" { result = append(result, ingress[i].IP) + } else if ingress[i].Hostname != "" { + result = append(result, ingress[i].Hostname) } } - return strings.Join(result, ",") + r := strings.Join(result, ",") + if !wide && len(r) > loadBalancerWidth { + r = r[0:(loadBalancerWidth-3)] + "..." + } + return r } -func getServiceExternalIP(svc *api.Service) string { +func getServiceExternalIP(svc *api.Service, wide bool) string { switch svc.Spec.Type { case api.ServiceTypeClusterIP: if len(svc.Spec.ExternalIPs) > 0 { @@ -955,7 +963,7 @@ func getServiceExternalIP(svc *api.Service) string { } return "" case api.ServiceTypeLoadBalancer: - lbIps := loadBalancerStatusStringer(svc.Status.LoadBalancer) + lbIps := loadBalancerStatusStringer(svc.Status.LoadBalancer, wide) if len(svc.Spec.ExternalIPs) > 0 { result := append(strings.Split(lbIps, ","), svc.Spec.ExternalIPs...) return strings.Join(result, ",") @@ -982,7 +990,7 @@ func printService(svc *api.Service, w io.Writer, options PrintOptions) error { namespace := svc.Namespace internalIP := svc.Spec.ClusterIP - externalIP := getServiceExternalIP(svc) + externalIP := getServiceExternalIP(svc, options.Wide) if options.WithNamespace { if _, err := fmt.Fprintf(w, "%s\t", namespace); err != nil { @@ -1069,7 +1077,7 @@ func printIngress(ingress *extensions.Ingress, w io.Writer, options PrintOptions if _, err := fmt.Fprintf(w, "%s\t%v\t%v\t%v\t%s", name, formatHosts(ingress.Spec.Rules), - loadBalancerStatusStringer(ingress.Status.LoadBalancer), + loadBalancerStatusStringer(ingress.Status.LoadBalancer, options.Wide), formatPorts(ingress.Spec.TLS), translateTimestamp(ingress.CreationTimestamp), ); err != nil { diff --git a/pkg/kubectl/resource_printer_test.go b/pkg/kubectl/resource_printer_test.go index e9e1f1cd74a..35461e3637b 100644 --- a/pkg/kubectl/resource_printer_test.go +++ b/pkg/kubectl/resource_printer_test.go @@ -706,7 +706,7 @@ func TestPrintHumanReadableService(t *testing.T) { }, { Spec: api.ServiceSpec{ - ClusterIP: "1.2.3.4", + ClusterIP: "1.3.4.5", Ports: []api.ServicePort{ { Port: 80, @@ -725,7 +725,7 @@ func TestPrintHumanReadableService(t *testing.T) { }, { Spec: api.ServiceSpec{ - ClusterIP: "1.2.3.4", + ClusterIP: "1.4.5.6", Type: "LoadBalancer", Ports: []api.ServicePort{ { @@ -754,7 +754,7 @@ func TestPrintHumanReadableService(t *testing.T) { }, { Spec: api.ServiceSpec{ - ClusterIP: "1.2.3.4", + ClusterIP: "1.5.6.7", Type: "LoadBalancer", Ports: []api.ServicePort{ { @@ -791,30 +791,33 @@ func TestPrintHumanReadableService(t *testing.T) { } for _, svc := range tests { - buff := bytes.Buffer{} - printService(&svc, &buff, PrintOptions{false, false, false, false, false, false, []string{}}) - output := string(buff.Bytes()) - ip := svc.Spec.ClusterIP - if !strings.Contains(output, ip) { - t.Errorf("expected to contain ClusterIP %s, but doesn't: %s", ip, output) - } - - for _, ingress := range svc.Status.LoadBalancer.Ingress { - ip = ingress.IP + for _, wide := range []bool{false, true} { + buff := bytes.Buffer{} + printService(&svc, &buff, PrintOptions{false, false, wide, false, false, false, []string{}}) + output := string(buff.Bytes()) + ip := svc.Spec.ClusterIP if !strings.Contains(output, ip) { - t.Errorf("expected to contain ingress ip %s, but doesn't: %s", ip, output) + t.Errorf("expected to contain ClusterIP %s, but doesn't: %s", ip, output) } - } - for _, port := range svc.Spec.Ports { - portSpec := fmt.Sprintf("%d/%s", port.Port, port.Protocol) - if !strings.Contains(output, portSpec) { - t.Errorf("expected to contain port: %s, but doesn't: %s", portSpec, output) + for n, ingress := range svc.Status.LoadBalancer.Ingress { + ip = ingress.IP + // For non-wide output, we only guarantee the first IP to be printed + if (n == 0 || wide) && !strings.Contains(output, ip) { + t.Errorf("expected to contain ingress ip %s with wide=%v, but doesn't: %s", ip, wide, output) + } + } + + for _, port := range svc.Spec.Ports { + portSpec := fmt.Sprintf("%d/%s", port.Port, port.Protocol) + if !strings.Contains(output, portSpec) { + t.Errorf("expected to contain port: %s, but doesn't: %s", portSpec, output) + } + } + // Each service should print on one line + if 1 != strings.Count(output, "\n") { + t.Errorf("expected a single newline, found %d", strings.Count(output, "\n")) } - } - // Each service should print on one line - if 1 != strings.Count(output, "\n") { - t.Errorf("expected a single newline, found %d", strings.Count(output, "\n")) } } }