Preserve API group order in discovery, prefer extensions over apps

Kubernetes-commit: 707f0fb131b5ff89caae7c45a0e4b59991bd7bc2

pkg/server/genericapiserver.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"mime"
 	"net/http"
-	"sort"
 	"strings"
 	"sync"
 	"time"
@@ -135,8 +134,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
@@ -374,6 +375,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
 }
 
@@ -381,6 +387,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)
 }
 
@@ -425,14 +441,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 {
-			groupNames = append(groupNames, groupName)
-		}
-		sort.Strings(groupNames)
-		for _, groupName := range groupNames {
+
+		// ranging over apiGroupNamesForDiscovery preserves the registration order
+		for _, groupName := range s.apiGroupNamesForDiscovery {
 			sortedGroups = append(sortedGroups, s.apiGroupsForDiscovery[groupName])
 		}
 

pkg/server/genericapiserver_test.go

@@ -531,6 +531,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{
 		{