diff --git a/pkg/apis/core/v1/defaults.go b/pkg/apis/core/v1/defaults.go index 28ceb24244e..96c8753ca8b 100644 --- a/pkg/apis/core/v1/defaults.go +++ b/pkg/apis/core/v1/defaults.go @@ -131,37 +131,6 @@ func SetDefaults_Service(obj *v1.Service) { obj.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeCluster } - if utilfeature.DefaultFeatureGate.Enabled(features.IPv6DualStack) { - // Default obj.Spec.IPFamilyPolicy if we *know* we can, otherwise it will - // be handled later in allocation. - if obj.Spec.Type != v1.ServiceTypeExternalName { - if obj.Spec.IPFamilyPolicy == nil { - if len(obj.Spec.ClusterIPs) == 2 || len(obj.Spec.IPFamilies) == 2 { - requireDualStack := v1.IPFamilyPolicyRequireDualStack - obj.Spec.IPFamilyPolicy = &requireDualStack - } - } - - // If the user demanded dual-stack, but only specified one family, we add - // the other. - if obj.Spec.IPFamilyPolicy != nil && *(obj.Spec.IPFamilyPolicy) == v1.IPFamilyPolicyRequireDualStack && len(obj.Spec.IPFamilies) == 1 { - if obj.Spec.IPFamilies[0] == v1.IPv4Protocol { - obj.Spec.IPFamilies = append(obj.Spec.IPFamilies, v1.IPv6Protocol) - } else { - obj.Spec.IPFamilies = append(obj.Spec.IPFamilies, v1.IPv4Protocol) - } - - // Any other dual-stack defaulting depends on cluster configuration. - // Further IPFamilies, IPFamilyPolicy defaulting is in ClusterIP alloc/reserve logic - // NOTE: strategy handles cases where ClusterIPs is used (but not ClusterIP). - } - } - - // any other defaulting depends on cluster configuration. - // further IPFamilies, IPFamilyPolicy defaulting is in ClusterIP alloc/reserve logic - // note: conversion logic handles cases where ClusterIPs is used (but not ClusterIP). - } - if utilfeature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) { if obj.Spec.Type == v1.ServiceTypeLoadBalancer { if obj.Spec.AllocateLoadBalancerNodePorts == nil { diff --git a/pkg/apis/core/v1/defaults_test.go b/pkg/apis/core/v1/defaults_test.go index dd7221be695..d0ea41f9af8 100644 --- a/pkg/apis/core/v1/defaults_test.go +++ b/pkg/apis/core/v1/defaults_test.go @@ -35,10 +35,6 @@ import ( // ensure types are installed _ "k8s.io/kubernetes/pkg/apis/core/install" - - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/features" ) // TestWorkloadDefaults detects changes to defaults within PodTemplateSpec. @@ -1802,262 +1798,3 @@ func TestSetDefaultEnableServiceLinks(t *testing.T) { t.Errorf("Expected enableServiceLinks value: %+v\ngot: %+v\n", v1.DefaultEnableServiceLinks, *output.Spec.EnableServiceLinks) } } - -func TestSetDefaultIPFamilies(t *testing.T) { - preferDualStack := v1.IPFamilyPolicyPreferDualStack - requireDualStack := v1.IPFamilyPolicyRequireDualStack - testCases := []struct { - name string - expectedIPFamiliesWithGate []v1.IPFamily - svc v1.Service - }{ - { - name: "must not set for ExternalName", - expectedIPFamiliesWithGate: nil, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeExternalName, - }, - }, - }, - { - name: "must not set for single stack", - expectedIPFamiliesWithGate: nil, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - }, - }, - }, - { - name: "must not set for single stack, even when a family is set", - expectedIPFamiliesWithGate: []v1.IPFamily{v1.IPv6Protocol}, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - IPFamilies: []v1.IPFamily{v1.IPv6Protocol}, - }, - }, - }, - { - name: "must not set for preferDualStack", - expectedIPFamiliesWithGate: []v1.IPFamily{v1.IPv6Protocol}, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - IPFamilyPolicy: &preferDualStack, - IPFamilies: []v1.IPFamily{v1.IPv6Protocol}, - }, - }, - }, - { - name: "must set for requireDualStack (6,4)", - expectedIPFamiliesWithGate: []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol}, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - IPFamilyPolicy: &requireDualStack, - IPFamilies: []v1.IPFamily{v1.IPv6Protocol}, - }, - }, - }, - { - name: "must set for requireDualStack (4,6)", - expectedIPFamiliesWithGate: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeClusterIP, - IPFamilyPolicy: &requireDualStack, - IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, - }, - }, - }, - } - for _, test := range testCases { - // run with gate - t.Run(test.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IPv6DualStack, true)() - obj2 := roundTrip(t, runtime.Object(&test.svc)) - svc2 := obj2.(*v1.Service) - - if len(test.expectedIPFamiliesWithGate) != len(svc2.Spec.IPFamilies) { - t.Fatalf("expected .spec.IPFamilies len:%v got %v", len(test.expectedIPFamiliesWithGate), len(svc2.Spec.IPFamilies)) - } - - for i, family := range test.expectedIPFamiliesWithGate { - if svc2.Spec.IPFamilies[i] != family { - t.Fatalf("failed. expected family %v at %v got %v", family, i, svc2.Spec.IPFamilies[i]) - } - } - }) - - // TODO: @khenidak - // with BETA feature is always on. This ensure we test for gate on/off scenarios - // as we move to stable this entire test will need to be folded into one test - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IPv6DualStack, false)() - - // run without gate (families should not change) - t.Run(fmt.Sprintf("without-gate:%s", test.name), func(t *testing.T) { - obj2 := roundTrip(t, runtime.Object(&test.svc)) - svc2 := obj2.(*v1.Service) - - if len(test.svc.Spec.IPFamilies) != len(svc2.Spec.IPFamilies) { - t.Fatalf("expected .spec.IPFamilies len:%v got %v", len(test.expectedIPFamiliesWithGate), len(svc2.Spec.IPFamilies)) - } - - for i, family := range test.svc.Spec.IPFamilies { - if svc2.Spec.IPFamilies[i] != family { - t.Fatalf("failed. expected family %v at %v got %v", family, i, svc2.Spec.IPFamilies[i]) - } - } - }) - - } -} - -func TestSetDefaultServiceIPFamilyPolicy(t *testing.T) { - singleStack := v1.IPFamilyPolicySingleStack - preferDualStack := v1.IPFamilyPolicyPreferDualStack - requireDualStack := v1.IPFamilyPolicyRequireDualStack - - testCases := []struct { - name string - expectedIPfamilyPolicy *v1.IPFamilyPolicyType - svc v1.Service - }{ - { - name: "must not set for ExternalName", - expectedIPfamilyPolicy: nil, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeExternalName, - }, - }, - }, - { - name: "must not set for ExternalName even with semantically valid data", - expectedIPfamilyPolicy: nil, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - Type: v1.ServiceTypeExternalName, - ClusterIPs: []string{"1.2.3.4", "2001::1"}, - IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, - }, - }, - }, - { - name: "must set if there are more than one ip", - expectedIPfamilyPolicy: &requireDualStack, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - ClusterIPs: []string{"1.2.3.4", "2001::1"}, - }, - }, - }, - { - name: "must set if there are more than one ip family", - expectedIPfamilyPolicy: &requireDualStack, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, - }, - }, - }, - { - name: "must not set if there is one ip", - expectedIPfamilyPolicy: nil, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - ClusterIPs: []string{"1.2.3.4"}, - }, - }, - }, - { - name: "must not set if there is one ip family", - expectedIPfamilyPolicy: nil, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - IPFamilies: []v1.IPFamily{v1.IPv4Protocol}, - }, - }, - }, - { - name: "must not override user input", - expectedIPfamilyPolicy: &singleStack, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - IPFamilyPolicy: &singleStack, - }, - }, - }, - { - name: "must not override user input/ preferDualStack", - expectedIPfamilyPolicy: &preferDualStack, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - IPFamilyPolicy: &preferDualStack, - }, - }, - }, - - { - name: "must not override user input even when it is invalid", - expectedIPfamilyPolicy: &preferDualStack, - - svc: v1.Service{ - Spec: v1.ServiceSpec{ - IPFamilies: []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol}, - IPFamilyPolicy: &preferDualStack, - }, - }, - }, - } - - for _, test := range testCases { - // with gate - t.Run(test.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IPv6DualStack, true)() - obj2 := roundTrip(t, runtime.Object(&test.svc)) - svc2 := obj2.(*v1.Service) - - if test.expectedIPfamilyPolicy == nil && svc2.Spec.IPFamilyPolicy != nil { - t.Fatalf("expected .spec.PreferDualStack to be unset (nil)") - } - if test.expectedIPfamilyPolicy != nil && (svc2.Spec.IPFamilyPolicy == nil || *(svc2.Spec.IPFamilyPolicy) != *(test.expectedIPfamilyPolicy)) { - t.Fatalf("expected .spec.PreferDualStack to be set to %v got %v", *(test.expectedIPfamilyPolicy), svc2.Spec.IPFamilyPolicy) - } - }) - - // without gate. IPFamilyPolicy should never change - t.Run(test.name, func(t *testing.T) { - //TODO: @khenidak - // BETA feature. gate is on. when we go stable fold the test into one scenario - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IPv6DualStack, false)() - - obj2 := roundTrip(t, runtime.Object(&test.svc)) - svc2 := obj2.(*v1.Service) - - if test.svc.Spec.IPFamilyPolicy == nil && svc2.Spec.IPFamilyPolicy != nil { - t.Fatalf("expected .spec.PreferDualStack to be unset (nil)") - } - if test.svc.Spec.IPFamilyPolicy != nil && (svc2.Spec.IPFamilyPolicy == nil || *(svc2.Spec.IPFamilyPolicy) != *(test.expectedIPfamilyPolicy)) { - t.Fatalf("expected .spec.PreferDualStack to be set to %v got %v", *(test.expectedIPfamilyPolicy), svc2.Spec.IPFamilyPolicy) - } - }) - - } -} diff --git a/pkg/registry/core/service/storage/rest.go b/pkg/registry/core/service/storage/rest.go index 06663d947ff..b080326e963 100644 --- a/pkg/registry/core/service/storage/rest.go +++ b/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. - if i > len(service.Spec.IPFamilies)-1 { + // Family is not specified yet. + 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 - if len(service.Spec.ClusterIPs) > 0 && service.Spec.ClusterIPs[0] == api.ClusterIPNone && len(service.Spec.Selector) == 0 { + // Infer IPFamilyPolicy from IPFamilies[]. This block does not handle the + // 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 || - (*(service.Spec.IPFamilyPolicy) == api.IPFamilyPolicyPreferDualStack && len(rs.serviceIPAllocatorsByFamily) == 2) { + if *(service.Spec.IPFamilyPolicy) != api.IPFamilyPolicySingleStack { // 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 - // web hooks, they must have already ran by now + // Finally, if IPFamilyPolicy is *still* not set, we can default it to + // SingleStack. If there are any webhooks, they have already run. if service.Spec.IPFamilyPolicy == nil { singleStack := api.IPFamilyPolicySingleStack service.Spec.IPFamilyPolicy = &singleStack diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index 4721988b9b7..bee9080e6f1 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/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