Two small bugs in dual-stack init

Imporved testing turned these up:

1) Headless+Selectorless, on a single-stack cluster, policy=PreferDual

Prior to this commit, the result was a single IPFamiliy (because we
checked that the 2nd allocator was present).  This changes that case to
populate both families (we don't care if the allocator exists), which is
the same as RequireDual.

2) ClusterIP, user specifies 2 families but no IPs

Prior to this commit, the policy was inferred to be SingleStack.  This
changes that case to correctly default to RequireDual when 2 families
are present but no IPs.

pkg/registry/core/service/storage/rest.go

@@ -872,7 +872,7 @@ func (rs *REST) tryDefaultValidateServiceClusterIPFields(service *api.Service) e
 		}
 	}
 
-	// default families according to cluster IPs
+	// Infer IPFamilies[] from ClusterIPs[].
 	for i, ip := range service.Spec.ClusterIPs {
 		if ip == api.ClusterIPNone {
 			break
@@ -882,8 +882,8 @@ func (rs *REST) tryDefaultValidateServiceClusterIPFields(service *api.Service) e
 		// so the following is safe to do
 		isIPv6 := netutil.IsIPv6String(ip)
 
-		// family is not there.
+		// Family is not specified yet.
-		if i > len(service.Spec.IPFamilies)-1 {
+		if i >= len(service.Spec.IPFamilies) {
 			if isIPv6 {
 				// first make sure that family(ip) is configured
 				if _, found := rs.serviceIPAllocatorsByFamily[api.IPv6Protocol]; !found {
@@ -902,15 +902,23 @@ func (rs *REST) tryDefaultValidateServiceClusterIPFields(service *api.Service) e
 		}
 	}
 
-	// default headless+selectorless
+	// Infer IPFamilyPolicy from IPFamilies[].  This block does not handle the
-	if len(service.Spec.ClusterIPs) > 0 && service.Spec.ClusterIPs[0] == api.ClusterIPNone && len(service.Spec.Selector) == 0 {
+	// final defaulting - that happens a bit later, after special-cases.
+	if service.Spec.IPFamilyPolicy == nil && len(service.Spec.IPFamilies) == 2 {
+		requireDualStack := api.IPFamilyPolicyRequireDualStack
+		service.Spec.IPFamilyPolicy = &requireDualStack
+	}
 
+	// Special-case: headless+selectorless
+	if len(service.Spec.ClusterIPs) > 0 && service.Spec.ClusterIPs[0] == api.ClusterIPNone && len(service.Spec.Selector) == 0 {
+		// If the use said nothing about policy and we can't infer it, they get dual-stack
 		if service.Spec.IPFamilyPolicy == nil {
 			requireDualStack := api.IPFamilyPolicyRequireDualStack
 			service.Spec.IPFamilyPolicy = &requireDualStack
 		}
 
-		// if not set by user
+		// If IPFamilies was not set by the user, start with the default
+		// family.
 		if len(service.Spec.IPFamilies) == 0 {
 			service.Spec.IPFamilies = []api.IPFamily{rs.defaultServiceIPFamily}
 		}
@@ -919,8 +927,7 @@ func (rs *REST) tryDefaultValidateServiceClusterIPFields(service *api.Service) e
 		// cluster the user is allowed to create headless services that has multi families
 		// the validation allows it
 		if len(service.Spec.IPFamilies) < 2 {
-			if *(service.Spec.IPFamilyPolicy) == api.IPFamilyPolicyRequireDualStack ||
+			if *(service.Spec.IPFamilyPolicy) != api.IPFamilyPolicySingleStack {
-				(*(service.Spec.IPFamilyPolicy) == api.IPFamilyPolicyPreferDualStack && len(rs.serviceIPAllocatorsByFamily) == 2) {
 				// add the alt ipfamily
 				if service.Spec.IPFamilies[0] == api.IPv4Protocol {
 					service.Spec.IPFamilies = append(service.Spec.IPFamilies, api.IPv6Protocol)
@@ -956,8 +963,8 @@ func (rs *REST) tryDefaultValidateServiceClusterIPFields(service *api.Service) e
 		return errors.NewInvalid(api.Kind("Service"), service.Name, el)
 	}
 
-	// default ipFamilyPolicy to SingleStack. if there are
+	// Finally, if IPFamilyPolicy is *still* not set, we can default it to
-	// web hooks, they must have already ran by now
+	// SingleStack. If there are any webhooks, they have already run.
 	if service.Spec.IPFamilyPolicy == nil {
 		singleStack := api.IPFamilyPolicySingleStack
 		service.Spec.IPFamilyPolicy = &singleStack

pkg/registry/core/service/storage/rest_test.go

@@ -3892,7 +3892,7 @@ func TestDefaultingValidation(t *testing.T) {
 				},
 			},
 			expectedIPFamilyPolicy: &preferDualStack,
-			expectedIPFamilies:     []api.IPFamily{api.IPv4Protocol},
+			expectedIPFamilies:     []api.IPFamily{api.IPv4Protocol, api.IPv6Protocol},
 			expectError:            false,
 		},
 		// tests incorrect setting for IPFamilyPolicy
@@ -4173,7 +4173,7 @@ func TestDefaultingValidation(t *testing.T) {
 				},
 			},
 			expectedIPFamilyPolicy: &preferDualStack,
-			expectedIPFamilies:     []api.IPFamily{api.IPv6Protocol},
+			expectedIPFamilies:     []api.IPFamily{api.IPv6Protocol, api.IPv4Protocol},
 			expectError:            false,
 		},
 		// tests incorrect setting for IPFamilyPolicy