Skip to content

Commit d0396a3

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2688 from k8s-infra-cherrypick-robot/cherry-pick-2687-to-release-0.17
[release-0.17] 🐛 Fix lazy rest mapper cache invalidation
2 parents 59c26c0 + 565aa5b commit d0396a3

File tree

2 files changed

+227
-4
lines changed

2 files changed

+227
-4
lines changed

‎pkg/client/apiutil/restmapper.go

+24-4
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func NewDynamicRESTMapper(cfg *rest.Config, httpClient *http.Client) (meta.RESTM
5353
// client for discovery information to do REST mappings.
5454
type mapper struct {
5555
mapper meta.RESTMapper
56-
client *discovery.DiscoveryClient
56+
client discovery.DiscoveryInterface
5757
knownGroups map[string]*restmapper.APIGroupResources
5858
apiGroups map[string]*metav1.APIGroup
5959

@@ -280,11 +280,15 @@ func (m *mapper) fetchGroupVersionResourcesLocked(groupName string, versions ...
280280
groupVersion := schema.GroupVersion{Group: groupName, Version: version}
281281

282282
apiResourceList, err := m.client.ServerResourcesForGroupVersion(groupVersion.String())
283-
if apierrors.IsNotFound(err) && m.isGroupVersionCached(groupVersion) {
283+
if apierrors.IsNotFound(err) {
284284
// If the version is not found, we remove the group from the cache
285285
// so it gets refreshed on the next call.
286-
delete(m.apiGroups, groupName)
287-
delete(m.knownGroups, groupName)
286+
if m.isAPIGroupCached(groupVersion) {
287+
delete(m.apiGroups, groupName)
288+
}
289+
if m.isGroupVersionCached(groupVersion) {
290+
delete(m.knownGroups, groupName)
291+
}
288292
continue
289293
} else if err != nil {
290294
failedGroups[groupVersion] = err
@@ -313,3 +317,19 @@ func (m *mapper) isGroupVersionCached(gv schema.GroupVersion) bool {
313317

314318
return false
315319
}
320+
321+
// isAPIGroupCached checks if a version for a group is cached in the api groups cache.
322+
func (m *mapper) isAPIGroupCached(gv schema.GroupVersion) bool {
323+
cachedGroup, ok := m.apiGroups[gv.Group]
324+
if !ok {
325+
return false
326+
}
327+
328+
for _, version := range cachedGroup.Versions {
329+
if version.Version == gv.Version {
330+
return true
331+
}
332+
}
333+
334+
return false
335+
}

‎pkg/client/apiutil/restmapper_wb_test.go

+203
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
/*
2+
Copyright 2024 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package apiutil
18+
19+
import (
20+
"testing"
21+
22+
gmg "github.com/onsi/gomega"
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
"k8s.io/client-go/kubernetes/fake"
25+
"k8s.io/client-go/restmapper"
26+
)
27+
28+
func TestLazyRestMapper_fetchGroupVersionResourcesLocked_CacheInvalidation(t *testing.T) {
29+
tests := []struct {
30+
name string
31+
groupName string
32+
versions []string
33+
cachedAPIGroups, expectedAPIGroups map[string]*metav1.APIGroup
34+
cachedKnownGroups, expectedKnownGroups map[string]*restmapper.APIGroupResources
35+
}{
36+
{
37+
name: "Not found version for cached groupVersion in apiGroups and knownGroups",
38+
groupName: "group1",
39+
versions: []string{"v1", "v2"},
40+
cachedAPIGroups: map[string]*metav1.APIGroup{
41+
"group1": {
42+
Name: "group1",
43+
Versions: []metav1.GroupVersionForDiscovery{
44+
{
45+
Version: "v1",
46+
},
47+
},
48+
},
49+
},
50+
cachedKnownGroups: map[string]*restmapper.APIGroupResources{
51+
"group1": {
52+
VersionedResources: map[string][]metav1.APIResource{
53+
"v1": {
54+
{
55+
Name: "resource1",
56+
},
57+
},
58+
},
59+
},
60+
},
61+
expectedAPIGroups: map[string]*metav1.APIGroup{},
62+
expectedKnownGroups: map[string]*restmapper.APIGroupResources{},
63+
},
64+
{
65+
name: "Not found version for cached groupVersion only in apiGroups",
66+
groupName: "group1",
67+
versions: []string{"v1", "v2"},
68+
cachedAPIGroups: map[string]*metav1.APIGroup{
69+
"group1": {
70+
Name: "group1",
71+
Versions: []metav1.GroupVersionForDiscovery{
72+
{
73+
Version: "v1",
74+
},
75+
},
76+
},
77+
},
78+
cachedKnownGroups: map[string]*restmapper.APIGroupResources{
79+
"group1": {
80+
VersionedResources: map[string][]metav1.APIResource{
81+
"v3": {
82+
{
83+
Name: "resource1",
84+
},
85+
},
86+
},
87+
},
88+
},
89+
expectedAPIGroups: map[string]*metav1.APIGroup{},
90+
expectedKnownGroups: map[string]*restmapper.APIGroupResources{
91+
"group1": {
92+
VersionedResources: map[string][]metav1.APIResource{
93+
"v3": {
94+
{
95+
Name: "resource1",
96+
},
97+
},
98+
},
99+
},
100+
},
101+
},
102+
{
103+
name: "Not found version for cached groupVersion only in knownGroups",
104+
groupName: "group1",
105+
versions: []string{"v1", "v2"},
106+
cachedAPIGroups: map[string]*metav1.APIGroup{
107+
"group1": {
108+
Name: "group1",
109+
Versions: []metav1.GroupVersionForDiscovery{
110+
{
111+
Version: "v3",
112+
},
113+
},
114+
},
115+
},
116+
cachedKnownGroups: map[string]*restmapper.APIGroupResources{
117+
"group1": {
118+
VersionedResources: map[string][]metav1.APIResource{
119+
"v2": {
120+
{
121+
Name: "resource1",
122+
},
123+
},
124+
},
125+
},
126+
},
127+
expectedAPIGroups: map[string]*metav1.APIGroup{
128+
"group1": {
129+
Name: "group1",
130+
Versions: []metav1.GroupVersionForDiscovery{
131+
{
132+
Version: "v3",
133+
},
134+
},
135+
},
136+
},
137+
expectedKnownGroups: map[string]*restmapper.APIGroupResources{},
138+
},
139+
{
140+
name: "Not found version for non cached groupVersion",
141+
groupName: "group1",
142+
versions: []string{"v1", "v2"},
143+
cachedAPIGroups: map[string]*metav1.APIGroup{
144+
"group1": {
145+
Name: "group1",
146+
Versions: []metav1.GroupVersionForDiscovery{
147+
{
148+
Version: "v3",
149+
},
150+
},
151+
},
152+
},
153+
cachedKnownGroups: map[string]*restmapper.APIGroupResources{
154+
"group1": {
155+
VersionedResources: map[string][]metav1.APIResource{
156+
"v3": {
157+
{
158+
Name: "resource1",
159+
},
160+
},
161+
},
162+
},
163+
},
164+
expectedAPIGroups: map[string]*metav1.APIGroup{
165+
"group1": {
166+
Name: "group1",
167+
Versions: []metav1.GroupVersionForDiscovery{
168+
{
169+
Version: "v3",
170+
},
171+
},
172+
},
173+
},
174+
expectedKnownGroups: map[string]*restmapper.APIGroupResources{
175+
"group1": {
176+
VersionedResources: map[string][]metav1.APIResource{
177+
"v3": {
178+
{
179+
Name: "resource1",
180+
},
181+
},
182+
},
183+
},
184+
},
185+
},
186+
}
187+
188+
for _, tt := range tests {
189+
t.Run(tt.name, func(t *testing.T) {
190+
g := gmg.NewWithT(t)
191+
m := &mapper{
192+
mapper: restmapper.NewDiscoveryRESTMapper([]*restmapper.APIGroupResources{}),
193+
client: fake.NewSimpleClientset().Discovery(),
194+
apiGroups: tt.cachedAPIGroups,
195+
knownGroups: tt.cachedKnownGroups,
196+
}
197+
_, err := m.fetchGroupVersionResourcesLocked(tt.groupName, tt.versions...)
198+
g.Expect(err).NotTo(gmg.HaveOccurred())
199+
g.Expect(m.apiGroups).To(gmg.BeComparableTo(tt.expectedAPIGroups))
200+
g.Expect(m.knownGroups).To(gmg.BeComparableTo(tt.expectedKnownGroups))
201+
})
202+
}
203+
}

0 commit comments

Comments
 (0)