Merge pull request #113764 from Jefftree/foobar

Fix bugs with aggregated discovery

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_discovery_controller.go

@@ -269,7 +269,7 @@ func (c *DiscoveryController) sync(version schema.GroupVersion) error {
 			Resources: aggregatedApiResourcesForDiscovery,
 		})
 		// Default priority for CRDs
-		c.resourceManager.SetGroupPriority(version.Group, 1000)
+		c.resourceManager.SetGroupVersionPriority(metav1.GroupVersion(version), 1000, 100)
 	}
 	return nil
 }

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_discovery_controller_test.go

@@ -273,13 +273,17 @@ func TestResourceManagerExistingCRD(t *testing.T) {
 
 	env.FakeResourceManager.Expect().
 		AddGroupVersion(coolFooCRD.Spec.Group, coolFooDiscovery)
-	env.FakeResourceManager.Expect().
-		SetGroupPriority(coolFooCRD.Spec.Group, 1000)
+	for _, v := range coolFooCRD.Spec.Versions {
+		env.FakeResourceManager.Expect().
+			SetGroupVersionPriority(metav1.GroupVersion{Group: coolFooCRD.Spec.Group, Version: v.Name}, 1000, 100)
+	}
 
 	env.FakeResourceManager.Expect().
 		AddGroupVersion(coolFooCRD.Spec.Group, coolFooDiscovery)
-	env.FakeResourceManager.Expect().
-		SetGroupPriority(coolFooCRD.Spec.Group, 1000)
+	for _, v := range coolFooCRD.Spec.Versions {
+		env.FakeResourceManager.Expect().
+			SetGroupVersionPriority(metav1.GroupVersion{Group: coolFooCRD.Spec.Group, Version: v.Name}, 1000, 100)
+	}
 
 	env.Start(ctx)
 	err = env.FakeResourceManager.WaitForActions(ctx, 1*time.Second)
@@ -295,7 +299,10 @@ func TestResourceManagerAddedCRD(t *testing.T) {
 	env := setup()
 	env.FakeResourceManager.Expect().
 		AddGroupVersion(coolFooCRD.Spec.Group, coolFooDiscovery)
-	env.FakeResourceManager.Expect().SetGroupPriority(coolFooCRD.Spec.Group, 1000)
+	for _, v := range coolFooCRD.Spec.Versions {
+		env.FakeResourceManager.Expect().
+			SetGroupVersionPriority(metav1.GroupVersion{Group: coolFooCRD.Spec.Group, Version: v.Name}, 1000, 100)
+	}
 
 	env.Start(ctx)
 
@@ -340,7 +347,9 @@ func TestMultipleCRDSameVersion(t *testing.T) {
 	require.NoError(t, err)
 	env.FakeResourceManager.Expect().
 		AddGroupVersion(coolFooCRD.Spec.Group, coolFooDiscovery)
-	env.FakeResourceManager.Expect().SetGroupPriority(coolFooCRD.Spec.Group, 1000)
+	for _, versionEntry := range coolFooCRD.Spec.Versions {
+		env.FakeResourceManager.Expect().SetGroupVersionPriority(metav1.GroupVersion{Group: coolFooCRD.Spec.Group, Version: versionEntry.Name}, 1000, 100)
+	}
 	err = env.FakeResourceManager.WaitForActions(ctx, 1*time.Second)
 	require.NoError(t, err)
 
@@ -358,7 +367,9 @@ func TestMultipleCRDSameVersion(t *testing.T) {
 
 	env.FakeResourceManager.Expect().
 		AddGroupVersion(coolFooCRD.Spec.Group, mergedDiscovery)
-	env.FakeResourceManager.Expect().SetGroupPriority(coolFooCRD.Spec.Group, 1000)
+	for _, versionEntry := range coolFooCRD.Spec.Versions {
+		env.FakeResourceManager.Expect().SetGroupVersionPriority(metav1.GroupVersion{Group: coolFooCRD.Spec.Group, Version: versionEntry.Name}, 1000, 100)
+	}
 	err = env.FakeResourceManager.WaitForActions(ctx, 1*time.Second)
 	require.NoError(t, err)
 }
@@ -388,7 +399,9 @@ func TestDiscoveryControllerResourceManagerRemovedCRD(t *testing.T) {
 	// Resource Manager
 	env.FakeResourceManager.Expect().
 		AddGroupVersion(coolFooCRD.Spec.Group, coolFooDiscovery)
-	env.FakeResourceManager.Expect().SetGroupPriority(coolFooCRD.Spec.Group, 1000)
+	for _, versionEntry := range coolFooCRD.Spec.Versions {
+		env.FakeResourceManager.Expect().SetGroupVersionPriority(metav1.GroupVersion{Group: coolFooCRD.Spec.Group, Version: versionEntry.Name}, 1000, 100)
+	}
 	err = env.FakeResourceManager.WaitForActions(ctx, 1*time.Second)
 	require.NoError(t, err)
 

staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/fake.go

@@ -110,14 +110,15 @@ func (f *fakeResourceManager) WaitForActions(ctx context.Context, timeout time.D
 	return err
 }
 
-func (f *recorderResourceManager) SetGroupPriority(groupName string, priority int) {
+func (f *recorderResourceManager) SetGroupVersionPriority(gv metav1.GroupVersion, grouppriority, versionpriority int) {
 	f.lock.Lock()
 	defer f.lock.Unlock()
 
 	f.Actions = append(f.Actions, recorderResourceManagerAction{
-		Type:  "SetGroupPriority",
-		Group: groupName,
-		Value: priority,
+		Type:    "SetGroupVersionPriority",
+		Group:   gv.Group,
+		Version: gv.Version,
+		Value:   versionpriority,
 	})
 }
 

staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler.go

@@ -24,6 +24,7 @@ import (
 
 	apidiscoveryv2beta1 "k8s.io/api/apidiscovery/v2beta1"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
+	"k8s.io/apimachinery/pkg/version"
 	"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
 
 	"sync/atomic"
@@ -42,10 +43,12 @@ type ResourceManager interface {
 	// Thread-safe
 	AddGroupVersion(groupName string, value apidiscoveryv2beta1.APIVersionDiscovery)
 
-	// Sets priority for a group for sorting discovery.
-	// If a priority is set before the group is known, the priority will be ignored
-	// Once a group is removed, the priority is forgotten.
-	SetGroupPriority(groupName string, priority int)
+	// Sets a priority to be used while sorting a specific group and
+	// group-version. If two versions report different priorities for
+	// the group, the higher one will be used. If the group is not
+	// known, the priority is ignored. The priority for this version
+	// is forgotten once the group-version is forgotten
+	SetGroupVersionPriority(gv metav1.GroupVersion, grouppriority, versionpriority int)
 
 	// Removes all group versions for a given group
 	// Thread-safe
@@ -71,28 +74,32 @@ type resourceDiscoveryManager struct {
 
 	// Writes protected by the lock.
 	// List of all apigroups & resources indexed by the resource manager
-	lock          sync.RWMutex
-	apiGroups     map[string]*apidiscoveryv2beta1.APIGroupDiscovery
-	apiGroupNames map[string]int
+	lock              sync.RWMutex
+	apiGroups         map[string]*apidiscoveryv2beta1.APIGroupDiscovery
+	versionPriorities map[metav1.GroupVersion]priorityInfo
+}
+
+type priorityInfo struct {
+	GroupPriorityMinimum int
+	VersionPriority      int
 }
 
 func NewResourceManager() ResourceManager {
 	scheme := runtime.NewScheme()
 	codecs := serializer.NewCodecFactory(scheme)
 	utilruntime.Must(apidiscoveryv2beta1.AddToScheme(scheme))
-	return &resourceDiscoveryManager{serializer: codecs, apiGroupNames: make(map[string]int)}
+	return &resourceDiscoveryManager{serializer: codecs, versionPriorities: make(map[metav1.GroupVersion]priorityInfo)}
 }
 
-func (rdm *resourceDiscoveryManager) SetGroupPriority(group string, priority int) {
+func (rdm *resourceDiscoveryManager) SetGroupVersionPriority(gv metav1.GroupVersion, groupPriorityMinimum, versionPriority int) {
 	rdm.lock.Lock()
 	defer rdm.lock.Unlock()
 
-	if _, exists := rdm.apiGroupNames[group]; exists {
-		rdm.apiGroupNames[group] = priority
-		rdm.cache.Store(nil)
-	} else {
-		klog.Warningf("DiscoveryManager: Attempted to set priority for group %s but does not exist", group)
+	rdm.versionPriorities[gv] = priorityInfo{
+		GroupPriorityMinimum: groupPriorityMinimum,
+		VersionPriority:      versionPriority,
 	}
+	rdm.cache.Store(nil)
 }
 
 func (rdm *resourceDiscoveryManager) SetGroups(groups []apidiscoveryv2beta1.APIGroupDiscovery) {
@@ -108,10 +115,25 @@ func (rdm *resourceDiscoveryManager) SetGroups(groups []apidiscoveryv2beta1.APIG
 		}
 	}
 
-	// Filter unused out apiGroupNames
-	for name := range rdm.apiGroupNames {
-		if _, exists := rdm.apiGroups[name]; !exists {
-			delete(rdm.apiGroupNames, name)
+	// Filter unused out priority entries
+	for gv := range rdm.versionPriorities {
+		entry, exists := rdm.apiGroups[gv.Group]
+		if !exists {
+			delete(rdm.versionPriorities, gv)
+			continue
+		}
+
+		containsVersion := false
+
+		for _, v := range entry.Versions {
+			if v.Version == gv.Version {
+				containsVersion = true
+				break
+			}
+		}
+
+		if !containsVersion {
+			delete(rdm.versionPriorities, gv)
 		}
 	}
 }
@@ -161,7 +183,14 @@ func (rdm *resourceDiscoveryManager) addGroupVersionLocked(groupName string, val
 			Versions: []apidiscoveryv2beta1.APIVersionDiscovery{value},
 		}
 		rdm.apiGroups[groupName] = group
-		rdm.apiGroupNames[groupName] = 0
+	}
+
+	gv := metav1.GroupVersion{Group: groupName, Version: value.Version}
+	if _, ok := rdm.versionPriorities[gv]; !ok {
+		rdm.versionPriorities[gv] = priorityInfo{
+			GroupPriorityMinimum: 1000,
+			VersionPriority:      15,
+		}
 	}
 
 	// Reset response document so it is recreated lazily
@@ -189,9 +218,9 @@ func (rdm *resourceDiscoveryManager) RemoveGroupVersion(apiGroup metav1.GroupVer
 		return
 	}
 
+	delete(rdm.versionPriorities, apiGroup)
 	if len(group.Versions) == 0 {
 		delete(rdm.apiGroups, group.Name)
-		delete(rdm.apiGroupNames, group.Name)
 	}
 
 	// Reset response document so it is recreated lazily
@@ -203,7 +232,12 @@ func (rdm *resourceDiscoveryManager) RemoveGroup(groupName string) {
 	defer rdm.lock.Unlock()
 
 	delete(rdm.apiGroups, groupName)
-	delete(rdm.apiGroupNames, groupName)
+
+	for k := range rdm.versionPriorities {
+		if k.Group == groupName {
+			delete(rdm.versionPriorities, k)
+		}
+	}
 
 	// Reset response document so it is recreated lazily
 	rdm.cache.Store(nil)
@@ -215,8 +249,40 @@ func (rdm *resourceDiscoveryManager) calculateAPIGroupsLocked() []apidiscoveryv2
 	// Re-order the apiGroups by their priority.
 	groups := []apidiscoveryv2beta1.APIGroupDiscovery{}
 	for _, group := range rdm.apiGroups {
-		groups = append(groups, *group.DeepCopy())
+		copied := *group.DeepCopy()
 
+		// Re-order versions based on their priority. Use kube-aware string
+		// comparison as a tie breaker
+		sort.SliceStable(copied.Versions, func(i, j int) bool {
+			iVersion := copied.Versions[i].Version
+			jVersion := copied.Versions[j].Version
+
+			iPriority := rdm.versionPriorities[metav1.GroupVersion{Group: group.Name, Version: iVersion}].VersionPriority
+			jPriority := rdm.versionPriorities[metav1.GroupVersion{Group: group.Name, Version: jVersion}].VersionPriority
+
+			// Sort by version string comparator if priority is equal
+			if iPriority == jPriority {
+				return version.CompareKubeAwareVersionStrings(iVersion, jVersion) > 0
+			}
+
+			// i sorts before j if it has a higher priority
+			return iPriority > jPriority
+		})
+
+		groups = append(groups, *copied.DeepCopy())
+
+	}
+
+	// For each group, determine the highest minimum group priority and use that
+	priorities := map[string]int{}
+	for gv, info := range rdm.versionPriorities {
+		if existing, exists := priorities[gv.Group]; exists {
+			if existing < info.GroupPriorityMinimum {
+				priorities[gv.Group] = info.GroupPriorityMinimum
+			}
+		} else {
+			priorities[gv.Group] = info.GroupPriorityMinimum
+		}
 	}
 
 	sort.SliceStable(groups, func(i, j int) bool {
@@ -224,8 +290,8 @@ func (rdm *resourceDiscoveryManager) calculateAPIGroupsLocked() []apidiscoveryv2
 		jName := groups[j].Name
 
 		// Default to 0 priority by default
-		iPriority := rdm.apiGroupNames[iName]
-		jPriority := rdm.apiGroupNames[jName]
+		iPriority := priorities[iName]
+		jPriority := priorities[jName]
 
 		// Sort discovery based on apiservice priority.
 		// Duplicated from staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/helpers.go
@@ -234,7 +300,7 @@ func (rdm *resourceDiscoveryManager) calculateAPIGroupsLocked() []apidiscoveryv2
 			return iName < jName
 		}
 
-		// i sorts before j if it has a lower priority
+		// i sorts before j if it has a higher priority
 		return iPriority > jPriority
 	})
 

staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler_test.go

@@ -35,6 +35,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
+	"k8s.io/apimachinery/pkg/version"
 	discoveryendpoint "k8s.io/apiserver/pkg/endpoints/discovery/aggregated"
 )
 
@@ -61,6 +62,9 @@ func fuzzAPIGroups(atLeastNumGroups, maxNumGroups int, seed int64) apidiscoveryv
 		atLeastOne := apidiscoveryv2beta1.APIVersionDiscovery{}
 		c.Fuzz(&atLeastOne)
 		o.Versions = append(o.Versions, atLeastOne)
+		sort.Slice(o.Versions[:], func(i, j int) bool {
+			return version.CompareKubeAwareVersionStrings(o.Versions[i].Version, o.Versions[j].Version) > 0
+		})
 
 		o.TypeMeta = metav1.TypeMeta{}
 		var name string
@@ -499,3 +503,123 @@ func TestAbuse(t *testing.T) {
 
 	waitGroup.Wait()
 }
+
+func TestVersionSortingNoPriority(t *testing.T) {
+	manager := discoveryendpoint.NewResourceManager()
+
+	manager.AddGroupVersion("default", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1alpha1",
+	})
+	manager.AddGroupVersion("default", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v2beta1",
+	})
+	manager.AddGroupVersion("default", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1",
+	})
+	manager.AddGroupVersion("default", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1beta1",
+	})
+	manager.AddGroupVersion("default", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v2",
+	})
+
+	response, _, decoded := fetchPath(manager, "application/json", discoveryPath, "")
+	assert.Equal(t, http.StatusOK, response.StatusCode, "response should be 200 OK")
+
+	versions := decoded.Items[0].Versions
+
+	// Ensure that v1 is sorted before v1alpha1
+	assert.Equal(t, versions[0].Version, "v2")
+	assert.Equal(t, versions[1].Version, "v1")
+	assert.Equal(t, versions[2].Version, "v2beta1")
+	assert.Equal(t, versions[3].Version, "v1beta1")
+	assert.Equal(t, versions[4].Version, "v1alpha1")
+}
+
+func TestVersionSortingWithPriority(t *testing.T) {
+	manager := discoveryendpoint.NewResourceManager()
+
+	manager.AddGroupVersion("default", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1",
+	})
+	manager.SetGroupVersionPriority(metav1.GroupVersion{Group: "default", Version: "v1"}, 1000, 100)
+	manager.AddGroupVersion("default", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1alpha1",
+	})
+	manager.SetGroupVersionPriority(metav1.GroupVersion{Group: "default", Version: "v1alpha1"}, 1000, 200)
+
+	response, _, decoded := fetchPath(manager, "application/json", discoveryPath, "")
+	assert.Equal(t, http.StatusOK, response.StatusCode, "response should be 200 OK")
+
+	versions := decoded.Items[0].Versions
+
+	// Ensure that reverse alpha sort order can be overridden by setting group version priorities.
+	assert.Equal(t, versions[0].Version, "v1alpha1")
+	assert.Equal(t, versions[1].Version, "v1")
+}
+
+// if two apiservices declare conflicting priorities for their group priority, take the higher one.
+func TestGroupVersionSortingConflictingPriority(t *testing.T) {
+	manager := discoveryendpoint.NewResourceManager()
+
+	manager.AddGroupVersion("default", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1",
+	})
+	manager.SetGroupVersionPriority(metav1.GroupVersion{Group: "default", Version: "v1"}, 1000, 100)
+	manager.AddGroupVersion("test", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1alpha1",
+	})
+	manager.SetGroupVersionPriority(metav1.GroupVersion{Group: "test", Version: "v1alpha1"}, 500, 100)
+	manager.AddGroupVersion("test", apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1alpha2",
+	})
+	manager.SetGroupVersionPriority(metav1.GroupVersion{Group: "test", Version: "v1alpha1"}, 2000, 100)
+
+	response, _, decoded := fetchPath(manager, "application/json", discoveryPath, "")
+	assert.Equal(t, http.StatusOK, response.StatusCode, "response should be 200 OK")
+
+	groups := decoded.Items
+
+	// Ensure that reverse alpha sort order can be overridden by setting group version priorities.
+	assert.Equal(t, groups[0].Name, "test")
+	assert.Equal(t, groups[1].Name, "default")
+}
+
+// Show that the GroupPriorityMinimum is not sticky if a higher group version is removed
+// after a lower one is added
+func TestStatelessGroupPriorityMinimum(t *testing.T) {
+	manager := discoveryendpoint.NewResourceManager()
+
+	stableGroup := "stable.example.com"
+	experimentalGroup := "experimental.example.com"
+
+	manager.AddGroupVersion(stableGroup, apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1",
+	})
+	manager.SetGroupVersionPriority(metav1.GroupVersion{Group: stableGroup, Version: "v1"}, 1000, 100)
+
+	manager.AddGroupVersion(experimentalGroup, apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1",
+	})
+	manager.SetGroupVersionPriority(metav1.GroupVersion{Group: experimentalGroup, Version: "v1"}, 100, 100)
+
+	manager.AddGroupVersion(experimentalGroup, apidiscoveryv2beta1.APIVersionDiscovery{
+		Version: "v1alpha1",
+	})
+	manager.SetGroupVersionPriority(metav1.GroupVersion{Group: experimentalGroup, Version: "v1alpha1"}, 10000, 100)
+
+	// Expect v1alpha1's group priority to be used and sort it first in the list
+	response, _, decoded := fetchPath(manager, "application/json", discoveryPath, "")
+	assert.Equal(t, http.StatusOK, response.StatusCode, "response should be 200 OK")
+	assert.Equal(t, decoded.Items[0].Name, "experimental.example.com")
+	assert.Equal(t, decoded.Items[1].Name, "stable.example.com")
+
+	// Remove v1alpha1 and expect the new lower priority to take hold
+	manager.RemoveGroupVersion(metav1.GroupVersion{Group: experimentalGroup, Version: "v1alpha1"})
+
+	response, _, decoded = fetchPath(manager, "application/json", discoveryPath, "")
+	assert.Equal(t, http.StatusOK, response.StatusCode, "response should be 200 OK")
+
+	assert.Equal(t, decoded.Items[0].Name, "stable.example.com")
+	assert.Equal(t, decoded.Items[1].Name, "experimental.example.com")
+}

staging/src/k8s.io/apiserver/pkg/endpoints/installer.go

@@ -71,7 +71,7 @@ type action struct {
 
 func ConvertGroupVersionIntoToDiscovery(list []metav1.APIResource) ([]apidiscoveryv2beta1.APIResourceDiscovery, error) {
 	var apiResourceList []apidiscoveryv2beta1.APIResourceDiscovery
-	parentResources := map[string]*apidiscoveryv2beta1.APIResourceDiscovery{}
+	parentResources := make(map[string]int)
 
 	// Loop through all top-level resources
 	for _, r := range list {
@@ -87,7 +87,7 @@ func ConvertGroupVersionIntoToDiscovery(list []metav1.APIResource) ([]apidiscove
 			scope = apidiscoveryv2beta1.ScopeCluster
 		}
 
-		apiResourceList = append(apiResourceList, apidiscoveryv2beta1.APIResourceDiscovery{
+		resource := apidiscoveryv2beta1.APIResourceDiscovery{
 			Resource: r.Name,
 			Scope:    scope,
 			ResponseKind: &metav1.GroupVersionKind{
@@ -99,8 +99,9 @@ func ConvertGroupVersionIntoToDiscovery(list []metav1.APIResource) ([]apidiscove
 			ShortNames:       r.ShortNames,
 			Categories:       r.Categories,
 			SingularResource: r.SingularName,
-		})
-		parentResources[r.Name] = &apiResourceList[len(apiResourceList)-1]
+		}
+		apiResourceList = append(apiResourceList, resource)
+		parentResources[r.Name] = len(apiResourceList) - 1
 	}
 
 	// Loop through all subresources
@@ -120,23 +121,19 @@ func ConvertGroupVersionIntoToDiscovery(list []metav1.APIResource) ([]apidiscove
 			scope = apidiscoveryv2beta1.ScopeCluster
 		}
 
-		var parent *apidiscoveryv2beta1.APIResourceDiscovery
-		var exists bool
-
-		parent, exists = parentResources[split[0]]
+		parentidx, exists := parentResources[split[0]]
 		if !exists {
 			// If a subresource exists without a parent, create a parent
 			apiResourceList = append(apiResourceList, apidiscoveryv2beta1.APIResourceDiscovery{
 				Resource: split[0],
 				Scope:    scope,
 			})
-			parentResources[split[0]] = &apiResourceList[len(apiResourceList)-1]
-			parent = &apiResourceList[len(apiResourceList)-1]
-			parentResources[split[0]] = parent
+			parentidx = len(apiResourceList) - 1
+			parentResources[split[0]] = parentidx
 		}
 
-		if parent.Scope != scope {
-			return nil, fmt.Errorf("Error: Parent %s (scope: %s) and subresource %s (scope: %s) scope do not match", split[0], parent.Scope, split[1], scope)
+		if apiResourceList[parentidx].Scope != scope {
+			return nil, fmt.Errorf("Error: Parent %s (scope: %s) and subresource %s (scope: %s) scope do not match", split[0], apiResourceList[parentidx].Scope, split[1], scope)
 			//
 		}
 
@@ -151,9 +148,9 @@ func ConvertGroupVersionIntoToDiscovery(list []metav1.APIResource) ([]apidiscove
 				Kind:    r.Kind,
 			}
 		}
-		parent.Subresources = append(parent.Subresources, subresource)
-
+		apiResourceList[parentidx].Subresources = append(apiResourceList[parentidx].Subresources, subresource)
 	}
+
 	return apiResourceList, nil
 }
 

staging/src/k8s.io/apiserver/pkg/endpoints/installer_test.go

@@ -206,6 +206,87 @@ func TestConvertAPIResourceToDiscovery(t *testing.T) {
 			},
 		},
 		{
+			name: "Test multiple resources and subresources",
+			resources: []metav1.APIResource{
+				{
+					Name:       "cronjobs",
+					Namespaced: true,
+					Kind:       "CronJob",
+					Group:      "batch",
+					Version:    "v1",
+					ShortNames: []string{"cj"},
+					Verbs:      []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+				},
+				{
+					Name:       "cronjobs/status",
+					Namespaced: true,
+					Kind:       "CronJob",
+					Group:      "batch",
+					Version:    "v1",
+					ShortNames: []string{"cj"},
+					Verbs:      []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+				},
+				{
+					Name:       "deployments",
+					Namespaced: true,
+					Kind:       "Deployment",
+					Group:      "apps",
+					Version:    "v1",
+					ShortNames: []string{"deploy"},
+					Verbs:      []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+				},
+				{
+					Name:       "deployments/status",
+					Namespaced: true,
+					Kind:       "Deployment",
+					Group:      "apps",
+					Version:    "v1",
+					ShortNames: []string{"deploy"},
+					Verbs:      []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+				},
+			},
+			wantAPIResourceDiscovery: []apidiscoveryv2beta1.APIResourceDiscovery{
+				{
+					Resource: "cronjobs",
+					Scope:    apidiscoveryv2beta1.ScopeNamespace,
+					ResponseKind: &metav1.GroupVersionKind{
+						Group:   "batch",
+						Version: "v1",
+						Kind:    "CronJob",
+					},
+					ShortNames: []string{"cj"},
+					Verbs:      []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+					Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{{
+						Subresource: "status",
+						ResponseKind: &metav1.GroupVersionKind{
+							Group:   "batch",
+							Version: "v1",
+							Kind:    "CronJob",
+						},
+						Verbs: []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+					}},
+				}, {
+					Resource: "deployments",
+					Scope:    apidiscoveryv2beta1.ScopeNamespace,
+					ResponseKind: &metav1.GroupVersionKind{
+						Group:   "apps",
+						Version: "v1",
+						Kind:    "Deployment",
+					},
+					ShortNames: []string{"deploy"},
+					Verbs:      []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+					Subresources: []apidiscoveryv2beta1.APISubresourceDiscovery{{
+						Subresource: "status",
+						ResponseKind: &metav1.GroupVersionKind{
+							Group:   "apps",
+							Version: "v1",
+							Kind:    "Deployment",
+						},
+						Verbs: []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"},
+					}},
+				},
+			},
+		}, {
 			name: "Test with subresource with no parent",
 			resources: []metav1.APIResource{
 				{

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

@@ -35,6 +35,7 @@ import (
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/google/uuid"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -931,7 +932,12 @@ func installAPI(s *GenericAPIServer, c *Config) {
 	routes.Version{Version: c.Version}.Install(s.Handler.GoRestfulContainer)
 
 	if c.EnableDiscovery {
-		s.Handler.GoRestfulContainer.Add(s.DiscoveryGroupManager.WebService())
+		if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.AggregatedDiscoveryEndpoint) {
+			wrapped := discoveryendpoint.WrapAggregatedDiscoveryToHandler(s.DiscoveryGroupManager, s.AggregatedDiscoveryGroupManager)
+			s.Handler.GoRestfulContainer.Add(wrapped.GenerateWebService("/apis", metav1.APIGroupList{}))
+		} else {
+			s.Handler.GoRestfulContainer.Add(s.DiscoveryGroupManager.WebService())
+		}
 	}
 	if c.FlowControl != nil && utilfeature.DefaultFeatureGate.Enabled(genericfeatures.APIPriorityAndFairness) {
 		c.FlowControl.Install(s.Handler.NonGoRestfulMux)

staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery.go

@@ -38,8 +38,11 @@ import (
 	"k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/helper"
 )
 
-var APIRegistrationGroup string = "apiregistration.k8s.io"
-var APIRegistrationGroupPriority int = 18000
+var APIRegistrationGroupVersion metav1.GroupVersion = metav1.GroupVersion{Group: "apiregistration.k8s.io", Version: "v1"}
+
+// Maximum is 20000. Set to higher than that so apiregistration always is listed
+// first (mirrors v1 discovery behavior)
+var APIRegistrationGroupPriority int = 20001
 
 // Given a list of APIServices and proxyHandlers for contacting them,
 // DiscoveryManager caches a list of discovery documents for each server
@@ -151,9 +154,12 @@ type groupVersionInfo struct {
 	// describes how to contact the server responsible for this GroupVersion.
 	service serviceKey
 
-	// groupPriority describes the priority of the APIService for sorting
+	// groupPriority describes the priority of the APIService's group for sorting
 	groupPriority int
 
+	// groupPriority describes the priority of the APIService version for sorting
+	versionPriority int
+
 	// Method for contacting the service
 	handler http.Handler
 }
@@ -232,7 +238,7 @@ func (dm *discoveryManager) fetchFreshDiscoveryForService(gv metav1.GroupVersion
 
 		dm.setCacheEntryForService(info.service, cached)
 		return &cached, nil
-	case http.StatusNotFound:
+	case http.StatusNotAcceptable:
 		// Discovery Document is not being served at all.
 		// Fall back to legacy discovery information
 		if len(gv.Version) == 0 {
@@ -390,6 +396,7 @@ func (dm *discoveryManager) syncAPIService(apiServiceName string) error {
 	}
 
 	dm.mergedDiscoveryHandler.AddGroupVersion(gv.Group, entry)
+	dm.mergedDiscoveryHandler.SetGroupVersionPriority(metav1.GroupVersion(gv), info.groupPriority, info.versionPriority)
 	return nil
 }
 
@@ -428,7 +435,7 @@ func (dm *discoveryManager) Run(stopCh <-chan struct{}) {
 	}
 
 	// Ensure that apiregistration.k8s.io is the first group in the discovery group.
-	dm.mergedDiscoveryHandler.SetGroupPriority(APIRegistrationGroup, APIRegistrationGroupPriority)
+	dm.mergedDiscoveryHandler.SetGroupVersionPriority(APIRegistrationGroupVersion, APIRegistrationGroupPriority, 0)
 
 	wait.PollUntil(1*time.Minute, func() (done bool, err error) {
 		dm.servicesLock.Lock()
@@ -458,6 +465,7 @@ func (dm *discoveryManager) AddAPIService(apiService *apiregistrationv1.APIServi
 	// Add or update APIService record and mark it as dirty
 	dm.setInfoForAPIService(apiService.Name, &groupVersionInfo{
 		groupPriority:   int(apiService.Spec.GroupPriorityMinimum),
+		versionPriority: int(apiService.Spec.VersionPriority),
 		handler:         handler,
 		lastMarkedDirty: time.Now(),
 		service:         newServiceKey(*apiService.Spec.Service),

staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_discovery_test.go

@@ -205,6 +205,7 @@ func TestRemoveAPIService(t *testing.T) {
 
 func TestLegacyFallback(t *testing.T) {
 	aggregatedResourceManager := discoveryendpoint.NewResourceManager()
+	rootAPIsHandler := discovery.NewRootAPIsHandler(discovery.DefaultAddresses{DefaultAddress: "192.168.1.1"}, scheme.Codecs)
 
 	legacyGroupHandler := discovery.NewAPIGroupHandler(scheme.Codecs, metav1.APIGroup{
 		Name: "stable.example.com",
@@ -262,9 +263,11 @@ func TestLegacyFallback(t *testing.T) {
 		} else if r.URL.Path == "/apis/stable.example.com/v1" {
 			// defer to legacy discovery
 			legacyResourceHandler.ServeHTTP(w, r)
+		} else if r.URL.Path == "/apis" {
+			rootAPIsHandler.ServeHTTP(w, r)
 		} else {
 			// Unknown url
-			w.WriteHeader(http.StatusNotFound)
+			t.Fatalf("unexpected request sent to %v", r.URL.Path)
 		}
 	}))
 	testCtx, cancel := context.WithCancel(context.Background())