Make expose use introspection to grab the port value if possible.

Also improve service printing to include public IP addresses.
duda · Apr 2, 2015 · 674efe6 · 674efe6
1 parent 92b6f49
commit 674efe6
Show file tree

Hide file tree

Showing 4 changed files with 192 additions and 11 deletions.
diff --git a/pkg/kubectl/cmd/cmd.go b/pkg/kubectl/cmd/cmd.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strconv"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
@@ -71,12 +72,24 @@ type Factory struct {
 	// PodSelectorForResource returns the pod selector associated with the provided resource name
 	// or an error.
 	PodSelectorForResource func(mapping *meta.RESTMapping, namespace, name string) (string, error)
+	// PortForResource returns the ports associated with the provided resource name or an error
+	PortsForResource func(mapping *meta.RESTMapping, namespace, name string) ([]string, error)
 	// Returns a schema that can validate objects stored on disk.
 	Validator func() (validation.Schema, error)
 	// Returns the default namespace to use in cases where no other namespace is specified
 	DefaultNamespace func() (string, error)
 }
 
+func getPorts(spec api.PodSpec) []string {
+	result := []string{}
+	for _, container := range spec.Containers {
+		for _, port := range container.Ports {
+			result = append(result, strconv.Itoa(port.ContainerPort))
+		}
+	}
+	return result
+}
+
 // NewFactory creates a factory with the default Kubernetes resources defined
 // if optionalClientConfig is nil, then flags will be bound to a new clientcmd.ClientConfig.
 // if optionalClientConfig is not nil, then this factory will make use of it.
@@ -168,6 +181,29 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 				return "", fmt.Errorf("it is not possible to get a pod selector from %s", mapping.Kind)
 			}
 		},
+		PortsForResource: func(mapping *meta.RESTMapping, namespace, name string) ([]string, error) {
+			// TODO: replace with a swagger schema based approach (identify pod selector via schema introspection)
+			client, err := clients.ClientForVersion("")
+			if err != nil {
+				return nil, err
+			}
+			switch mapping.Kind {
+			case "ReplicationController":
+				rc, err := client.ReplicationControllers(namespace).Get(name)
+				if err != nil {
+					return nil, err
+				}
+				return getPorts(rc.Spec.Template.Spec), nil
+			case "Pod":
+				pod, err := client.Pods(namespace).Get(name)
+				if err != nil {
+					return nil, err
+				}
+				return getPorts(pod.Spec), nil
+			default:
+				return nil, fmt.Errorf("it is not possible to get ports from %s", mapping.Kind)
+			}
+		},
 		Resizer: func(mapping *meta.RESTMapping) (kubectl.Resizer, error) {
 			client, err := clients.ClientForVersion(mapping.APIVersion)
 			if err != nil {

diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go
@@ -93,17 +93,14 @@ func RunExpose(f *Factory, out io.Writer, cmd *cobra.Command, args []string) err
 	if !found {
 		return util.UsageError(cmd, fmt.Sprintf("generator %q not found.", generator))
 	}
-	if util.GetFlagInt(cmd, "port") < 1 {
-		return util.UsageError(cmd, "--port is required and must be a positive integer.")
-	}
 	names := generator.ParamNames()
 	params := kubectl.MakeParams(cmd, names)
 	if len(util.GetFlagString(cmd, "service-name")) == 0 {
 		params["name"] = name
 	} else {
 		params["name"] = util.GetFlagString(cmd, "service-name")
 	}
-	if s, found := params["selector"]; !found || len(s) == 0 {
+	if s, found := params["selector"]; !found || len(s) == 0 || util.GetFlagInt(cmd, "port") < 1 {
 		mapper, _ := f.Object()
 		v, k, err := mapper.VersionAndKindForResource(resource)
 		if err != nil {
@@ -113,11 +110,26 @@ func RunExpose(f *Factory, out io.Writer, cmd *cobra.Command, args []string) err
 		if err != nil {
 			return err
 		}
-		s, err := f.PodSelectorForResource(mapping, namespace, name)
-		if err != nil {
-			return err
+		if len(s) == 0 {
+			s, err := f.PodSelectorForResource(mapping, namespace, name)
+			if err != nil {
+				return err
+			}
+			params["selector"] = s
+		}
+		if util.GetFlagInt(cmd, "port") < 0 {
+			ports, err := f.PortsForResource(mapping, namespace, name)
+			if err != nil {
+				return err
+			}
+			if len(ports) == 0 {
+				return util.UsageError(cmd, "couldn't find a suitable port via --port flag or introspection")
+			}
+			if len(ports) > 1 {
+				return util.UsageError(cmd, "more than one port to choose from, please explicitly specify a port using the --port flag.")
+			}
+			params["port"] = ports[0]
 		}
-		params["selector"] = s
 	}
 	if util.GetFlagBool(cmd, "create-external-load-balancer") {
 		params["create-external-load-balancer"] = "true"

diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go
@@ -390,13 +390,30 @@ func printReplicationControllerList(list *api.ReplicationControllerList, w io.Wr
 }
 
 func printService(svc *api.Service, w io.Writer) error {
+	ips := []string{svc.Spec.PortalIP}
+	for _, publicIP := range svc.Spec.PublicIPs {
+		ips = append(ips, publicIP)
+	}
 	if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d/%s\n", svc.Name, formatLabels(svc.Labels),
-		formatLabels(svc.Spec.Selector), svc.Spec.PortalIP, svc.Spec.Ports[0].Port, svc.Spec.Ports[0].Protocol); err != nil {
+		formatLabels(svc.Spec.Selector), ips[0], svc.Spec.Ports[0].Port, svc.Spec.Ports[0].Protocol); err != nil {
 		return err
 	}
-	for i := 1; i < len(svc.Spec.Ports); i++ {
+
+	count := len(svc.Spec.Ports)
+	if len(ips) > count {
+		count = len(ips)
+	}
+	for i := 1; i < count; i++ {
+		ip := ""
+		if len(ips) > i {
+			ip = ips[i]
+		}
+		port := ""
+		if len(svc.Spec.Ports) > i {
+			port = fmt.Sprintf("%d/%s", svc.Spec.Ports[i].Port, svc.Spec.Ports[i].Protocol)
+		}
 		// Lay out additional ports.
-		if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d/%s\n", "", "", "", "", svc.Spec.Ports[i].Port, svc.Spec.Ports[i].Protocol); err != nil {
+		if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", "", "", "", ip, port); err != nil {
 			return err
 		}
 	}

diff --git a/pkg/kubectl/resource_printer_test.go b/pkg/kubectl/resource_printer_test.go
@@ -630,3 +630,119 @@ func contains(fields []string, field string) bool {
 	}
 	return false
 }
+
+func TestPrintHumanReadableService(t *testing.T) {
+	tests := []api.Service{
+		{
+			Spec: api.ServiceSpec{
+				PortalIP: "1.2.3.4",
+				PublicIPs: []string{
+					"2.3.4.5",
+					"3.4.5.6",
+				},
+				Ports: []api.ServicePort{
+					{
+						Port:     80,
+						Protocol: "TCP",
+					},
+				},
+			},
+		},
+		{
+			Spec: api.ServiceSpec{
+				PortalIP: "1.2.3.4",
+				Ports: []api.ServicePort{
+					{
+						Port:     80,
+						Protocol: "TCP",
+					},
+					{
+						Port:     8090,
+						Protocol: "UDP",
+					},
+					{
+						Port:     8000,
+						Protocol: "TCP",
+					},
+				},
+			},
+		},
+		{
+			Spec: api.ServiceSpec{
+				PortalIP: "1.2.3.4",
+				PublicIPs: []string{
+					"2.3.4.5",
+				},
+				Ports: []api.ServicePort{
+					{
+						Port:     80,
+						Protocol: "TCP",
+					},
+					{
+						Port:     8090,
+						Protocol: "UDP",
+					},
+					{
+						Port:     8000,
+						Protocol: "TCP",
+					},
+				},
+			},
+		},
+		{
+			Spec: api.ServiceSpec{
+				PortalIP: "1.2.3.4",
+				PublicIPs: []string{
+					"2.3.4.5",
+					"4.5.6.7",
+					"5.6.7.8",
+				},
+				Ports: []api.ServicePort{
+					{
+						Port:     80,
+						Protocol: "TCP",
+					},
+					{
+						Port:     8090,
+						Protocol: "UDP",
+					},
+					{
+						Port:     8000,
+						Protocol: "TCP",
+					},
+				},
+			},
+		},
+	}
+
+	for _, svc := range tests {
+		buff := bytes.Buffer{}
+		printService(&svc, &buff)
+		output := string(buff.Bytes())
+		ip := svc.Spec.PortalIP
+		if !strings.Contains(output, ip) {
+			t.Errorf("expected to contain portal ip %s, but doesn't: %s", ip, output)
+		}
+
+		for _, ip = range svc.Spec.PublicIPs {
+			if !strings.Contains(output, ip) {
+				t.Errorf("expected to contain public ip %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)
+			}
+		}
+		// Max of # ports and (# public ip + portal ip)
+		count := len(svc.Spec.Ports)
+		if len(svc.Spec.PublicIPs)+1 > count {
+			count = len(svc.Spec.PublicIPs) + 1
+		}
+		if count != strings.Count(output, "\n") {
+			t.Errorf("expected %d newlines, found %d", count, strings.Count(output, "\n"))
+		}
+	}
+}