Change the v1b3 default for service ContainerPort

In v1b1 and v1b2 we choose the "first defined port" if you do not specify a
ContainerPort.  I am proposing that v1b3 just assume the ContainerPort is the
same as the service port unless explicitly provided.  This leaves named ports
for now, but that is under discussion on its own.

This is strictly compatible, though to implement this we have to leave the
internal objects with the looser behavior until v1b[12] die.  This also adds a
link dependency so that when we DO kill v1b[12] the endpoints controller will
blow up, prompting a fix.

pkg/api/testing/fuzzer.go

@@ -245,6 +245,27 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer {
 			c.Fuzz(&http.Port)
 			c.Fuzz(&http.Host)
 		},
+		func(ss *api.ServiceSpec, c fuzz.Continue) {
+			// TODO: I wish I could say "fuzz myself but without the custom fuzz-func"
+			c.Fuzz(&ss.Port)
+			c.Fuzz(&ss.Protocol)
+			c.Fuzz(&ss.Selector)
+			c.Fuzz(&ss.PortalIP)
+			c.Fuzz(&ss.CreateExternalLoadBalancer)
+			c.Fuzz(&ss.PublicIPs)
+			c.Fuzz(&ss.SessionAffinity)
+			// TODO: would be great if types could voluntarily fuzz themselves.
+			kinds := []util.IntstrKind{util.IntstrInt, util.IntstrString}
+			ss.ContainerPort.Kind = kinds[c.Rand.Intn(len(kinds))]
+			switch ss.ContainerPort.Kind {
+			case util.IntstrInt:
+				ss.ContainerPort.IntVal = 1 + c.Rand.Intn(65535) // non-zero
+				ss.ContainerPort.StrVal = ""                     // needed because we reuse objects without zeroing them
+			case util.IntstrString:
+				ss.ContainerPort.StrVal = "x" + c.RandString() // non-empty
+				ss.ContainerPort.IntVal = 0                    // needed because we reuse objects without zeroing them
+			}
+		},
 	)
 	return f
 }

pkg/api/types.go

@@ -724,6 +724,8 @@ type ServiceSpec struct {
 	// ContainerPort is the name or number of the port on the container to direct traffic to.
 	// This is useful if the containers the service points to have multiple open ports.
 	// Optional: If unspecified, the first port on the container will be used.
+	// As of v1beta3 this field will become required in the internal API,
+	// and the versioned APIs must provide a default value.
 	ContainerPort util.IntOrString `json:"containerPort,omitempty"`
 
 	// Required: Supports "ClientIP" and "None".  Used to maintain session affinity.

pkg/api/v1beta1/register.go

@@ -24,6 +24,12 @@ import (
 // Codec encodes internal objects to the v1beta1 scheme
 var Codec = runtime.CodecFor(api.Scheme, "v1beta1")
 
+// Dependency does nothing but give a hook for other packages to force a
+// compile-time error when this API version is eventually removed.  This is
+// useful, for example, to clean up things that are implicitly tied to
+// semantics of older APIs.
+const Dependency = true
+
 func init() {
 	api.Scheme.AddKnownTypes("v1beta1",
 		&Pod{},

pkg/api/v1beta2/register.go

@@ -24,6 +24,12 @@ import (
 // Codec encodes internal objects to the v1beta2 scheme
 var Codec = runtime.CodecFor(api.Scheme, "v1beta2")
 
+// Dependency does nothing but give a hook for other packages to force a
+// compile-time error when this API version is eventually removed.  This is
+// useful, for example, to clean up things that are implicitly tied to
+// semantics of older APIs.
+const Dependency = true
+
 func init() {
 	api.Scheme.AddKnownTypes("v1beta2",
 		&Pod{},

pkg/api/v1beta3/defaults.go

@@ -90,5 +90,11 @@ func init() {
 				obj.Path = "/"
 			}
 		},
+		func(obj *ServiceSpec) {
+			if obj.ContainerPort.Kind == util.IntstrInt && obj.ContainerPort.IntVal == 0 ||
+				obj.ContainerPort.Kind == util.IntstrString && obj.ContainerPort.StrVal == "" {
+				obj.ContainerPort = util.NewIntOrStringFromInt(obj.Port)
+			}
+		},
 	)
 }

pkg/api/v1beta3/defaults_test.go

@@ -22,6 +22,7 @@ import (
 
 	current "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
 func roundTrip(t *testing.T, obj runtime.Object) runtime.Object {
@@ -112,3 +113,19 @@ func TestSetDefaulEndpointsProtocol(t *testing.T) {
 		t.Errorf("Expected protocol %s, got %s", current.ProtocolTCP, out.Protocol)
 	}
 }
+
+func TestSetDefaulServiceDestinationPort(t *testing.T) {
+	in := &current.Service{Spec: current.ServiceSpec{Port: 1234}}
+	obj := roundTrip(t, runtime.Object(in))
+	out := obj.(*current.Service)
+	if out.Spec.ContainerPort.Kind != util.IntstrInt || out.Spec.ContainerPort.IntVal != 1234 {
+		t.Errorf("Expected ContainerPort to be defaulted, got %s", out.Spec.ContainerPort)
+	}
+
+	in = &current.Service{Spec: current.ServiceSpec{Port: 1234, ContainerPort: util.NewIntOrStringFromInt(5678)}}
+	obj = roundTrip(t, runtime.Object(in))
+	out = obj.(*current.Service)
+	if out.Spec.ContainerPort.Kind != util.IntstrInt || out.Spec.ContainerPort.IntVal != 5678 {
+		t.Errorf("Expected ContainerPort to be unchanged, got %s", out.Spec.ContainerPort)
+	}
+}

pkg/api/v1beta3/types.go

@@ -747,7 +747,7 @@ type ServiceSpec struct {
 
 	// ContainerPort is the name or number of the port on the container to direct traffic to.
 	// This is useful if the containers the service points to have multiple open ports.
-	// Optional: If unspecified, the first port on the container will be used.
+	// Optional: If unspecified, the service port is used (an identity map).
 	ContainerPort util.IntOrString `json:"containerPort,omitempty"`
 
 	// Optional: Supports "ClientIP" and "None".  Used to maintain session affinity.

pkg/service/endpoints_controller.go

@@ -21,6 +21,8 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta2"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
@@ -65,6 +67,10 @@ func (e *EndpointController) SyncServiceEndpoints() error {
 		endpoints := []api.Endpoint{}
 
 		for _, pod := range pods.Items {
+			// TODO: Once v1beta1 and v1beta2 are EOL'ed, this can
+			// assume that service.Spec.ContainerPort is populated.
+			_ = v1beta1.Dependency
+			_ = v1beta2.Dependency
 			port, err := findPort(&pod, service.Spec.ContainerPort)
 			if err != nil {
 				glog.Errorf("Failed to find port for service %s/%s: %v", service.Namespace, service.Name, err)