services strategy no longer depends on IPFamilies

since the refactor on the Service API registry, the strategy for
service no longer needs to keep information about the cluster
configuration and its ipFamilies.

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

@@ -41,7 +41,6 @@ import (
 	"k8s.io/kubernetes/pkg/printers"
 	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
 	printerstorage "k8s.io/kubernetes/pkg/printers/storage"
-	"k8s.io/kubernetes/pkg/registry/core/service"
 	svcreg "k8s.io/kubernetes/pkg/registry/core/service"
 	"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
 	"k8s.io/kubernetes/pkg/registry/core/service/portallocator"
@@ -86,18 +85,16 @@ func NewREST(
 	pods PodStorage,
 	proxyTransport http.RoundTripper) (*REST, *StatusREST, *svcreg.ProxyREST, error) {
 
-	strategy, _ := svcreg.StrategyForServiceCIDRs(ipAllocs[serviceIPFamily].CIDR(), len(ipAllocs) > 1)
-
 	store := &genericregistry.Store{
 		NewFunc:                  func() runtime.Object { return &api.Service{} },
 		NewListFunc:              func() runtime.Object { return &api.ServiceList{} },
 		DefaultQualifiedResource: api.Resource("services"),
 		ReturnDeletedObject:      true,
 
-		CreateStrategy:      strategy,
+		CreateStrategy:      svcreg.Strategy,
-		UpdateStrategy:      strategy,
+		UpdateStrategy:      svcreg.Strategy,
-		DeleteStrategy:      strategy,
+		DeleteStrategy:      svcreg.Strategy,
-		ResetFieldsStrategy: strategy,
+		ResetFieldsStrategy: svcreg.Strategy,
 
 		TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)},
 	}
@@ -107,9 +104,8 @@ func NewREST(
 	}
 
 	statusStore := *store
-	statusStrategy := service.NewServiceStatusStrategy(strategy)
+	statusStore.UpdateStrategy = svcreg.StatusStrategy
-	statusStore.UpdateStrategy = statusStrategy
+	statusStore.ResetFieldsStrategy = svcreg.StatusStrategy
-	statusStore.ResetFieldsStrategy = statusStrategy
 
 	var primaryIPFamily api.IPFamily = serviceIPFamily
 	var secondaryIPFamily api.IPFamily = "" // sentinel value

pkg/registry/core/service/strategy.go

@@ -18,74 +18,29 @@ package service
 
 import (
 	"context"
-	"net"
 	"reflect"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/apiserver/pkg/storage/names"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/core/validation"
 	"k8s.io/kubernetes/pkg/features"
-	netutil "k8s.io/utils/net"
 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
-type Strategy interface {
-	rest.RESTCreateUpdateStrategy
-	rest.ResetFieldsStrategy
-}
-
 // svcStrategy implements behavior for Services
 type svcStrategy struct {
 	runtime.ObjectTyper
 	names.NameGenerator
-
-	ipFamilies []api.IPFamily
 }
 
-// StrategyForServiceCIDRs returns the appropriate service strategy for the given configuration.
+// Strategy is the default logic that applies when creating and updating Services
-func StrategyForServiceCIDRs(primaryCIDR net.IPNet, hasSecondary bool) (Strategy, api.IPFamily) {
+// objects via the REST API.
-	// detect this cluster default Service IPFamily (ipfamily of --service-cluster-ip-range)
+var Strategy = svcStrategy{legacyscheme.Scheme, names.SimpleNameGenerator}
-	// we do it once here, to avoid having to do it over and over during ipfamily assignment
-	serviceIPFamily := api.IPv4Protocol
-	if netutil.IsIPv6CIDR(&primaryCIDR) {
-		serviceIPFamily = api.IPv6Protocol
-	}
-
-	var strategy Strategy
-	switch {
-	case hasSecondary && serviceIPFamily == api.IPv4Protocol:
-		strategy = svcStrategy{
-			ObjectTyper:   legacyscheme.Scheme,
-			NameGenerator: names.SimpleNameGenerator,
-			ipFamilies:    []api.IPFamily{api.IPv4Protocol, api.IPv6Protocol},
-		}
-	case hasSecondary && serviceIPFamily == api.IPv6Protocol:
-		strategy = svcStrategy{
-			ObjectTyper:   legacyscheme.Scheme,
-			NameGenerator: names.SimpleNameGenerator,
-			ipFamilies:    []api.IPFamily{api.IPv6Protocol, api.IPv4Protocol},
-		}
-	case serviceIPFamily == api.IPv6Protocol:
-		strategy = svcStrategy{
-			ObjectTyper:   legacyscheme.Scheme,
-			NameGenerator: names.SimpleNameGenerator,
-			ipFamilies:    []api.IPFamily{api.IPv6Protocol},
-		}
-	default:
-		strategy = svcStrategy{
-			ObjectTyper:   legacyscheme.Scheme,
-			NameGenerator: names.SimpleNameGenerator,
-			ipFamilies:    []api.IPFamily{api.IPv4Protocol},
-		}
-	}
-	return strategy, serviceIPFamily
-}
 
 // NamespaceScoped is true for services.
 func (svcStrategy) NamespaceScoped() bool {
@@ -105,7 +60,7 @@ func (svcStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set {
 }
 
 // PrepareForCreate sets contextual defaults and clears fields that are not allowed to be set by end users on creation.
-func (strategy svcStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
+func (svcStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	service := obj.(*api.Service)
 	service.Status = api.ServiceStatus{}
 
@@ -113,7 +68,7 @@ func (strategy svcStrategy) PrepareForCreate(ctx context.Context, obj runtime.Ob
 }
 
 // PrepareForUpdate sets contextual defaults and clears fields that are not allowed to be set by end users on update.
-func (strategy svcStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
+func (svcStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
 	newService := obj.(*api.Service)
 	oldService := old.(*api.Service)
 	newService.Status = oldService.Status
@@ -123,7 +78,7 @@ func (strategy svcStrategy) PrepareForUpdate(ctx context.Context, obj, old runti
 }
 
 // Validate validates a new service.
-func (strategy svcStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
+func (svcStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
 	service := obj.(*api.Service)
 	allErrs := validation.ValidateServiceCreate(service)
 	allErrs = append(allErrs, validation.ValidateConditionalService(service, nil)...)
@@ -211,13 +166,11 @@ func serviceInternalTrafficPolicyInUse(svc *api.Service) bool {
 }
 
 type serviceStatusStrategy struct {
-	Strategy
+	svcStrategy
 }
 
-// NewServiceStatusStrategy creates a status strategy for the provided base strategy.
+// StatusStrategy wraps and exports the used svcStrategy for the storage package.
-func NewServiceStatusStrategy(strategy Strategy) Strategy {
+var StatusStrategy = serviceStatusStrategy{Strategy}
-	return serviceStatusStrategy{strategy}
-}
 
 // GetResetFields returns the set of fields that get reset by the strategy
 // and should not be modified by the user.

pkg/registry/core/service/strategy_test.go

@@ -31,38 +31,26 @@ import (
 	api "k8s.io/kubernetes/pkg/apis/core"
 	_ "k8s.io/kubernetes/pkg/apis/core/install"
 	"k8s.io/kubernetes/pkg/features"
-	netutils "k8s.io/utils/net"
 	utilpointer "k8s.io/utils/pointer"
 )
 
-func newStrategy(cidr string, hasSecondary bool) (testStrategy Strategy, testStatusStrategy Strategy) {
-	_, testCIDR, err := netutils.ParseCIDRSloppy(cidr)
-	if err != nil {
-		panic("invalid CIDR")
-	}
-	testStrategy, _ = StrategyForServiceCIDRs(*testCIDR, hasSecondary)
-	testStatusStrategy = NewServiceStatusStrategy(testStrategy)
-	return
-}
-
 func TestCheckGeneratedNameError(t *testing.T) {
 	ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{
 		Resource: "foos",
 	})
 
-	testStrategy, _ := newStrategy("10.0.0.0/16", false)
 	expect := errors.NewNotFound(api.Resource("foos"), "bar")
-	if err := rest.CheckGeneratedNameError(ctx, testStrategy, expect, &api.Service{}); err != expect {
+	if err := rest.CheckGeneratedNameError(ctx, Strategy, expect, &api.Service{}); err != expect {
 		t.Errorf("NotFoundError should be ignored: %v", err)
 	}
 
 	expect = errors.NewAlreadyExists(api.Resource("foos"), "bar")
-	if err := rest.CheckGeneratedNameError(ctx, testStrategy, expect, &api.Service{}); err != expect {
+	if err := rest.CheckGeneratedNameError(ctx, Strategy, expect, &api.Service{}); err != expect {
 		t.Errorf("AlreadyExists should be returned when no GenerateName field: %v", err)
 	}
 
 	expect = errors.NewAlreadyExists(api.Resource("foos"), "bar")
-	if err := rest.CheckGeneratedNameError(ctx, testStrategy, expect, &api.Service{ObjectMeta: metav1.ObjectMeta{GenerateName: "foo"}}); err == nil || !errors.IsAlreadyExists(err) {
+	if err := rest.CheckGeneratedNameError(ctx, Strategy, expect, &api.Service{ObjectMeta: metav1.ObjectMeta{GenerateName: "foo"}}); err == nil || !errors.IsAlreadyExists(err) {
 		t.Errorf("expected try again later error: %v", err)
 	}
 }
@@ -114,9 +102,8 @@ func makeValidServiceCustom(tweaks ...func(svc *api.Service)) *api.Service {
 }
 
 func TestServiceStatusStrategy(t *testing.T) {
-	_, testStatusStrategy := newStrategy("10.0.0.0/16", false)
 	ctx := genericapirequest.NewDefaultContext()
-	if !testStatusStrategy.NamespaceScoped() {
+	if !StatusStrategy.NamespaceScoped() {
 		t.Errorf("Service must be namespace scoped")
 	}
 	oldService := makeValidService()
@@ -131,14 +118,14 @@ func TestServiceStatusStrategy(t *testing.T) {
 			},
 		},
 	}
-	testStatusStrategy.PrepareForUpdate(ctx, newService, oldService)
+	StatusStrategy.PrepareForUpdate(ctx, newService, oldService)
 	if newService.Status.LoadBalancer.Ingress[0].IP != "127.0.0.2" {
 		t.Errorf("Service status updates should allow change of status fields")
 	}
 	if newService.Spec.SessionAffinity != "None" {
 		t.Errorf("PrepareForUpdate should have preserved old spec")
 	}
-	errs := testStatusStrategy.ValidateUpdate(ctx, newService, oldService)
+	errs := StatusStrategy.ValidateUpdate(ctx, newService, oldService)
 	if len(errs) != 0 {
 		t.Errorf("Unexpected error %v", errs)
 	}