dual stack services

cmd/kube-apiserver/app/options/validation.go

@@ -55,9 +55,12 @@ func validateClusterIPFlags(options *ServerRunOptions) []error {
 	}
 
 	// Secondary IP validation
+	// while api-server dualstack bits does not have dependency on EndPointSlice, its
+	// a good idea to have validation consistent across all components (ControllerManager
+	// needs EndPointSlice + DualStack feature flags).
 	secondaryServiceClusterIPRangeUsed := (options.SecondaryServiceClusterIPRange.IP != nil)
-	if secondaryServiceClusterIPRangeUsed && !utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) {
-		errs = append(errs, fmt.Errorf("--secondary-service-cluster-ip-range can only be used if %v feature is enabled", string(features.IPv6DualStack)))
+	if secondaryServiceClusterIPRangeUsed && (!utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) || !utilfeature.DefaultFeatureGate.Enabled(features.EndpointSlice)) {
+		errs = append(errs, fmt.Errorf("secondary service cluster-ip range(--service-cluster-ip-range[1]) can only be used if %v and %v feature is enabled", string(features.IPv6DualStack), string(features.EndpointSlice)))
 	}
 
 	// note: While the cluster might be dualstack (i.e. pods with multiple IPs), the user may choose
@@ -68,14 +71,14 @@ func validateClusterIPFlags(options *ServerRunOptions) []error {
 		// Should be dualstack IPFamily(PrimaryServiceClusterIPRange) != IPFamily(SecondaryServiceClusterIPRange)
 		dualstack, err := netutils.IsDualStackCIDRs([]*net.IPNet{&options.PrimaryServiceClusterIPRange, &options.SecondaryServiceClusterIPRange})
 		if err != nil {
-			errs = append(errs, errors.New("error attempting to validate dualstack for --service-cluster-ip-range and --secondary-service-cluster-ip-range"))
+			errs = append(errs, fmt.Errorf("error attempting to validate dualstack for --service-cluster-ip-range value error:%v", err))
 		}
 
 		if !dualstack {
-			errs = append(errs, errors.New("--service-cluster-ip-range and --secondary-service-cluster-ip-range must be of different IP family"))
+			errs = append(errs, errors.New("--service-cluster-ip-range[0] and --service-cluster-ip-range[1] must be of different IP family"))
 		}
 
-		if err := validateMaxCIDRRange(options.SecondaryServiceClusterIPRange, maxCIDRBits, "--secondary-service-cluster-ip-range"); err != nil {
+		if err := validateMaxCIDRRange(options.SecondaryServiceClusterIPRange, maxCIDRBits, "--service-cluster-ip-range[1]"); err != nil {
 			errs = append(errs, err)
 		}
 	}

cmd/kube-apiserver/app/options/validation_test.go

@@ -54,10 +54,11 @@ func makeOptionsWithCIDRs(serviceCIDR string, secondaryServiceCIDR string) *Serv
 
 func TestClusterSerivceIPRange(t *testing.T) {
 	testCases := []struct {
-		name            string
-		options         *ServerRunOptions
-		enableDualStack bool
-		expectErrors    bool
+		name                string
+		options             *ServerRunOptions
+		enableDualStack     bool
+		enableEndpointSlice bool
+		expectErrors        bool
 	}{
 		{
 			name:            "no service cidr",
@@ -66,10 +67,11 @@ func TestClusterSerivceIPRange(t *testing.T) {
 			enableDualStack: false,
 		},
 		{
-			name:            "only secondary service cidr, dual stack gate on",
-			expectErrors:    true,
-			options:         makeOptionsWithCIDRs("", "10.0.0.0/16"),
-			enableDualStack: true,
+			name:                "only secondary service cidr, dual stack gate on",
+			expectErrors:        true,
+			options:             makeOptionsWithCIDRs("", "10.0.0.0/16"),
+			enableDualStack:     true,
+			enableEndpointSlice: true,
 		},
 		{
 			name:            "only secondary service cidr, dual stack gate off",
@@ -78,16 +80,18 @@ func TestClusterSerivceIPRange(t *testing.T) {
 			enableDualStack: false,
 		},
 		{
-			name:            "primary and secondary are provided but not dual stack v4-v4",
-			expectErrors:    true,
-			options:         makeOptionsWithCIDRs("10.0.0.0/16", "11.0.0.0/16"),
-			enableDualStack: true,
+			name:                "primary and secondary are provided but not dual stack v4-v4",
+			expectErrors:        true,
+			options:             makeOptionsWithCIDRs("10.0.0.0/16", "11.0.0.0/16"),
+			enableDualStack:     true,
+			enableEndpointSlice: true,
 		},
 		{
-			name:            "primary and secondary are provided but not dual stack v6-v6",
-			expectErrors:    true,
-			options:         makeOptionsWithCIDRs("2000::/108", "3000::/108"),
-			enableDualStack: true,
+			name:                "primary and secondary are provided but not dual stack v6-v6",
+			expectErrors:        true,
+			options:             makeOptionsWithCIDRs("2000::/108", "3000::/108"),
+			enableDualStack:     true,
+			enableEndpointSlice: true,
 		},
 		{
 			name:            "valid dual stack with gate disabled",
@@ -96,16 +100,33 @@ func TestClusterSerivceIPRange(t *testing.T) {
 			enableDualStack: false,
 		},
 		{
-			name:            "service cidr to big",
-			expectErrors:    true,
-			options:         makeOptionsWithCIDRs("10.0.0.0/8", ""),
-			enableDualStack: true,
+			name:                "service cidr is too big",
+			expectErrors:        true,
+			options:             makeOptionsWithCIDRs("10.0.0.0/8", ""),
+			enableDualStack:     true,
+			enableEndpointSlice: true,
 		},
 		{
-			name:            "dual-stack secondary cidr to big",
-			expectErrors:    true,
-			options:         makeOptionsWithCIDRs("10.0.0.0/16", "3000::/64"),
-			enableDualStack: true,
+			name:                "dual-stack secondary cidr too big",
+			expectErrors:        true,
+			options:             makeOptionsWithCIDRs("10.0.0.0/16", "3000::/64"),
+			enableDualStack:     true,
+			enableEndpointSlice: true,
+		},
+		{
+			name:                "valid v6-v4 dual stack + gate on + endpointSlice gate is on",
+			expectErrors:        false,
+			options:             makeOptionsWithCIDRs("3000::/108", "10.0.0.0/16"),
+			enableDualStack:     true,
+			enableEndpointSlice: true,
+		},
+
+		{
+			name:                "valid v4-v6 dual stack + gate on + endpointSlice is off",
+			expectErrors:        true,
+			options:             makeOptionsWithCIDRs("10.0.0.0/16", "3000::/108"),
+			enableDualStack:     true,
+			enableEndpointSlice: false,
 		},
 		/* success cases */
 		{
@@ -115,22 +136,25 @@ func TestClusterSerivceIPRange(t *testing.T) {
 			enableDualStack: false,
 		},
 		{
-			name:            "valid v4-v6 dual stack + gate on",
-			expectErrors:    false,
-			options:         makeOptionsWithCIDRs("10.0.0.0/16", "3000::/108"),
-			enableDualStack: true,
+			name:                "valid v4-v6 dual stack + gate on",
+			expectErrors:        false,
+			options:             makeOptionsWithCIDRs("10.0.0.0/16", "3000::/108"),
+			enableDualStack:     true,
+			enableEndpointSlice: true,
 		},
 		{
-			name:            "valid v6-v4 dual stack + gate on",
-			expectErrors:    false,
-			options:         makeOptionsWithCIDRs("3000::/108", "10.0.0.0/16"),
-			enableDualStack: true,
+			name:                "valid v6-v4 dual stack + gate on",
+			expectErrors:        false,
+			options:             makeOptionsWithCIDRs("3000::/108", "10.0.0.0/16"),
+			enableDualStack:     true,
+			enableEndpointSlice: true,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IPv6DualStack, tc.enableDualStack)()
+			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EndpointSlice, tc.enableEndpointSlice)()
 			errs := validateClusterIPFlags(tc.options)
 			if len(errs) > 0 && !tc.expectErrors {
 				t.Errorf("expected no errors, errors found %+v", errs)

cmd/kube-controller-manager/app/core.go

@@ -110,14 +110,14 @@ func startNodeIpamController(ctx ControllerContext) (http.Handler, bool, error)
 		return nil, false, err
 	}
 
-	// failure: more than one cidr and dual stack is not enabled
-	if len(clusterCIDRs) > 1 && !utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) {
-		return nil, false, fmt.Errorf("len of ClusterCIDRs==%v and dualstack feature is not enabled", len(clusterCIDRs))
+	// failure: more than one cidr and dual stack is not enabled and/or endpoint slice is not enabled
+	if len(clusterCIDRs) > 1 && (!utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) || !utilfeature.DefaultFeatureGate.Enabled(features.EndpointSlice)) {
+		return nil, false, fmt.Errorf("len of ClusterCIDRs==%v and dualstack or EndpointSlice feature is not enabled", len(clusterCIDRs))
 	}
 
 	// failure: more than one cidr but they are not configured as dual stack
 	if len(clusterCIDRs) > 1 && !dualStack {
-		return nil, false, fmt.Errorf("len of ClusterCIDRs==%v and they are not configured as dual stack (at least one from each IPFamily", len(clusterCIDRs))
+		return nil, false, fmt.Errorf("len of ClusterCIDRs==%v and they are not configured as dual stack (at least one from each IPFamily)", len(clusterCIDRs))
 	}
 
 	// failure: more than cidrs is not allowed even with dual stack

pkg/apis/core/helper/helpers.go

@@ -267,7 +267,10 @@ func IsIntegerResourceName(str string) bool {
 // IsServiceIPSet aims to check if the service's ClusterIP is set or not
 // the objective is not to perform validation here
 func IsServiceIPSet(service *core.Service) bool {
-	return service.Spec.ClusterIP != core.ClusterIPNone && service.Spec.ClusterIP != ""
+	// This function assumes that the service is semantically validated
+	// it does not test if the IP is valid, just makes sure that it is set.
+	return len(service.Spec.ClusterIP) > 0 &&
+		service.Spec.ClusterIP != core.ClusterIPNone
 }
 
 var standardFinalizers = sets.NewString(

pkg/apis/core/helper/helpers_test.go

@@ -294,3 +294,73 @@ func TestIsOvercommitAllowed(t *testing.T) {
 		}
 	}
 }
+
+func TestIsServiceIPSet(t *testing.T) {
+	testCases := []struct {
+		input  core.ServiceSpec
+		output bool
+		name   string
+	}{
+		{
+			name: "nil cluster ip",
+			input: core.ServiceSpec{
+				ClusterIPs: nil,
+			},
+
+			output: false,
+		},
+		{
+			name: "headless service",
+			input: core.ServiceSpec{
+				ClusterIP:  "None",
+				ClusterIPs: []string{"None"},
+			},
+			output: false,
+		},
+		// true cases
+		{
+			name: "one ipv4",
+			input: core.ServiceSpec{
+				ClusterIP:  "1.2.3.4",
+				ClusterIPs: []string{"1.2.3.4"},
+			},
+			output: true,
+		},
+		{
+			name: "one ipv6",
+			input: core.ServiceSpec{
+				ClusterIP:  "2001::1",
+				ClusterIPs: []string{"2001::1"},
+			},
+			output: true,
+		},
+		{
+			name: "v4, v6",
+			input: core.ServiceSpec{
+				ClusterIP:  "1.2.3.4",
+				ClusterIPs: []string{"1.2.3.4", "2001::1"},
+			},
+			output: true,
+		},
+		{
+			name: "v6, v4",
+			input: core.ServiceSpec{
+				ClusterIP:  "2001::1",
+				ClusterIPs: []string{"2001::1", "1.2.3.4"},
+			},
+
+			output: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			s := core.Service{
+				Spec: tc.input,
+			}
+			if IsServiceIPSet(&s) != tc.output {
+				t.Errorf("case, input: %v, expected: %v, got: %v", tc.input, tc.output, !tc.output)
+			}
+		})
+	}
+}