separate group and version priority

api/openapi-spec/swagger.json

@@ -46746,7 +46746,8 @@
     "required": [
      "service",
      "caBundle",
-     "priority"
+     "groupPriorityMinimum",
+     "versionPriority"
     ],
     "properties": {
      "caBundle": {
@@ -46758,22 +46759,27 @@
       "description": "Group is the API group name this server hosts",
       "type": "string"
      },
+     "groupPriorityMinimum": {
+      "description": "GroupPriorityMininum is the priority this group should have at least. Higher priority means that the group is prefered by clients over lower priority ones. Note that other versions of this group might specify even higher GroupPriorityMininum values such that the whole group gets a higher priority. The primary sort is based on GroupPriorityMinimum, ordered highest number to lowest (20 before 10). The secondary sort is based on the alphabetical comparison of the name of the object.  (v1.bar before v1.foo) We'd recommend something like: *.k8s.io (except extensions) at 18000 and PaaSes (OpenShift, Deis) are recommended to be in the 2000s",
+      "type": "integer",
+      "format": "int32"
+     },
      "insecureSkipTLSVerify": {
       "description": "InsecureSkipTLSVerify disables TLS certificate verification when communicating with this server. This is strongly discouraged.  You should use the CABundle instead.",
       "type": "boolean"
      },
-     "priority": {
-      "description": "Priority controls the ordering of this API group in the overall discovery document that gets served. Client tools like `kubectl` use this ordering to derive preference, so this ordering mechanism is important. Values must be between 1 and 1000 The primary sort is based on priority, ordered lowest number to highest (10 before 20). The secondary sort is based on the alphabetical comparison of the name of the object.  (v1.bar before v1.foo) We'd recommend something like: *.k8s.io (except extensions) at 100, extensions at 150 PaaSes (OpenShift, Deis) are recommended to be in the 200s",
-      "type": "integer",
-      "format": "int64"
-     },
      "service": {
       "description": "Service is a reference to the service for this API server.  It must communicate on port 443 If the Service is nil, that means the handling for the API groupversion is handled locally on this server. The call will simply delegate to the normal handler chain to be fulfilled.",
       "$ref": "#/definitions/io.k8s.kube-aggregator.pkg.apis.apiregistration.v1beta1.ServiceReference"
      },
      "version": {
       "description": "Version is the API version this server hosts.  For example, \"v1\"",
       "type": "string"
+     },
+     "versionPriority": {
+      "description": "VersionPriority controls the ordering of this API version inside of its group.  Must be greater than zero. The primary sort is based on VersionPriority, ordered highest to lowest (20 before 10). The secondary sort is based on the alphabetical comparison of the name of the object.  (v1.bar before v1.foo) Since it's inside of a group, the number can be small, probably in the 10s.",
+      "type": "integer",
+      "format": "int32"
      }
     }
    },

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,16 +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,
-			Priority: 100,
+			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{}
 
@@ -175,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.Priority = 110
+		if apiService == nil {
+			continue
 		}
-
 		registration.AddAPIServiceToSync(apiService)
 		apiServices = append(apiServices, apiService)
 	}

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},

pkg/master/thirdparty/tprregistration_controller.go

@@ -255,9 +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,
-			Priority: 500, // 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
 		},
 	})
 

pkg/master/thirdparty/tprregistration_controller_test.go

@@ -84,9 +84,10 @@ func TestHandleVersionUpdate(t *testing.T) {
 				{
 					ObjectMeta: metav1.ObjectMeta{Name: "v1.group.com"},
 					Spec: apiregistration.APIServiceSpec{
-						Group:    "group.com",
-						Version:  "v1",
-						Priority: 500,
+						Group:                "group.com",
+						Version:              "v1",
+						GroupPriorityMinimum: 1000,
+						VersionPriority:      100,
 					},
 				},
 			},
@@ -121,9 +122,10 @@ func TestHandleVersionUpdate(t *testing.T) {
 				{
 					ObjectMeta: metav1.ObjectMeta{Name: "v1.group.com"},
 					Spec: apiregistration.APIServiceSpec{
-						Group:    "group.com",
-						Version:  "v1",
-						Priority: 500,
+						Group:                "group.com",
+						Version:              "v1",
+						GroupPriorityMinimum: 1000,
+						VersionPriority:      100,
 					},
 				},
 			},

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)

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)

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)

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)

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)

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)

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)

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)

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)

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)

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)

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)