diff --git a/api/swagger-spec/resourceListing.json b/api/swagger-spec/resourceListing.json index 761bb44535c..3bdebc17147 100644 --- a/api/swagger-spec/resourceListing.json +++ b/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", - "description": "API at /apis/settings.k8s.io/v1alpha1" + "path": "/apis/apps/v1beta1", + "description": "API at /apis/apps/v1beta1" }, { - "path": "/apis/settings.k8s.io", + "path": "/apis/apps", "description": "get information of a group" } ], diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 1f3ed7b95ee..2d2f16fc892 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/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 diff --git a/pkg/master/master.go b/pkg/master/master.go index ea0f87232b3..95bea0dcc37 100644 --- a/pkg/master/master.go +++ b/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...) diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go index e129120bcf1..331e3a447bd 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go +++ b/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 { - 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]) } diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go index d2895b1d5cd..8868e8b9cb7 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver_test.go +++ b/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{ {