From 963c85e1c807efcdbb82dd44439dc3c55f6a0bfd Mon Sep 17 00:00:00 2001 From: deads2k Date: Fri, 9 Jun 2017 16:34:04 -0400 Subject: [PATCH] sort current API versions and fallback for others --- cmd/kube-apiserver/app/aggregator.go | 61 +++++++++++++++---- pkg/master/master.go | 4 ++ .../thirdparty/tprregistration_controller.go | 8 +-- .../tprregistration_controller_test.go | 16 ++--- .../rest/storage_apiserver.go | 2 + pkg/registry/apps/rest/storage_apps.go | 2 + .../rest/storage_authentication.go | 2 + .../rest/storage_authorization.go | 2 + .../autoscaling/rest/storage_autoscaling.go | 2 + pkg/registry/batch/rest/storage_batch.go | 2 + .../certificates/rest/storage_certificates.go | 2 + .../extensions/rest/storage_extensions.go | 2 + .../networking/rest/storage_settings.go | 2 + pkg/registry/policy/rest/storage_policy.go | 2 + pkg/registry/rbac/rest/storage_rbac.go | 2 + .../settings/rest/storage_settings.go | 2 + pkg/registry/storage/rest/storage_storage.go | 2 + .../pkg/apis/apiregistration/helpers.go | 26 +++++--- .../apiregistration/validation/validation.go | 4 +- .../pkg/apiserver/handler_apis.go | 2 - .../pkg/apiserver/handler_apis_test.go | 20 +++--- test/integration/examples/apiserver_test.go | 16 ++--- 22 files changed, 129 insertions(+), 54 deletions(-) diff --git a/cmd/kube-apiserver/app/aggregator.go b/cmd/kube-apiserver/app/aggregator.go index 8bf4f16d48a..38fe52aaac2 100644 --- a/cmd/kube-apiserver/app/aggregator.go +++ b/cmd/kube-apiserver/app/aggregator.go @@ -25,6 +25,8 @@ import ( "net/http" "strings" + "github.com/golang/glog" + apiextensionsinformers "k8s.io/apiextensions-apiserver/pkg/client/informers/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -144,17 +146,59 @@ func createAggregatorServer(aggregatorConfig *aggregatorapiserver.Config, delega } func makeAPIService(gv schema.GroupVersion) *apiregistration.APIService { + apiServicePriority, ok := apiVersionPriorities[gv] + if !ok { + // if we aren't found, then we shouldn't register ourselves because it could result in a CRD group version + // being permanently stuck in the APIServices list. + glog.Infof("Skipping APIService creation for %v", gv) + return nil + } return &apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: gv.Version + "." + gv.Group}, Spec: apiregistration.APIServiceSpec{ - Group: gv.Group, - Version: gv.Version, - GroupPriority: 1000, - VersionPriority: 10, + Group: gv.Group, + Version: gv.Version, + GroupPriorityMinimum: apiServicePriority.group, + VersionPriority: apiServicePriority.version, }, } } +type priority struct { + group int32 + version int32 +} + +// The proper way to resolve this letting the aggregator know the desired group and version-within-group order of the underlying servers +// is to refactor the genericapiserver.DelegationTarget to include a list of priorities based on which APIs were installed. +// This requires the APIGroupInfo struct to evolve and include the concept of priorities and to avoid mistakes, the core storage map there needs to be updated. +// That ripples out every bit as far as you'd expect, so for 1.7 we'll include the list here instead of being built up during storage. +var apiVersionPriorities = map[schema.GroupVersion]priority{ + {Group: "", Version: "v1"}: {group: 18000, version: 1}, + // extensions is above the rest for CLI compatibility, though the level of unqalified resource compatibility we + // can reasonably expect seems questionable. + {Group: "extensions", Version: "v1beta1"}: {group: 17900, version: 1}, + // to my knowledge, nothing below here collides + {Group: "apps", Version: "v1beta1"}: {group: 17800, version: 1}, + {Group: "authentication.k8s.io", Version: "v1"}: {group: 17700, version: 15}, + {Group: "authentication.k8s.io", Version: "v1beta1"}: {group: 17700, version: 9}, + {Group: "authorization.k8s.io", Version: "v1"}: {group: 17600, version: 15}, + {Group: "authorization.k8s.io", Version: "v1beta1"}: {group: 17600, version: 9}, + {Group: "autoscaling", Version: "v1"}: {group: 17500, version: 15}, + {Group: "autoscaling", Version: "v2alpha1"}: {group: 17500, version: 9}, + {Group: "batch", Version: "v1"}: {group: 17400, version: 15}, + {Group: "batch", Version: "v2alpha1"}: {group: 17400, version: 9}, + {Group: "certificates.k8s.io", Version: "v1beta1"}: {group: 17300, version: 9}, + {Group: "networking.k8s.io", Version: "v1"}: {group: 17200, version: 15}, + {Group: "policy", Version: "v1beta1"}: {group: 17100, version: 9}, + {Group: "rbac.authorization.k8s.io", Version: "v1beta1"}: {group: 17000, version: 12}, + {Group: "rbac.authorization.k8s.io", Version: "v1alpha1"}: {group: 17000, version: 9}, + {Group: "settings.k8s.io", Version: "v1alpha1"}: {group: 16900, version: 9}, + {Group: "storage.k8s.io", Version: "v1"}: {group: 16800, version: 15}, + {Group: "storage.k8s.io", Version: "v1beta1"}: {group: 16800, version: 9}, + {Group: "apiextensions.k8s.io", Version: "v1beta1"}: {group: 16700, version: 9}, +} + func apiServicesToRegister(delegateAPIServer genericapiserver.DelegationTarget, registration autoregister.AutoAPIServiceRegistration) []*apiregistration.APIService { apiServices := []*apiregistration.APIService{} @@ -176,14 +220,9 @@ func apiServicesToRegister(delegateAPIServer genericapiserver.DelegationTarget, } apiService := makeAPIService(schema.GroupVersion{Group: tokens[2], Version: tokens[3]}) - - // TODO this is probably an indication that we need explicit and precise control over the discovery chain - // but for now its a special case - // apps has to come last for compatibility with 1.5 kubectl clients - if apiService.Spec.Group == "apps" { - apiService.Spec.GroupPriority = 1100 + if apiService == nil { + continue } - registration.AddAPIServiceToSync(apiService) apiServices = append(apiServices, apiService) } diff --git a/pkg/master/master.go b/pkg/master/master.go index 7574a657df0..6760d4e9b18 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -249,6 +249,10 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget, // The order here is preserved in discovery. // If resources with identical names exist in more than one of these groups (e.g. "deployments.apps"" and "deployments.extensions"), // the order of this list determines which group an unqualified resource name (e.g. "deployments") should prefer. + // This priority order is used for local discovery, but it ends up aggregated in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go + // with specific priorities. + // TODO: describe the priority all the way down in the RESTStorageProviders and plumb it back through the various discovery + // handlers that we have. restStorageProviders := []RESTStorageProvider{ authenticationrest.RESTStorageProvider{Authenticator: c.GenericConfig.Authenticator}, authorizationrest.RESTStorageProvider{Authorizer: c.GenericConfig.Authorizer}, diff --git a/pkg/master/thirdparty/tprregistration_controller.go b/pkg/master/thirdparty/tprregistration_controller.go index 5cf9d8a488d..6e0bcc3fac5 100644 --- a/pkg/master/thirdparty/tprregistration_controller.go +++ b/pkg/master/thirdparty/tprregistration_controller.go @@ -255,10 +255,10 @@ func (c *tprRegistrationController) handleVersionUpdate(groupVersion schema.Grou c.apiServiceRegistration.AddAPIServiceToSync(&apiregistration.APIService{ ObjectMeta: metav1.ObjectMeta{Name: apiServiceName}, Spec: apiregistration.APIServiceSpec{ - Group: groupVersion.Group, - Version: groupVersion.Version, - GroupPriority: 5000, // TPRs should have relatively low priority - VersionPriority: 20, // TPRs should have relatively low priority + Group: groupVersion.Group, + Version: groupVersion.Version, + GroupPriorityMinimum: 1000, // TPRs should have relatively low priority + VersionPriority: 100, // TPRs should have relatively low priority }, }) diff --git a/pkg/master/thirdparty/tprregistration_controller_test.go b/pkg/master/thirdparty/tprregistration_controller_test.go index 3c5d83010f6..97c6e28ec2f 100644 --- a/pkg/master/thirdparty/tprregistration_controller_test.go +++ b/pkg/master/thirdparty/tprregistration_controller_test.go @@ -84,10 +84,10 @@ func TestHandleVersionUpdate(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{Name: "v1.group.com"}, Spec: apiregistration.APIServiceSpec{ - Group: "group.com", - Version: "v1", - GroupPriority: 5000, - VersionPriority: 20, + Group: "group.com", + Version: "v1", + GroupPriorityMinimum: 1000, + VersionPriority: 100, }, }, }, @@ -122,10 +122,10 @@ func TestHandleVersionUpdate(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{Name: "v1.group.com"}, Spec: apiregistration.APIServiceSpec{ - Group: "group.com", - Version: "v1", - GroupPriority: 5000, - VersionPriority: 20, + Group: "group.com", + Version: "v1", + GroupPriorityMinimum: 1000, + VersionPriority: 100, }, }, }, diff --git a/pkg/registry/admissionregistration/rest/storage_apiserver.go b/pkg/registry/admissionregistration/rest/storage_apiserver.go index a315cabf5d9..a5dd06d803f 100644 --- a/pkg/registry/admissionregistration/rest/storage_apiserver.go +++ b/pkg/registry/admissionregistration/rest/storage_apiserver.go @@ -32,6 +32,8 @@ type RESTStorageProvider struct{} func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(admissionregistration.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(admissionregistrationv1alpha1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[admissionregistrationv1alpha1.SchemeGroupVersion.Version] = p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/apps/rest/storage_apps.go b/pkg/registry/apps/rest/storage_apps.go index 56f98143b21..339451b3d2d 100644 --- a/pkg/registry/apps/rest/storage_apps.go +++ b/pkg/registry/apps/rest/storage_apps.go @@ -33,6 +33,8 @@ type RESTStorageProvider struct{} func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(apps.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(appsapiv1beta1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[appsapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/authentication/rest/storage_authentication.go b/pkg/registry/authentication/rest/storage_authentication.go index 4f8d1ce698b..f8f47939012 100644 --- a/pkg/registry/authentication/rest/storage_authentication.go +++ b/pkg/registry/authentication/rest/storage_authentication.go @@ -40,6 +40,8 @@ func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorag // } apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(authentication.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(authenticationv1beta1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[authenticationv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/authorization/rest/storage_authorization.go b/pkg/registry/authorization/rest/storage_authorization.go index f875e5c5573..a543949fb26 100644 --- a/pkg/registry/authorization/rest/storage_authorization.go +++ b/pkg/registry/authorization/rest/storage_authorization.go @@ -41,6 +41,8 @@ func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorag } apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(authorization.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(authorizationv1beta1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[authorizationv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/autoscaling/rest/storage_autoscaling.go b/pkg/registry/autoscaling/rest/storage_autoscaling.go index 60b3bf3a19d..5fcbfc1ad2c 100644 --- a/pkg/registry/autoscaling/rest/storage_autoscaling.go +++ b/pkg/registry/autoscaling/rest/storage_autoscaling.go @@ -32,6 +32,8 @@ type RESTStorageProvider struct{} func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(autoscaling.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(autoscalingapiv2alpha1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[autoscalingapiv2alpha1.SchemeGroupVersion.Version] = p.v2alpha1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/batch/rest/storage_batch.go b/pkg/registry/batch/rest/storage_batch.go index d523b8ab087..55582a80291 100644 --- a/pkg/registry/batch/rest/storage_batch.go +++ b/pkg/registry/batch/rest/storage_batch.go @@ -34,6 +34,8 @@ type RESTStorageProvider struct{} func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(batch.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(batchapiv2alpha1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[batchapiv2alpha1.SchemeGroupVersion.Version] = p.v2alpha1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/certificates/rest/storage_certificates.go b/pkg/registry/certificates/rest/storage_certificates.go index 7a6a6437326..c03fb9ecea0 100644 --- a/pkg/registry/certificates/rest/storage_certificates.go +++ b/pkg/registry/certificates/rest/storage_certificates.go @@ -31,6 +31,8 @@ type RESTStorageProvider struct{} func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(certificates.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(certificatesapiv1beta1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[certificatesapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/extensions/rest/storage_extensions.go b/pkg/registry/extensions/rest/storage_extensions.go index f0cfe5518c6..ec3bdeb0cf4 100644 --- a/pkg/registry/extensions/rest/storage_extensions.go +++ b/pkg/registry/extensions/rest/storage_extensions.go @@ -48,6 +48,8 @@ type RESTStorageProvider struct { func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(extensions.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(extensionsapiv1beta1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[extensionsapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/networking/rest/storage_settings.go b/pkg/registry/networking/rest/storage_settings.go index c662025dd22..3b773bd9e34 100644 --- a/pkg/registry/networking/rest/storage_settings.go +++ b/pkg/registry/networking/rest/storage_settings.go @@ -31,6 +31,8 @@ type RESTStorageProvider struct{} func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(networking.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(networkingapiv1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[networkingapiv1.SchemeGroupVersion.Version] = p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/policy/rest/storage_policy.go b/pkg/registry/policy/rest/storage_policy.go index c1067d5b0f9..26a9594eebd 100644 --- a/pkg/registry/policy/rest/storage_policy.go +++ b/pkg/registry/policy/rest/storage_policy.go @@ -31,6 +31,8 @@ type RESTStorageProvider struct{} func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(policy.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(policyapiv1beta1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[policyapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/rbac/rest/storage_rbac.go b/pkg/registry/rbac/rest/storage_rbac.go index fdddc867f32..34df168d877 100644 --- a/pkg/registry/rbac/rest/storage_rbac.go +++ b/pkg/registry/rbac/rest/storage_rbac.go @@ -65,6 +65,8 @@ var _ genericapiserver.PostStartHookProvider = RESTStorageProvider{} func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(rbac.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(rbacapiv1alpha1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[rbacapiv1alpha1.SchemeGroupVersion.Version] = p.storage(rbacapiv1alpha1.SchemeGroupVersion, apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/settings/rest/storage_settings.go b/pkg/registry/settings/rest/storage_settings.go index 8e9a41b4db4..949faac847c 100644 --- a/pkg/registry/settings/rest/storage_settings.go +++ b/pkg/registry/settings/rest/storage_settings.go @@ -31,6 +31,8 @@ type RESTStorageProvider struct{} func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(settings.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(settingsapiv1alpha1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[settingsapiv1alpha1.SchemeGroupVersion.Version] = p.v1alpha1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/pkg/registry/storage/rest/storage_storage.go b/pkg/registry/storage/rest/storage_storage.go index c12f8038243..0eebcd0dbee 100644 --- a/pkg/registry/storage/rest/storage_storage.go +++ b/pkg/registry/storage/rest/storage_storage.go @@ -33,6 +33,8 @@ type RESTStorageProvider struct { func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, bool) { apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(storageapi.GroupName, api.Registry, api.Scheme, api.ParameterCodec, api.Codecs) + // If you add a version here, be sure to add an entry in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go with specific priorities. + // TODO refactor the plumbing to provide the information in the APIGroupInfo if apiResourceConfigSource.AnyResourcesForVersionEnabled(storageapiv1beta1.SchemeGroupVersion) { apiGroupInfo.VersionedResourcesStorageMap[storageapiv1beta1.SchemeGroupVersion.Version] = p.v1beta1Storage(apiResourceConfigSource, restOptionsGetter) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers.go b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers.go index 74a1be927fa..1f6790b32e3 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/helpers.go @@ -24,11 +24,11 @@ import ( ) func SortedByGroupAndVersion(servers []*APIService) [][]*APIService { - serversByGroupPriority := ByGroupPriority(servers) - sort.Sort(serversByGroupPriority) + serversByGroupPriorityMinimum := ByGroupPriorityMinimum(servers) + sort.Sort(serversByGroupPriorityMinimum) ret := [][]*APIService{} - for _, curr := range serversByGroupPriority { + for _, curr := range serversByGroupPriorityMinimum { // check to see if we already have an entry for this group existingIndex := -1 for j, groupInReturn := range ret { @@ -50,24 +50,30 @@ func SortedByGroupAndVersion(servers []*APIService) [][]*APIService { return ret } -type ByGroupPriority []*APIService +// ByGroupPriorityMinimum sorts with the highest group number first, then by name. +// This is not a simple reverse, because we want the name sorting to be alpha, not +// reverse alpha. +type ByGroupPriorityMinimum []*APIService -func (s ByGroupPriority) Len() int { return len(s) } -func (s ByGroupPriority) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -func (s ByGroupPriority) Less(i, j int) bool { - if s[i].Spec.GroupPriority != s[j].Spec.GroupPriority { - return s[i].Spec.GroupPriority < s[j].Spec.GroupPriority +func (s ByGroupPriorityMinimum) Len() int { return len(s) } +func (s ByGroupPriorityMinimum) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s ByGroupPriorityMinimum) Less(i, j int) bool { + if s[i].Spec.GroupPriorityMinimum != s[j].Spec.GroupPriorityMinimum { + return s[i].Spec.GroupPriorityMinimum > s[j].Spec.GroupPriorityMinimum } return s[i].Name < s[j].Name } +// ByVersionPriority sorts with the highest version number first, then by name. +// This is not a simple reverse, because we want the name sorting to be alpha, not +// reverse alpha. type ByVersionPriority []*APIService func (s ByVersionPriority) Len() int { return len(s) } func (s ByVersionPriority) Swap(i, j int) { s[i], s[j] = s[j], s[i] } func (s ByVersionPriority) Less(i, j int) bool { if s[i].Spec.VersionPriority != s[j].Spec.VersionPriority { - return s[i].Spec.VersionPriority < s[j].Spec.VersionPriority + return s[i].Spec.VersionPriority > s[j].Spec.VersionPriority } return s[i].Name < s[j].Name } diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/validation/validation.go b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/validation/validation.go index 6eda8bcca2d..725cf71337f 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/validation/validation.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/validation/validation.go @@ -58,8 +58,8 @@ func ValidateAPIService(apiService *apiregistration.APIService) field.ErrorList allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), apiService.Spec.Version, errString)) } - if apiService.Spec.GroupPriorityMinimum <= 0 || apiService.Spec.GroupPriorityMinimum > 10000 { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "groupPriorityMinimum"), apiService.Spec.GroupPriorityMinimum, "must be positive and less than 10000")) + if apiService.Spec.GroupPriorityMinimum <= 0 || apiService.Spec.GroupPriorityMinimum > 20000 { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "groupPriorityMinimum"), apiService.Spec.GroupPriorityMinimum, "must be positive and less than 20000")) } if apiService.Spec.VersionPriority <= 0 || apiService.Spec.VersionPriority > 1000 { allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "versionPriority"), apiService.Spec.VersionPriority, "must be positive and less than 1000")) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis.go index 09b632f5b8a..e7b15b18821 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis.go @@ -18,7 +18,6 @@ package apiserver import ( "errors" - "fmt" "net/http" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -97,7 +96,6 @@ func convertToDiscoveryAPIGroup(apiServices []*apiregistrationapi.APIService) *m var discoveryGroup *metav1.APIGroup for _, apiService := range apiServicesByGroup { - fmt.Printf("#### %#v\n", apiService) if !apiregistrationapi.IsAPIServiceConditionTrue(apiService, apiregistrationapi.Available) { continue } diff --git a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis_test.go b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis_test.go index d2dd3b028ad..d3aba698bd3 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis_test.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis_test.go @@ -62,7 +62,7 @@ func TestAPIs(t *testing.T) { }, Group: "foo", Version: "v1", - GroupPriorityMinimum: 10, + GroupPriorityMinimum: 11, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -79,7 +79,7 @@ func TestAPIs(t *testing.T) { }, Group: "bar", Version: "v1", - GroupPriorityMinimum: 11, + GroupPriorityMinimum: 10, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -133,8 +133,8 @@ func TestAPIs(t *testing.T) { }, Group: "foo", Version: "v1", - GroupPriorityMinimum: 1, - VersionPriority: 15, + GroupPriorityMinimum: 20, + VersionPriority: 10, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -168,8 +168,8 @@ func TestAPIs(t *testing.T) { }, Group: "foo", Version: "v2", - GroupPriorityMinimum: 20, - VersionPriority: 10, + GroupPriorityMinimum: 1, + VersionPriority: 15, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -340,8 +340,8 @@ func TestAPIGroup(t *testing.T) { }, Group: "foo", Version: "v1", - GroupPriorityMinimum: 1, - VersionPriority: 15, + GroupPriorityMinimum: 20, + VersionPriority: 10, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ @@ -375,8 +375,8 @@ func TestAPIGroup(t *testing.T) { }, Group: "foo", Version: "v2", - GroupPriorityMinimum: 20, - VersionPriority: 10, + GroupPriorityMinimum: 1, + VersionPriority: 15, }, Status: apiregistration.APIServiceStatus{ Conditions: []apiregistration.APIServiceCondition{ diff --git a/test/integration/examples/apiserver_test.go b/test/integration/examples/apiserver_test.go index f2fb02a2fa0..a940efe81f9 100644 --- a/test/integration/examples/apiserver_test.go +++ b/test/integration/examples/apiserver_test.go @@ -315,10 +315,11 @@ func TestAggregatedAPIServer(t *testing.T) { Namespace: "kube-wardle", Name: "api", }, - Group: "wardle.k8s.io", - Version: "v1alpha1", - CABundle: wardleCA, - Priority: 200, + Group: "wardle.k8s.io", + Version: "v1alpha1", + CABundle: wardleCA, + GroupPriorityMinimum: 200, + VersionPriority: 200, }, }) if err != nil { @@ -337,9 +338,10 @@ func TestAggregatedAPIServer(t *testing.T) { Spec: apiregistrationv1beta1.APIServiceSpec{ // register this as a loca service so it doesn't try to lookup the default kubernetes service // which will have an unroutable IP address since its fake. - Group: "", - Version: "v1", - Priority: 100, + Group: "", + Version: "v1", + GroupPriorityMinimum: 100, + VersionPriority: 100, }, }) if err != nil {