Merge pull request #4936 from thockin/plural_24_default_dest_port

Change the v1b3 default for service ContainerPort

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)