Preserve API group order in discovery, prefer extensions over apps

api/swagger-spec/resourceListing.json

@@ -21,14 +21,6 @@
     "path": "/api",
     "description": "get available API versions"
    },
-   {
-    "path": "/apis/apps/v1beta1",
-    "description": "API at /apis/apps/v1beta1"
-   },
-   {
-    "path": "/apis/apps",
-    "description": "get information of a group"
-   },
    {
     "path": "/apis/authentication.k8s.io/v1",
     "description": "API at /apis/authentication.k8s.io/v1"
@@ -113,6 +105,14 @@
     "path": "/apis/rbac.authorization.k8s.io",
     "description": "get information of a group"
    },
+   {
+    "path": "/apis/settings.k8s.io/v1alpha1",
+    "description": "API at /apis/settings.k8s.io/v1alpha1"
+   },
+   {
+    "path": "/apis/settings.k8s.io",
+    "description": "get information of a group"
+   },
    {
     "path": "/apis/storage.k8s.io/v1beta1",
     "description": "API at /apis/storage.k8s.io/v1beta1"
@@ -126,11 +126,11 @@
     "description": "get information of a group"
    },
    {
-    "path": "/apis/settings.k8s.io/v1alpha1",
+    "path": "/apis/apps/v1beta1",
-    "description": "API at /apis/settings.k8s.io/v1alpha1"
+    "description": "API at /apis/apps/v1beta1"
    },
    {
-    "path": "/apis/settings.k8s.io",
+    "path": "/apis/apps",
     "description": "get information of a group"
    }
   ],

hack/make-rules/test-cmd-util.sh

@@ -1151,7 +1151,7 @@ run_kubectl_get_tests() {
   kube::test::if_has_string "${output_message}" "/apis/apps/v1beta1/namespaces/default/statefulsets 200 OK"
   kube::test::if_has_string "${output_message}" "/apis/autoscaling/v1/namespaces/default/horizontalpodautoscalers 200"
   kube::test::if_has_string "${output_message}" "/apis/batch/v1/namespaces/default/jobs 200 OK"
-  kube::test::if_has_string "${output_message}" "/apis/apps/v1beta1/namespaces/default/deployments 200 OK"
+  kube::test::if_has_string "${output_message}" "/apis/extensions/v1beta1/namespaces/default/deployments 200 OK"
   kube::test::if_has_string "${output_message}" "/apis/extensions/v1beta1/namespaces/default/replicasets 200 OK"
 
   ### Test --allow-missing-template-keys

pkg/master/master.go

@@ -240,8 +240,10 @@ func (c completedConfig) New() (*Master, error) {
 		m.InstallLegacyAPI(c.Config, c.Config.GenericConfig.RESTOptionsGetter, legacyRESTStorageProvider)
 	}
 
+	// 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.
 	restStorageProviders := []RESTStorageProvider{
-		appsrest.RESTStorageProvider{},
 		authenticationrest.RESTStorageProvider{Authenticator: c.GenericConfig.Authenticator},
 		authorizationrest.RESTStorageProvider{Authorizer: c.GenericConfig.Authorizer},
 		autoscalingrest.RESTStorageProvider{},
@@ -250,8 +252,11 @@ func (c completedConfig) New() (*Master, error) {
 		extensionsrest.RESTStorageProvider{ResourceInterface: thirdparty.NewThirdPartyResourceServer(s, c.StorageFactory)},
 		policyrest.RESTStorageProvider{},
 		rbacrest.RESTStorageProvider{Authorizer: c.GenericConfig.Authorizer},
-		storagerest.RESTStorageProvider{},
 		settingsrest.RESTStorageProvider{},
+		storagerest.RESTStorageProvider{},
+		// keep apps after extensions so legacy clients resolve the extensions versions of shared resource names.
+		// See https://github.com/kubernetes/kubernetes/issues/42392
+		appsrest.RESTStorageProvider{},
 	}
 	m.InstallAPIs(c.Config.APIResourceConfigSource, c.Config.GenericConfig.RESTOptionsGetter, restStorageProviders...)
 

staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"mime"
 	"net/http"
-	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -128,8 +127,10 @@ type GenericAPIServer struct {
 
 	// Map storing information about all groups to be exposed in discovery response.
 	// The map is from name to the group.
+	// The slice preserves group name insertion order.
 	apiGroupsForDiscoveryLock sync.RWMutex
 	apiGroupsForDiscovery     map[string]metav1.APIGroup
+	apiGroupNamesForDiscovery []string
 
 	// Enable swagger and/or OpenAPI if these configs are non-nil.
 	swaggerConfig *swagger.Config
@@ -334,6 +335,11 @@ func (s *GenericAPIServer) AddAPIGroupForDiscovery(apiGroup metav1.APIGroup) {
 	s.apiGroupsForDiscoveryLock.Lock()
 	defer s.apiGroupsForDiscoveryLock.Unlock()
 
+	// Insert the group into the ordered list if it is not already present
+	if _, exists := s.apiGroupsForDiscovery[apiGroup.Name]; !exists {
+		s.apiGroupNamesForDiscovery = append(s.apiGroupNamesForDiscovery, apiGroup.Name)
+	}
+
 	s.apiGroupsForDiscovery[apiGroup.Name] = apiGroup
 }
 
@@ -341,6 +347,16 @@ func (s *GenericAPIServer) RemoveAPIGroupForDiscovery(groupName string) {
 	s.apiGroupsForDiscoveryLock.Lock()
 	defer s.apiGroupsForDiscoveryLock.Unlock()
 
+	// Remove the group from the ordered list
+	// https://github.com/golang/go/wiki/SliceTricks#filtering-without-allocating
+	newOrder := s.apiGroupNamesForDiscovery[:0]
+	for _, orderedGroupName := range s.apiGroupNamesForDiscovery {
+		if orderedGroupName != groupName {
+			newOrder = append(newOrder, orderedGroupName)
+		}
+	}
+	s.apiGroupNamesForDiscovery = newOrder
+
 	delete(s.apiGroupsForDiscovery, groupName)
 }
 
@@ -385,14 +401,10 @@ func (s *GenericAPIServer) DynamicApisDiscovery() *restful.WebService {
 		s.apiGroupsForDiscoveryLock.RLock()
 		defer s.apiGroupsForDiscoveryLock.RUnlock()
 
-		// sort to have a deterministic order
 		sortedGroups := []metav1.APIGroup{}
-		groupNames := make([]string, 0, len(s.apiGroupsForDiscovery))
+
-		for groupName := range s.apiGroupsForDiscovery {
+		// ranging over apiGroupNamesForDiscovery preserves the registration order
-			groupNames = append(groupNames, groupName)
+		for _, groupName := range s.apiGroupNamesForDiscovery {
-		}
-		sort.Strings(groupNames)
-		for _, groupName := range groupNames {
 			sortedGroups = append(sortedGroups, s.apiGroupsForDiscovery[groupName])
 		}
 

staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go

@@ -529,6 +529,64 @@ func TestDiscoveryAtAPIS(t *testing.T) {
 	assert.Equal(0, len(groupList.Groups))
 }
 
+func TestDiscoveryOrdering(t *testing.T) {
+	master, etcdserver, _, assert := newMaster(t)
+	defer etcdserver.Terminate(t)
+
+	server := httptest.NewServer(master.InsecureHandler)
+	groupList, err := getGroupList(server)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	assert.Equal(0, len(groupList.Groups))
+
+	// Register three groups
+	master.AddAPIGroupForDiscovery(metav1.APIGroup{Name: "x"})
+	master.AddAPIGroupForDiscovery(metav1.APIGroup{Name: "y"})
+	master.AddAPIGroupForDiscovery(metav1.APIGroup{Name: "z"})
+	// Register three additional groups that come earlier alphabetically
+	master.AddAPIGroupForDiscovery(metav1.APIGroup{Name: "a"})
+	master.AddAPIGroupForDiscovery(metav1.APIGroup{Name: "b"})
+	master.AddAPIGroupForDiscovery(metav1.APIGroup{Name: "c"})
+	// Make sure re-adding doesn't double-register or make a group lose its place
+	master.AddAPIGroupForDiscovery(metav1.APIGroup{Name: "x"})
+
+	groupList, err = getGroupList(server)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	assert.Equal(6, len(groupList.Groups))
+	assert.Equal("x", groupList.Groups[0].Name)
+	assert.Equal("y", groupList.Groups[1].Name)
+	assert.Equal("z", groupList.Groups[2].Name)
+	assert.Equal("a", groupList.Groups[3].Name)
+	assert.Equal("b", groupList.Groups[4].Name)
+	assert.Equal("c", groupList.Groups[5].Name)
+
+	// Remove a group.
+	master.RemoveAPIGroupForDiscovery("a")
+	groupList, err = getGroupList(server)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	assert.Equal(5, len(groupList.Groups))
+
+	// Re-adding should move to the end.
+	master.AddAPIGroupForDiscovery(metav1.APIGroup{Name: "a"})
+	groupList, err = getGroupList(server)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	assert.Equal(6, len(groupList.Groups))
+	assert.Equal("x", groupList.Groups[0].Name)
+	assert.Equal("y", groupList.Groups[1].Name)
+	assert.Equal("z", groupList.Groups[2].Name)
+	assert.Equal("b", groupList.Groups[3].Name)
+	assert.Equal("c", groupList.Groups[4].Name)
+	assert.Equal("a", groupList.Groups[5].Name)
+}
+
 func TestGetServerAddressByClientCIDRs(t *testing.T) {
 	publicAddressCIDRMap := []metav1.ServerAddressByClientCIDR{
 		{