From fed7bb7c51e9f85db122ad6d111cb8326dce9f8a Mon Sep 17 00:00:00 2001 From: Sean Sullivan Date: Tue, 28 Feb 2023 19:44:34 +0000 Subject: [PATCH] Plumb stale GroupVersions through aggregated discovery --- .../discovery/aggregated_discovery.go | 44 +- .../discovery/aggregated_discovery_test.go | 182 ++++++- .../discovery/cached/memory/memcache.go | 25 +- .../discovery/cached/memory/memcache_test.go | 112 ++++- .../client-go/discovery/discovery_client.go | 95 ++-- .../discovery/discovery_client_test.go | 447 +++++++++++++++++- 6 files changed, 835 insertions(+), 70 deletions(-) diff --git a/staging/src/k8s.io/client-go/discovery/aggregated_discovery.go b/staging/src/k8s.io/client-go/discovery/aggregated_discovery.go index 033a4c8fc3c1..758b0a3ac8fe 100644 --- a/staging/src/k8s.io/client-go/discovery/aggregated_discovery.go +++ b/staging/src/k8s.io/client-go/discovery/aggregated_discovery.go @@ -24,19 +24,36 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) +// StaleGroupVersionError encasulates failed GroupVersion marked "stale" +// in the returned AggregatedDiscovery format. +type StaleGroupVersionError struct { + gv schema.GroupVersion +} + +func (s StaleGroupVersionError) Error() string { + return fmt.Sprintf("stale GroupVersion discovery: %v", s.gv) +} + // SplitGroupsAndResources transforms "aggregated" discovery top-level structure into // the previous "unaggregated" discovery groups and resources. -func SplitGroupsAndResources(aggregatedGroups apidiscovery.APIGroupDiscoveryList) (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList) { +func SplitGroupsAndResources(aggregatedGroups apidiscovery.APIGroupDiscoveryList) ( + *metav1.APIGroupList, + map[schema.GroupVersion]*metav1.APIResourceList, + map[schema.GroupVersion]error) { // Aggregated group list will contain the entirety of discovery, including - // groups, versions, and resources. + // groups, versions, and resources. GroupVersions marked "stale" are failed. groups := []*metav1.APIGroup{} + failedGVs := map[schema.GroupVersion]error{} resourcesByGV := map[schema.GroupVersion]*metav1.APIResourceList{} for _, aggGroup := range aggregatedGroups.Items { - group, resources := convertAPIGroup(aggGroup) + group, resources, failed := convertAPIGroup(aggGroup) groups = append(groups, group) for gv, resourceList := range resources { resourcesByGV[gv] = resourceList } + for gv, err := range failed { + failedGVs[gv] = err + } } // Transform slice of groups to group list before returning. groupList := &metav1.APIGroupList{} @@ -44,23 +61,32 @@ func SplitGroupsAndResources(aggregatedGroups apidiscovery.APIGroupDiscoveryList for _, group := range groups { groupList.Groups = append(groupList.Groups, *group) } - return groupList, resourcesByGV + return groupList, resourcesByGV, failedGVs } // convertAPIGroup tranforms an "aggregated" APIGroupDiscovery to an "legacy" APIGroup, // also returning the map of APIResourceList for resources within GroupVersions. -func convertAPIGroup(g apidiscovery.APIGroupDiscovery) (*metav1.APIGroup, map[schema.GroupVersion]*metav1.APIResourceList) { +func convertAPIGroup(g apidiscovery.APIGroupDiscovery) ( + *metav1.APIGroup, + map[schema.GroupVersion]*metav1.APIResourceList, + map[schema.GroupVersion]error) { // Iterate through versions to convert to group and resources. group := &metav1.APIGroup{} gvResources := map[schema.GroupVersion]*metav1.APIResourceList{} + failedGVs := map[schema.GroupVersion]error{} group.Name = g.ObjectMeta.Name - for i, v := range g.Versions { - version := metav1.GroupVersionForDiscovery{} + for _, v := range g.Versions { gv := schema.GroupVersion{Group: g.Name, Version: v.Version} + if v.Freshness == apidiscovery.DiscoveryFreshnessStale { + failedGVs[gv] = StaleGroupVersionError{gv: gv} + continue + } + version := metav1.GroupVersionForDiscovery{} version.GroupVersion = gv.String() version.Version = v.Version group.Versions = append(group.Versions, version) - if i == 0 { + // PreferredVersion is first non-stale Version + if group.PreferredVersion == (metav1.GroupVersionForDiscovery{}) { group.PreferredVersion = version } resourceList := &metav1.APIResourceList{} @@ -76,7 +102,7 @@ func convertAPIGroup(g apidiscovery.APIGroupDiscovery) (*metav1.APIGroup, map[sc } gvResources[gv] = resourceList } - return group, gvResources + return group, gvResources, failedGVs } // convertAPIResource tranforms a APIResourceDiscovery to an APIResource. diff --git a/staging/src/k8s.io/client-go/discovery/aggregated_discovery_test.go b/staging/src/k8s.io/client-go/discovery/aggregated_discovery_test.go index b232e1a0b2b5..11815a0671a5 100644 --- a/staging/src/k8s.io/client-go/discovery/aggregated_discovery_test.go +++ b/staging/src/k8s.io/client-go/discovery/aggregated_discovery_test.go @@ -31,6 +31,7 @@ func TestSplitGroupsAndResources(t *testing.T) { agg apidiscovery.APIGroupDiscoveryList expectedGroups metav1.APIGroupList expectedGVResources map[schema.GroupVersion]*metav1.APIResourceList + expectedFailedGVs map[schema.GroupVersion]error }{ { name: "Aggregated discovery: core/v1 group and pod resource", @@ -90,6 +91,7 @@ func TestSplitGroupsAndResources(t *testing.T) { }, }, }, + expectedFailedGVs: map[schema.GroupVersion]error{}, }, { name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/2 versions/1 resources at /apis", @@ -179,6 +181,7 @@ func TestSplitGroupsAndResources(t *testing.T) { }, }, }, + expectedFailedGVs: map[schema.GroupVersion]error{}, }, { name: "Aggregated discovery: 1 group/2 resources at /api, 1 group/2 resources at /apis", @@ -313,6 +316,7 @@ func TestSplitGroupsAndResources(t *testing.T) { }, }, }, + expectedFailedGVs: map[schema.GroupVersion]error{}, }, { name: "Aggregated discovery: multiple groups with cluster-scoped resources", @@ -447,6 +451,7 @@ func TestSplitGroupsAndResources(t *testing.T) { }, }, }, + expectedFailedGVs: map[schema.GroupVersion]error{}, }, { name: "Aggregated discovery with single subresource", @@ -534,6 +539,7 @@ func TestSplitGroupsAndResources(t *testing.T) { }, }, }, + expectedFailedGVs: map[schema.GroupVersion]error{}, }, { name: "Aggregated discovery with multiple subresources", @@ -633,11 +639,185 @@ func TestSplitGroupsAndResources(t *testing.T) { }, }, }, + expectedFailedGVs: map[schema.GroupVersion]error{}, + }, + { + name: "Aggregated discovery: single failed GV at /api", + agg: apidiscovery.APIGroupDiscoveryList{ + Items: []apidiscovery.APIGroupDiscovery{ + { + Versions: []apidiscovery.APIVersionDiscovery{ + { + Version: "v1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "pods", + ResponseKind: &metav1.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Pod", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "services", + ResponseKind: &metav1.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Service", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, + }, + }, + }, + }, + // Single core Group/Version is stale, so no Version within Group. + expectedGroups: metav1.APIGroupList{ + Groups: []metav1.APIGroup{{Name: ""}}, + }, + // Single core Group/Version is stale, so there are no expected resources. + expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{}, + expectedFailedGVs: map[schema.GroupVersion]error{ + {Group: "", Version: "v1"}: StaleGroupVersionError{gv: schema.GroupVersion{Group: "", Version: "v1"}}, + }, + }, + { + name: "Aggregated discovery: single failed GV at /apis", + agg: apidiscovery.APIGroupDiscoveryList{ + Items: []apidiscovery.APIGroupDiscovery{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "apps", + }, + Versions: []apidiscovery.APIVersionDiscovery{ + { + Version: "v1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "statefulsets", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1", + Kind: "StatefulSets", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, + }, + }, + }, + }, + // Single apps/v1 Group/Version is stale, so no Version within Group. + expectedGroups: metav1.APIGroupList{ + Groups: []metav1.APIGroup{{Name: "apps"}}, + }, + // Single apps/v1 Group/Version is stale, so there are no expected resources. + expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{}, + expectedFailedGVs: map[schema.GroupVersion]error{ + {Group: "apps", Version: "v1"}: StaleGroupVersionError{gv: schema.GroupVersion{Group: "apps", Version: "v1"}}, + }, + }, + { + name: "Aggregated discovery: 1 group/2 versions/1 failed GV at /apis", + agg: apidiscovery.APIGroupDiscoveryList{ + Items: []apidiscovery.APIGroupDiscovery{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "apps", + }, + Versions: []apidiscovery.APIVersionDiscovery{ + // Stale v2 should report failed GV. + { + Version: "v2", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "daemonsets", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v2", + Kind: "DaemonSets", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, + { + Version: "v1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + }, + }, + }, + }, + }, + // Only apps/v1 is non-stale expected Group/Version + expectedGroups: metav1.APIGroupList{ + Groups: []metav1.APIGroup{ + { + Name: "apps", + Versions: []metav1.GroupVersionForDiscovery{ + { + GroupVersion: "apps/v1", + Version: "v1", + }, + }, + // PreferredVersion must be apps/v1 + PreferredVersion: metav1.GroupVersionForDiscovery{ + GroupVersion: "apps/v1", + Version: "v1", + }, + }, + }, + }, + // Only apps/v1 resources expected. + expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{ + {Group: "apps", Version: "v1"}: { + GroupVersion: "apps/v1", + APIResources: []metav1.APIResource{ + { + Name: "deployments", + Namespaced: true, + Group: "apps", + Version: "v1", + Kind: "Deployment", + }, + }, + }, + }, + expectedFailedGVs: map[schema.GroupVersion]error{ + {Group: "apps", Version: "v2"}: StaleGroupVersionError{gv: schema.GroupVersion{Group: "apps", Version: "v2"}}, + }, }, } for _, test := range tests { - apiGroups, resourcesByGV := SplitGroupsAndResources(test.agg) + apiGroups, resourcesByGV, failedGVs := SplitGroupsAndResources(test.agg) + assert.Equal(t, test.expectedFailedGVs, failedGVs) assert.Equal(t, test.expectedGroups, *apiGroups) assert.Equal(t, test.expectedGVResources, resourcesByGV) } diff --git a/staging/src/k8s.io/client-go/discovery/cached/memory/memcache.go b/staging/src/k8s.io/client-go/discovery/cached/memory/memcache.go index b0fc7c211486..9143ce00ab42 100644 --- a/staging/src/k8s.io/client-go/discovery/cached/memory/memcache.go +++ b/staging/src/k8s.io/client-go/discovery/cached/memory/memcache.go @@ -136,32 +136,38 @@ func (d *memCacheClient) ServerGroupsAndResources() ([]*metav1.APIGroup, []*meta // GroupsAndMaybeResources returns the list of APIGroups, and possibly the map of group/version // to resources. The returned groups will never be nil, but the resources map can be nil // if there are no cached resources. -func (d *memCacheClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error) { +func (d *memCacheClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) { d.lock.Lock() defer d.lock.Unlock() if !d.cacheValid { if err := d.refreshLocked(); err != nil { - return nil, nil, err + return nil, nil, nil, err } } // Build the resourceList from the cache? var resourcesMap map[schema.GroupVersion]*metav1.APIResourceList + var failedGVs map[schema.GroupVersion]error if d.receivedAggregatedDiscovery && len(d.groupToServerResources) > 0 { resourcesMap = map[schema.GroupVersion]*metav1.APIResourceList{} + failedGVs = map[schema.GroupVersion]error{} for gv, cacheEntry := range d.groupToServerResources { groupVersion, err := schema.ParseGroupVersion(gv) if err != nil { - return nil, nil, fmt.Errorf("failed to parse group version (%v): %v", gv, err) + return nil, nil, nil, fmt.Errorf("failed to parse group version (%v): %v", gv, err) + } + if cacheEntry.err != nil { + failedGVs[groupVersion] = cacheEntry.err + } else { + resourcesMap[groupVersion] = cacheEntry.resourceList } - resourcesMap[groupVersion] = cacheEntry.resourceList } } - return d.groupList, resourcesMap, nil + return d.groupList, resourcesMap, failedGVs, nil } func (d *memCacheClient) ServerGroups() (*metav1.APIGroupList, error) { - groups, _, err := d.GroupsAndMaybeResources() + groups, _, _, err := d.GroupsAndMaybeResources() if err != nil { return nil, err } @@ -235,7 +241,8 @@ func (d *memCacheClient) refreshLocked() error { if ad, ok := d.delegate.(discovery.AggregatedDiscoveryInterface); ok { var resources map[schema.GroupVersion]*metav1.APIResourceList - gl, resources, err = ad.GroupsAndMaybeResources() + var failedGVs map[schema.GroupVersion]error + gl, resources, failedGVs, err = ad.GroupsAndMaybeResources() if resources != nil && err == nil { // Cache the resources. d.groupToServerResources = map[string]*cacheEntry{} @@ -243,6 +250,10 @@ func (d *memCacheClient) refreshLocked() error { for gv, resources := range resources { d.groupToServerResources[gv.String()] = &cacheEntry{resources, nil} } + // Cache GroupVersion discovery errors + for gv, err := range failedGVs { + d.groupToServerResources[gv.String()] = &cacheEntry{nil, err} + } d.receivedAggregatedDiscovery = true d.cacheValid = true return nil diff --git a/staging/src/k8s.io/client-go/discovery/cached/memory/memcache_test.go b/staging/src/k8s.io/client-go/discovery/cached/memory/memcache_test.go index 6be6f5216450..a86ece331a91 100644 --- a/staging/src/k8s.io/client-go/discovery/cached/memory/memcache_test.go +++ b/staging/src/k8s.io/client-go/discovery/cached/memory/memcache_test.go @@ -32,6 +32,7 @@ import ( errorsutil "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/discovery" "k8s.io/client-go/discovery/fake" @@ -599,10 +600,11 @@ func TestMemCacheGroupsAndMaybeResources(t *testing.T) { groupToServerResources: map[string]*cacheEntry{}, } assert.False(t, memClient.Fresh()) - apiGroupList, resourcesMap, err := memClient.GroupsAndMaybeResources() + apiGroupList, resourcesMap, failedGVs, err := memClient.GroupsAndMaybeResources() require.NoError(t, err) // "Unaggregated" discovery always returns nil for resources. assert.Nil(t, resourcesMap) + assert.True(t, len(failedGVs) == 0, "expected empty failed GroupVersions, got (%d)", len(failedGVs)) assert.False(t, memClient.receivedAggregatedDiscovery) assert.True(t, memClient.Fresh()) // Test the expected groups are returned for the aggregated format. @@ -618,7 +620,7 @@ func TestMemCacheGroupsAndMaybeResources(t *testing.T) { // Invalidate the cache and retrieve the server groups and resources again. memClient.Invalidate() assert.False(t, memClient.Fresh()) - apiGroupList, resourcesMap, err = memClient.GroupsAndMaybeResources() + apiGroupList, resourcesMap, _, err = memClient.GroupsAndMaybeResources() require.NoError(t, err) assert.Nil(t, resourcesMap) assert.False(t, memClient.receivedAggregatedDiscovery) @@ -638,6 +640,7 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { expectedGroupNames []string expectedGroupVersions []string expectedGVKs []string + expectedFailedGVs []string }{ { name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/1 resources at /apis", @@ -694,9 +697,10 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { "/v1/Pod", "apps/v1/Deployment", }, + expectedFailedGVs: []string{}, }, { - name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/2 versions/1 resources at /apis", + name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/2 versions/1 resources/1 stale GV at /apis", corev1: &apidiscovery.APIGroupDiscoveryList{ Items: []apidiscovery.APIGroupDiscovery{ { @@ -741,6 +745,7 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { }, }, { + // Stale Version is not included in discovery. Version: "v2", Resources: []apidiscovery.APIResourceDiscovery{ { @@ -753,18 +758,19 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { Scope: apidiscovery.ScopeNamespace, }, }, + Freshness: apidiscovery.DiscoveryFreshnessStale, }, }, }, }, }, expectedGroupNames: []string{"", "apps"}, - expectedGroupVersions: []string{"v1", "apps/v1", "apps/v2"}, + expectedGroupVersions: []string{"v1", "apps/v1"}, expectedGVKs: []string{ "/v1/Pod", "apps/v1/Deployment", - "apps/v2/Deployment", }, + expectedFailedGVs: []string{"apps/v2"}, }, { name: "Aggregated discovery: 1 group/2 resources at /api, 1 group/2 resources at /apis", @@ -841,9 +847,10 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { "apps/v1/Deployment", "apps/v1/StatefulSet", }, + expectedFailedGVs: []string{}, }, { - name: "Aggregated discovery: 1 group/2 resources at /api, 2 group/2 resources at /apis", + name: "Aggregated discovery: 1 group/2 resources at /api, 2 group/2 resources/1 stale GV at /apis", corev1: &apidiscovery.APIGroupDiscoveryList{ Items: []apidiscovery.APIGroupDiscovery{ { @@ -905,6 +912,31 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { }, }, }, + { + // Stale version is not included in discovery. + Version: "v1beta1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1beta1", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "statefulsets", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1beta1", + Kind: "StatefulSet", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, }, }, { @@ -949,9 +981,10 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { "batch/v1/Job", "batch/v1/CronJob", }, + expectedFailedGVs: []string{"apps/v1beta1"}, }, { - name: "Aggregated discovery: /api returns nothing, 2 groups/2 resources at /apis", + name: "Aggregated discovery: /api returns nothing, 2 groups/2 resources/2 stale GV at /apis", corev1: &apidiscovery.APIGroupDiscoveryList{}, apis: &apidiscovery.APIGroupDiscoveryList{ Items: []apidiscovery.APIGroupDiscovery{ @@ -960,6 +993,7 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { Name: "apps", }, Versions: []apidiscovery.APIVersionDiscovery{ + // Statel "v1" Version is not included in discovery. { Version: "v1", Resources: []apidiscovery.APIResourceDiscovery{ @@ -982,6 +1016,30 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { Scope: apidiscovery.ScopeNamespace, }, }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, + { + Version: "v1beta1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1beta1", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "statefulsets", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1beta1", + Kind: "StatefulSet", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, }, }, }, @@ -1013,18 +1071,35 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { }, }, }, + { + // Stale Version is not included in discovery. + Version: "v1beta1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "jobs", + ResponseKind: &metav1.GroupVersionKind{ + Group: "batch", + Version: "v1beta1", + Kind: "Job", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, }, }, }, }, expectedGroupNames: []string{"apps", "batch"}, - expectedGroupVersions: []string{"apps/v1", "batch/v1"}, + expectedGroupVersions: []string{"apps/v1beta1", "batch/v1"}, expectedGVKs: []string{ - "apps/v1/Deployment", - "apps/v1/StatefulSet", + "apps/v1beta1/Deployment", + "apps/v1beta1/StatefulSet", "batch/v1/Job", "batch/v1/CronJob", }, + expectedFailedGVs: []string{"apps/v1", "batch/v1beta1"}, }, } @@ -1054,7 +1129,7 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { groupToServerResources: map[string]*cacheEntry{}, } assert.False(t, memClient.Fresh()) - apiGroupList, resourcesMap, err := memClient.GroupsAndMaybeResources() + apiGroupList, resourcesMap, failedGVs, err := memClient.GroupsAndMaybeResources() require.NoError(t, err) assert.True(t, memClient.receivedAggregatedDiscovery) assert.True(t, memClient.Fresh()) @@ -1077,10 +1152,15 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) { actualGVKs := sets.NewString(groupVersionKinds(resources)...) assert.True(t, expectedGVKs.Equal(actualGVKs), "%s: Expected GVKs (%s), got (%s)", test.name, expectedGVKs.List(), actualGVKs.List()) + // Test the returned failed GroupVersions are correct. + expectedFailedGVs := sets.NewString(test.expectedFailedGVs...) + actualFailedGVs := sets.NewString(failedGroupVersions(failedGVs)...) + assert.True(t, expectedFailedGVs.Equal(actualFailedGVs), + "%s: Expected Failed GroupVersions (%s), got (%s)", test.name, expectedFailedGVs.List(), actualFailedGVs.List()) // Invalidate the cache and retrieve the server groups again. memClient.Invalidate() assert.False(t, memClient.Fresh()) - apiGroupList, _, err = memClient.GroupsAndMaybeResources() + apiGroupList, _, _, err = memClient.GroupsAndMaybeResources() require.NoError(t, err) // Test the expected groups are returned for the aggregated format. actualGroupNames = sets.NewString(groupNamesFromList(apiGroupList)...) @@ -1410,3 +1490,11 @@ func groupVersionKinds(resources []*metav1.APIResourceList) []string { } return result } + +func failedGroupVersions(gvs map[schema.GroupVersion]error) []string { + result := []string{} + for gv := range gvs { + result = append(result, gv.String()) + } + return result +} diff --git a/staging/src/k8s.io/client-go/discovery/discovery_client.go b/staging/src/k8s.io/client-go/discovery/discovery_client.go index 52390bf934b0..641568008b7b 100644 --- a/staging/src/k8s.io/client-go/discovery/discovery_client.go +++ b/staging/src/k8s.io/client-go/discovery/discovery_client.go @@ -86,7 +86,7 @@ type DiscoveryInterface interface { type AggregatedDiscoveryInterface interface { DiscoveryInterface - GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error) + GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) } // CachedDiscoveryInterface is a DiscoveryInterface with cache invalidation and freshness. @@ -186,18 +186,23 @@ func apiVersionsToAPIGroup(apiVersions *metav1.APIVersions) (apiGroup metav1.API // and resources from /api and /apis (either aggregated or not). Legacy groups // must be ordered first. The server will either return both endpoints (/api, /apis) // as aggregated discovery format or legacy format. For safety, resources will only -// be returned if both endpoints returned resources. -func (d *DiscoveryClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error) { +// be returned if both endpoints returned resources. Returned "failedGVs" can be +// empty, but will only be nil in the case an error is returned. +func (d *DiscoveryClient) GroupsAndMaybeResources() ( + *metav1.APIGroupList, + map[schema.GroupVersion]*metav1.APIResourceList, + map[schema.GroupVersion]error, + error) { // Legacy group ordered first (there is only one -- core/v1 group). Returned groups must // be non-nil, but it could be empty. Returned resources, apiResources map could be nil. - groups, resources, err := d.downloadLegacy() + groups, resources, failedGVs, err := d.downloadLegacy() if err != nil { - return nil, nil, err + return nil, nil, nil, err } // Discovery groups and (possibly) resources downloaded from /apis. - apiGroups, apiResources, aerr := d.downloadAPIs() + apiGroups, apiResources, failedApisGVs, aerr := d.downloadAPIs() if aerr != nil { - return nil, nil, aerr + return nil, nil, nil, aerr } // Merge apis groups into the legacy groups. for _, group := range apiGroups.Groups { @@ -211,14 +216,23 @@ func (d *DiscoveryClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[s } else if resources != nil { resources = nil } - return groups, resources, err + // Merge failed GroupVersions from /api and /apis + for gv, err := range failedApisGVs { + failedGVs[gv] = err + } + return groups, resources, failedGVs, err } // downloadLegacy returns the discovery groups and possibly resources // for the legacy v1 GVR at /api, or an error if one occurred. It is // possible for the resource map to be nil if the server returned -// the unaggregated discovery. -func (d *DiscoveryClient) downloadLegacy() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error) { +// the unaggregated discovery. Returned "failedGVs" can be empty, but +// will only be nil in the case of a returned error. +func (d *DiscoveryClient) downloadLegacy() ( + *metav1.APIGroupList, + map[schema.GroupVersion]*metav1.APIResourceList, + map[schema.GroupVersion]error, + error) { accept := acceptDiscoveryFormats if d.UseLegacyDiscovery { accept = AcceptV1 @@ -230,16 +244,19 @@ func (d *DiscoveryClient) downloadLegacy() (*metav1.APIGroupList, map[schema.Gro Do(context.TODO()). ContentType(&responseContentType). Raw() + apiGroupList := &metav1.APIGroupList{} + failedGVs := map[schema.GroupVersion]error{} if err != nil { // Tolerate 404, since aggregated api servers can return it. if errors.IsNotFound(err) { - return &metav1.APIGroupList{}, nil, nil + // Return empty structures and no error. + emptyGVMap := map[schema.GroupVersion]*metav1.APIResourceList{} + return apiGroupList, emptyGVMap, failedGVs, nil } else { - return nil, nil, err + return nil, nil, nil, err } } - apiGroupList := &metav1.APIGroupList{} var resourcesByGV map[schema.GroupVersion]*metav1.APIResourceList // Switch on content-type server responded with: aggregated or unaggregated. switch responseContentType { @@ -247,7 +264,7 @@ func (d *DiscoveryClient) downloadLegacy() (*metav1.APIGroupList, map[schema.Gro var v metav1.APIVersions err = json.Unmarshal(body, &v) if err != nil { - return nil, nil, err + return nil, nil, nil, err } apiGroup := metav1.APIGroup{} if len(v.Versions) != 0 { @@ -258,20 +275,25 @@ func (d *DiscoveryClient) downloadLegacy() (*metav1.APIGroupList, map[schema.Gro var aggregatedDiscovery apidiscovery.APIGroupDiscoveryList err = json.Unmarshal(body, &aggregatedDiscovery) if err != nil { - return nil, nil, err + return nil, nil, nil, err } - apiGroupList, resourcesByGV = SplitGroupsAndResources(aggregatedDiscovery) + apiGroupList, resourcesByGV, failedGVs = SplitGroupsAndResources(aggregatedDiscovery) default: - return nil, nil, fmt.Errorf("Unknown discovery response content-type: %s", responseContentType) + return nil, nil, nil, fmt.Errorf("Unknown discovery response content-type: %s", responseContentType) } - return apiGroupList, resourcesByGV, nil + return apiGroupList, resourcesByGV, failedGVs, nil } // downloadAPIs returns the discovery groups and (if aggregated format) the // discovery resources. The returned groups will always exist, but the -// resources map may be nil. -func (d *DiscoveryClient) downloadAPIs() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error) { +// resources map may be nil. Returned "failedGVs" can be empty, but will +// only be nil in the case of a returned error. +func (d *DiscoveryClient) downloadAPIs() ( + *metav1.APIGroupList, + map[schema.GroupVersion]*metav1.APIResourceList, + map[schema.GroupVersion]error, + error) { accept := acceptDiscoveryFormats if d.UseLegacyDiscovery { accept = AcceptV1 @@ -284,36 +306,37 @@ func (d *DiscoveryClient) downloadAPIs() (*metav1.APIGroupList, map[schema.Group ContentType(&responseContentType). Raw() if err != nil { - return nil, nil, err + return nil, nil, nil, err } apiGroupList := &metav1.APIGroupList{} + failedGVs := map[schema.GroupVersion]error{} var resourcesByGV map[schema.GroupVersion]*metav1.APIResourceList // Switch on content-type server responded with: aggregated or unaggregated. switch responseContentType { case AcceptV1: err = json.Unmarshal(body, apiGroupList) if err != nil { - return nil, nil, err + return nil, nil, nil, err } case AcceptV2Beta1: var aggregatedDiscovery apidiscovery.APIGroupDiscoveryList err = json.Unmarshal(body, &aggregatedDiscovery) if err != nil { - return nil, nil, err + return nil, nil, nil, err } - apiGroupList, resourcesByGV = SplitGroupsAndResources(aggregatedDiscovery) + apiGroupList, resourcesByGV, failedGVs = SplitGroupsAndResources(aggregatedDiscovery) default: - return nil, nil, fmt.Errorf("Unknown discovery response content-type: %s", responseContentType) + return nil, nil, nil, fmt.Errorf("Unknown discovery response content-type: %s", responseContentType) } - return apiGroupList, resourcesByGV, nil + return apiGroupList, resourcesByGV, failedGVs, nil } // ServerGroups returns the supported groups, with information like supported versions and the // preferred version. func (d *DiscoveryClient) ServerGroups() (*metav1.APIGroupList, error) { - groups, _, err := d.GroupsAndMaybeResources() + groups, _, _, err := d.GroupsAndMaybeResources() if err != nil { return nil, err } @@ -380,13 +403,14 @@ func IsGroupDiscoveryFailedError(err error) bool { func ServerGroupsAndResources(d DiscoveryInterface) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) { var sgs *metav1.APIGroupList var resources []*metav1.APIResourceList + var failedGVs map[schema.GroupVersion]error var err error // If the passed discovery object implements the wider AggregatedDiscoveryInterface, // then attempt to retrieve aggregated discovery with both groups and the resources. if ad, ok := d.(AggregatedDiscoveryInterface); ok { var resourcesByGV map[schema.GroupVersion]*metav1.APIResourceList - sgs, resourcesByGV, err = ad.GroupsAndMaybeResources() + sgs, resourcesByGV, failedGVs, err = ad.GroupsAndMaybeResources() for _, resourceList := range resourcesByGV { resources = append(resources, resourceList) } @@ -401,8 +425,15 @@ func ServerGroupsAndResources(d DiscoveryInterface) ([]*metav1.APIGroup, []*meta for i := range sgs.Groups { resultGroups = append(resultGroups, &sgs.Groups[i]) } + // resources is non-nil if aggregated discovery succeeded. if resources != nil { - return resultGroups, resources, nil + // Any stale Group/Versions returned by aggregated discovery + // must be surfaced to the caller as failed Group/Versions. + var ferr error + if len(failedGVs) > 0 { + ferr = &ErrGroupDiscoveryFailed{Groups: failedGVs} + } + return resultGroups, resources, ferr } groupVersionResources, failedGroups := fetchGroupVersionResources(d, sgs) @@ -433,16 +464,18 @@ func ServerPreferredResources(d DiscoveryInterface) ([]*metav1.APIResourceList, var err error // If the passed discovery object implements the wider AggregatedDiscoveryInterface, - // then it is attempt to retrieve both the groups and the resources. + // then it is attempt to retrieve both the groups and the resources. "failedGroups" + // are Group/Versions returned as stale in AggregatedDiscovery format. ad, ok := d.(AggregatedDiscoveryInterface) if ok { - serverGroupList, groupVersionResources, err = ad.GroupsAndMaybeResources() + serverGroupList, groupVersionResources, failedGroups, err = ad.GroupsAndMaybeResources() } else { serverGroupList, err = d.ServerGroups() } if err != nil { return nil, err } + // Non-aggregated discovery must fetch resources from Groups. if groupVersionResources == nil { groupVersionResources, failedGroups = fetchGroupVersionResources(d, serverGroupList) } diff --git a/staging/src/k8s.io/client-go/discovery/discovery_client_test.go b/staging/src/k8s.io/client-go/discovery/discovery_client_test.go index fbf1ddff4e55..f41a051bca99 100644 --- a/staging/src/k8s.io/client-go/discovery/discovery_client_test.go +++ b/staging/src/k8s.io/client-go/discovery/discovery_client_test.go @@ -1435,6 +1435,7 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { expectedGroupNames []string expectedGroupVersions []string expectedGVKs []string + expectedFailedGVs []string }{ { name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/1 resources at /apis", @@ -1563,6 +1564,78 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { "apps/v2/Deployment", }, }, + { + name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/2 versions/1 resources at /apis", + corev1: &apidiscovery.APIGroupDiscoveryList{ + Items: []apidiscovery.APIGroupDiscovery{ + { + Versions: []apidiscovery.APIVersionDiscovery{ + { + Version: "v1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "pods", + ResponseKind: &metav1.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Pod", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + }, + }, + }, + }, + }, + apis: &apidiscovery.APIGroupDiscoveryList{ + Items: []apidiscovery.APIGroupDiscovery{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "apps", + }, + Versions: []apidiscovery.APIVersionDiscovery{ + { + Version: "v1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + }, + { + Version: "v2", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v2", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, + }, + }, + }, + }, + expectedGroupNames: []string{"", "apps"}, + expectedGroupVersions: []string{"v1", "apps/v1"}, + expectedGVKs: []string{ + "/v1/Pod", + "apps/v1/Deployment", + }, + expectedFailedGVs: []string{"apps/v2"}, + }, { name: "Aggregated discovery: 1 group/2 resources at /api, 1 group/2 resources at /apis", corev1: &apidiscovery.APIGroupDiscoveryList{ @@ -1603,6 +1676,31 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { Name: "apps", }, Versions: []apidiscovery.APIVersionDiscovery{ + // Stale "v2" version not included. + { + Version: "v2", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v2", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "statefulsets", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v2", + Kind: "StatefulSet", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, { Version: "v1", Resources: []apidiscovery.APIResourceDiscovery{ @@ -1638,9 +1736,10 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { "apps/v1/Deployment", "apps/v1/StatefulSet", }, + expectedFailedGVs: []string{"apps/v2"}, }, { - name: "Aggregated discovery: 1 group/2 resources at /api, 2 group/2 resources at /apis", + name: "Aggregated discovery: 1 group/2 resources at /api, 2 group/2 resources/1 stale GV at /apis", corev1: &apidiscovery.APIGroupDiscoveryList{ Items: []apidiscovery.APIGroupDiscovery{ { @@ -1709,6 +1808,7 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { Name: "batch", }, Versions: []apidiscovery.APIVersionDiscovery{ + // Stale Group/Version is not included { Version: "v1", Resources: []apidiscovery.APIResourceDiscovery{ @@ -1731,21 +1831,46 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { Scope: apidiscovery.ScopeNamespace, }, }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, + { + Version: "v1beta1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "jobs", + ResponseKind: &metav1.GroupVersionKind{ + Group: "batch", + Version: "v1beta1", + Kind: "Job", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "cronjobs", + ResponseKind: &metav1.GroupVersionKind{ + Group: "batch", + Version: "v1beta1", + Kind: "CronJob", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, }, }, }, }, }, expectedGroupNames: []string{"", "apps", "batch"}, - expectedGroupVersions: []string{"v1", "apps/v1", "batch/v1"}, + expectedGroupVersions: []string{"v1", "apps/v1", "batch/v1beta1"}, expectedGVKs: []string{ "/v1/Pod", "/v1/Service", "apps/v1/Deployment", "apps/v1/StatefulSet", - "batch/v1/Job", - "batch/v1/CronJob", + "batch/v1beta1/Job", + "batch/v1beta1/CronJob", }, + expectedFailedGVs: []string{"batch/v1"}, }, { name: "Aggregated discovery: /api returns nothing, 2 groups/2 resources at /apis", @@ -1810,6 +1935,31 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { }, }, }, + { + // Stale "v1beta1" not included. + Version: "v1beta1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "jobs", + ResponseKind: &metav1.GroupVersionKind{ + Group: "batch", + Version: "v1beta1", + Kind: "Job", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "cronjobs", + ResponseKind: &metav1.GroupVersionKind{ + Group: "batch", + Version: "v1beta1", + Kind: "CronJob", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, }, }, }, @@ -1822,6 +1972,7 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { "batch/v1/Job", "batch/v1/CronJob", }, + expectedFailedGVs: []string{"batch/v1beta1"}, }, } @@ -1847,7 +1998,15 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { defer server.Close() client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL}) apiGroups, resources, err := client.ServerGroupsAndResources() - require.NoError(t, err) + if len(test.expectedFailedGVs) > 0 { + require.Error(t, err) + expectedFailedGVs := sets.NewString(test.expectedFailedGVs...) + actualFailedGVs := sets.NewString(failedGroupVersions(err)...) + assert.True(t, expectedFailedGVs.Equal(actualFailedGVs), + "%s: Expected Failed GVs (%s), got (%s)", test.name, expectedFailedGVs, actualFailedGVs) + } else { + require.NoError(t, err) + } // Test the expected groups are returned for the aggregated format. expectedGroupNames := sets.NewString(test.expectedGroupNames...) actualGroupNames := sets.NewString(groupNames(apiGroups)...) @@ -1874,12 +2033,138 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) { } } +func TestAggregatedServerGroupsAndResourcesWithErrors(t *testing.T) { + tests := []struct { + name string + corev1 *apidiscovery.APIGroupDiscoveryList + coreHttpStatus int + apis *apidiscovery.APIGroupDiscoveryList + apisHttpStatus int + expectedGroups []string + expectedResources []string + expectedErr bool + }{ + { + name: "Aggregated Discovery: 404 for core/v1 is tolerated", + corev1: &apidiscovery.APIGroupDiscoveryList{}, + coreHttpStatus: http.StatusNotFound, + apis: &apidiscovery.APIGroupDiscoveryList{ + Items: []apidiscovery.APIGroupDiscovery{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "apps", + }, + Versions: []apidiscovery.APIVersionDiscovery{ + { + Version: "v1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "daemonsets", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1", + Kind: "DaemonSet", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + }, + }, + }, + }, + }, + apisHttpStatus: http.StatusOK, + expectedGroups: []string{"apps"}, + expectedResources: []string{"apps/v1/Deployment", "apps/v1/DaemonSet"}, + expectedErr: false, + }, + { + name: "Aggregated Discovery: 403 for core/v1 causes error", + corev1: &apidiscovery.APIGroupDiscoveryList{}, + coreHttpStatus: http.StatusForbidden, + apis: &apidiscovery.APIGroupDiscoveryList{}, + apisHttpStatus: http.StatusOK, + expectedErr: true, + }, + { + name: "Aggregated Discovery: 404 for /apis causes error", + corev1: &apidiscovery.APIGroupDiscoveryList{}, + coreHttpStatus: http.StatusOK, + apis: &apidiscovery.APIGroupDiscoveryList{}, + apisHttpStatus: http.StatusNotFound, + expectedErr: true, + }, + { + name: "Aggregated Discovery: 403 for /apis causes error", + corev1: &apidiscovery.APIGroupDiscoveryList{}, + coreHttpStatus: http.StatusOK, + apis: &apidiscovery.APIGroupDiscoveryList{}, + apisHttpStatus: http.StatusForbidden, + expectedErr: true, + }, + } + + for _, test := range tests { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + var agg *apidiscovery.APIGroupDiscoveryList + var status int + switch req.URL.Path { + case "/api": + agg = test.corev1 + status = test.coreHttpStatus + case "/apis": + agg = test.apis + status = test.apisHttpStatus + default: + w.WriteHeader(http.StatusNotFound) + return + } + output, err := json.Marshal(agg) + require.NoError(t, err) + // Content-type is "aggregated" discovery format. + w.Header().Set("Content-Type", AcceptV2Beta1) + w.WriteHeader(status) + w.Write(output) + })) + defer server.Close() + client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL}) + apiGroups, resources, err := client.ServerGroupsAndResources() + if test.expectedErr { + require.Error(t, err) + require.Nil(t, apiGroups) + require.Nil(t, resources) + continue + } + require.NoError(t, err) + // First check the returned groups + expectedGroups := sets.NewString(test.expectedGroups...) + actualGroups := sets.NewString(groupNames(apiGroups)...) + assert.True(t, expectedGroups.Equal(actualGroups), + "%s: Expected GVKs (%s), got (%s)", test.name, expectedGroups.List(), actualGroups.List()) + // Next check the returned resources + expectedGVKs := sets.NewString(test.expectedResources...) + actualGVKs := sets.NewString(groupVersionKinds(resources)...) + assert.True(t, expectedGVKs.Equal(actualGVKs), + "%s: Expected GVKs (%s), got (%s)", test.name, expectedGVKs.List(), actualGVKs.List()) + } +} + func TestAggregatedServerPreferredResources(t *testing.T) { tests := []struct { - name string - corev1 *apidiscovery.APIGroupDiscoveryList - apis *apidiscovery.APIGroupDiscoveryList - expectedGVKs []string + name string + corev1 *apidiscovery.APIGroupDiscoveryList + apis *apidiscovery.APIGroupDiscoveryList + expectedGVKs []string + expectedFailedGVs []string }{ { name: "Aggregated discovery: basic corev1 and apps/v1 preferred resources returned", @@ -2005,6 +2290,78 @@ func TestAggregatedServerPreferredResources(t *testing.T) { "apps/v2/Deployment", }, }, + { + name: "Aggregated discovery: stale Group/Version can not produce preferred version", + corev1: &apidiscovery.APIGroupDiscoveryList{ + Items: []apidiscovery.APIGroupDiscovery{ + { + Versions: []apidiscovery.APIVersionDiscovery{ + { + Version: "v1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "pods", + ResponseKind: &metav1.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Pod", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + }, + }, + }, + }, + }, + apis: &apidiscovery.APIGroupDiscoveryList{ + Items: []apidiscovery.APIGroupDiscovery{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "apps", + }, + Versions: []apidiscovery.APIVersionDiscovery{ + // v2 is "stale", so it can not be "preferred". + { + Version: "v2", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v2", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, + { + Version: "v1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + }, + }, + }, + }, + }, + // Only v1 resources from apps group; v2 would be preferred but it is "stale". + expectedGVKs: []string{ + "/v1/Pod", + "apps/v1/Deployment", + }, + expectedFailedGVs: []string{"apps/v2"}, + }, { name: "Aggregated discovery: preferred multiple resources from multiple group/versions", corev1: &apidiscovery.APIGroupDiscoveryList{ @@ -2068,6 +2425,30 @@ func TestAggregatedServerPreferredResources(t *testing.T) { }, }, }, + { + Version: "v1beta1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1beta1", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "statefulsets", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1beta1", + Kind: "StatefulSet", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + Freshness: apidiscovery.DiscoveryFreshnessStale, + }, }, }, }, @@ -2078,6 +2459,7 @@ func TestAggregatedServerPreferredResources(t *testing.T) { "apps/v1/Deployment", "apps/v1/StatefulSet", }, + expectedFailedGVs: []string{"apps/v1beta1"}, }, { name: "Aggregated discovery: resources from multiple preferred group versions at /apis", @@ -2142,6 +2524,30 @@ func TestAggregatedServerPreferredResources(t *testing.T) { }, }, }, + { + // Not included because "v1" is preferred. + Version: "v1beta1", + Resources: []apidiscovery.APIResourceDiscovery{ + { + Resource: "deployments", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1beta1", + Kind: "Deployment", + }, + Scope: apidiscovery.ScopeNamespace, + }, + { + Resource: "statefulsets", + ResponseKind: &metav1.GroupVersionKind{ + Group: "apps", + Version: "v1beta1", + Kind: "StatefulSet", + }, + Scope: apidiscovery.ScopeNamespace, + }, + }, + }, }, }, { @@ -2279,6 +2685,7 @@ func TestAggregatedServerPreferredResources(t *testing.T) { }, }, { + // Not included, since "v1" is preferred. Version: "v1beta1", Resources: []apidiscovery.APIResourceDiscovery{ { @@ -2339,7 +2746,15 @@ func TestAggregatedServerPreferredResources(t *testing.T) { defer server.Close() client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL}) resources, err := client.ServerPreferredResources() - require.NoError(t, err) + if len(test.expectedFailedGVs) > 0 { + require.Error(t, err) + expectedFailedGVs := sets.NewString(test.expectedFailedGVs...) + actualFailedGVs := sets.NewString(failedGroupVersions(err)...) + assert.True(t, expectedFailedGVs.Equal(actualFailedGVs), + "%s: Expected Failed GVs (%s), got (%s)", test.name, expectedFailedGVs, actualFailedGVs) + } else { + require.NoError(t, err) + } // Test the expected preferred GVKs are returned from the aggregated discovery. expectedGVKs := sets.NewString(test.expectedGVKs...) actualGVKs := sets.NewString(groupVersionKinds(resources)...) @@ -2421,3 +2836,15 @@ func groupVersionKinds(resources []*metav1.APIResourceList) []string { } return result } + +func failedGroupVersions(err error) []string { + result := []string{} + ferr, ok := err.(*ErrGroupDiscoveryFailed) + if !ok { + return result + } + for gv := range ferr.Groups { + result = append(result, gv.String()) + } + return result +}