diff --git a/pkg/registry/core/service/storage/storage.go b/pkg/registry/core/service/storage/storage.go index b5330da1df4..4ee5428e819 100644 --- a/pkg/registry/core/service/storage/storage.go +++ b/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, - UpdateStrategy: strategy, - DeleteStrategy: strategy, - ResetFieldsStrategy: strategy, + CreateStrategy: svcreg.Strategy, + UpdateStrategy: svcreg.Strategy, + DeleteStrategy: svcreg.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 = statusStrategy - statusStore.ResetFieldsStrategy = statusStrategy + statusStore.UpdateStrategy = svcreg.StatusStrategy + statusStore.ResetFieldsStrategy = svcreg.StatusStrategy var primaryIPFamily api.IPFamily = serviceIPFamily var secondaryIPFamily api.IPFamily = "" // sentinel value diff --git a/pkg/registry/core/service/strategy.go b/pkg/registry/core/service/strategy.go index 6dc188d4c59..c7598b68ae4 100644 --- a/pkg/registry/core/service/strategy.go +++ b/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. -func StrategyForServiceCIDRs(primaryCIDR net.IPNet, hasSecondary bool) (Strategy, api.IPFamily) { - // detect this cluster default Service IPFamily (ipfamily of --service-cluster-ip-range) - // 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 -} +// Strategy is the default logic that applies when creating and updating Services +// objects via the REST API. +var Strategy = svcStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} // 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. -func NewServiceStatusStrategy(strategy Strategy) Strategy { - return serviceStatusStrategy{strategy} -} +// StatusStrategy wraps and exports the used svcStrategy for the storage package. +var StatusStrategy = serviceStatusStrategy{Strategy} // GetResetFields returns the set of fields that get reset by the strategy // and should not be modified by the user. diff --git a/pkg/registry/core/service/strategy_test.go b/pkg/registry/core/service/strategy_test.go index 292a87ba784..7769a428b22 100644 --- a/pkg/registry/core/service/strategy_test.go +++ b/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) }