Skip to content

Commit e2d8821

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2208 from Fedosin/lazyrestmapper_runtime
🐛 Allow lazy restmapper to work with CRDs created at runtime
2 parents 6229718 + e95db6c commit e2d8821

File tree

2 files changed

+139
-42
lines changed

2 files changed

+139
-42
lines changed

‎pkg/client/apiutil/lazyrestmapper.go

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type lazyRESTMapper struct {
3333
mapper meta.RESTMapper
3434
client *discovery.DiscoveryClient
3535
knownGroups map[string]*restmapper.APIGroupResources
36-
apiGroups *metav1.APIGroupList
36+
apiGroups []metav1.APIGroup
3737

3838
// mutex to provide thread-safe mapper reloading.
3939
mu sync.Mutex
@@ -45,6 +45,7 @@ func newLazyRESTMapperWithClient(discoveryClient *discovery.DiscoveryClient) (me
4545
mapper: restmapper.NewDiscoveryRESTMapper([]*restmapper.APIGroupResources{}),
4646
client: discoveryClient,
4747
knownGroups: map[string]*restmapper.APIGroupResources{},
48+
apiGroups: []metav1.APIGroup{},
4849
}, nil
4950
}
5051

@@ -147,7 +148,7 @@ func (m *lazyRESTMapper) addKnownGroupAndReload(groupName string, versions ...st
147148
// This operation requires 2 requests: /api and /apis, but only once. For all subsequent calls
148149
// this data will be taken from cache.
149150
if len(versions) == 0 {
150-
apiGroup, err := m.findAPIGroupByName(groupName)
151+
apiGroup, err := m.findAPIGroupByNameLocked(groupName)
151152
if err != nil {
152153
return err
153154
}
@@ -176,11 +177,22 @@ func (m *lazyRESTMapper) addKnownGroupAndReload(groupName string, versions ...st
176177
}
177178

178179
// Update information for group resources about the API group by adding new versions.
180+
// Ignore the versions that are already registered.
179181
for _, version := range versions {
180-
groupResources.Group.Versions = append(groupResources.Group.Versions, metav1.GroupVersionForDiscovery{
181-
GroupVersion: metav1.GroupVersion{Group: groupName, Version: version}.String(),
182-
Version: version,
183-
})
182+
found := false
183+
for _, v := range groupResources.Group.Versions {
184+
if v.Version == version {
185+
found = true
186+
break
187+
}
188+
}
189+
190+
if !found {
191+
groupResources.Group.Versions = append(groupResources.Group.Versions, metav1.GroupVersionForDiscovery{
192+
GroupVersion: metav1.GroupVersion{Group: groupName, Version: version}.String(),
193+
Version: version,
194+
})
195+
}
184196
}
185197

186198
// Update data in the cache.
@@ -197,28 +209,34 @@ func (m *lazyRESTMapper) addKnownGroupAndReload(groupName string, versions ...st
197209
return nil
198210
}
199211

200-
// findAPIGroupByName returns API group by its name.
201-
func (m *lazyRESTMapper) findAPIGroupByName(groupName string) (metav1.APIGroup, error) {
202-
// Ensure that required info about existing API groups is received and stored in the mapper.
203-
// It will make 2 API calls to /api and /apis, but only once.
204-
if m.apiGroups == nil {
205-
apiGroups, err := m.client.ServerGroups()
206-
if err != nil {
207-
return metav1.APIGroup{}, fmt.Errorf("failed to get server groups: %w", err)
208-
}
209-
if len(apiGroups.Groups) == 0 {
210-
return metav1.APIGroup{}, fmt.Errorf("received an empty API groups list")
212+
// findAPIGroupByNameLocked returns API group by its name.
213+
func (m *lazyRESTMapper) findAPIGroupByNameLocked(groupName string) (metav1.APIGroup, error) {
214+
// Looking in the cache first.
215+
for _, apiGroup := range m.apiGroups {
216+
if groupName == apiGroup.Name {
217+
return apiGroup, nil
211218
}
219+
}
212220

213-
m.apiGroups = apiGroups
221+
// Update the cache if nothing was found.
222+
apiGroups, err := m.client.ServerGroups()
223+
if err != nil {
224+
return metav1.APIGroup{}, fmt.Errorf("failed to get server groups: %w", err)
214225
}
226+
if len(apiGroups.Groups) == 0 {
227+
return metav1.APIGroup{}, fmt.Errorf("received an empty API groups list")
228+
}
229+
230+
m.apiGroups = apiGroups.Groups
215231

216-
for i := range m.apiGroups.Groups {
217-
if groupName == (&m.apiGroups.Groups[i]).Name {
218-
return m.apiGroups.Groups[i], nil
232+
// Looking in the cache again.
233+
for _, apiGroup := range m.apiGroups {
234+
if groupName == apiGroup.Name {
235+
return apiGroup, nil
219236
}
220237
}
221238

239+
// If there is still nothing, return an error.
222240
return metav1.APIGroup{}, fmt.Errorf("failed to find API group %s", groupName)
223241
}
224242

‎pkg/client/apiutil/lazyrestmapper_test.go

Lines changed: 100 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,19 @@ limitations under the License.
1717
package apiutil_test
1818

1919
import (
20+
"context"
2021
"net/http"
2122
"testing"
2223

2324
_ "github.com/onsi/ginkgo/v2"
2425
gmg "github.com/onsi/gomega"
2526

27+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2628
"k8s.io/apimachinery/pkg/runtime/schema"
29+
"k8s.io/apimachinery/pkg/types"
30+
"k8s.io/client-go/kubernetes/scheme"
2731
"k8s.io/client-go/rest"
32+
"sigs.k8s.io/controller-runtime/pkg/client"
2833
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
2934
"sigs.k8s.io/controller-runtime/pkg/envtest"
3035
)
@@ -83,10 +88,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
8388
t.Run("LazyRESTMapper should fetch data based on the request", func(t *testing.T) {
8489
g := gmg.NewWithT(t)
8590

86-
// To initialize mapper does 2 requests:
87-
// GET https://host/api
88-
// GET https://host/apis
89-
// Then, for each new group it performs just one request to the API server:
91+
// For each new group it performs just one request to the API server:
9092
// GET https://host/apis/<group>/<version>
9193

9294
httpClient, err := rest.HTTPClientFor(restCfg)
@@ -101,38 +103,38 @@ func TestLazyRestMapperProvider(t *testing.T) {
101103
// There are no requests before any call
102104
g.Expect(crt.GetRequestCount()).To(gmg.Equal(0))
103105

104-
mapping, err := lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "deployment"})
106+
mapping, err := lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "deployment"}, "v1")
105107
g.Expect(err).NotTo(gmg.HaveOccurred())
106108
g.Expect(mapping.GroupVersionKind.Kind).To(gmg.Equal("deployment"))
107-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(3))
109+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(1))
108110

109-
mappings, err := lazyRestMapper.RESTMappings(schema.GroupKind{Group: "", Kind: "pod"})
111+
mappings, err := lazyRestMapper.RESTMappings(schema.GroupKind{Group: "", Kind: "pod"}, "v1")
110112
g.Expect(err).NotTo(gmg.HaveOccurred())
111113
g.Expect(len(mappings)).To(gmg.Equal(1))
112114
g.Expect(mappings[0].GroupVersionKind.Kind).To(gmg.Equal("pod"))
113-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
115+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(2))
114116

115117
kind, err := lazyRestMapper.KindFor(schema.GroupVersionResource{Group: "networking.k8s.io", Version: "v1", Resource: "ingresses"})
116118
g.Expect(err).NotTo(gmg.HaveOccurred())
117119
g.Expect(kind.Kind).To(gmg.Equal("Ingress"))
118-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(5))
120+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(3))
119121

120122
kinds, err := lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "authentication.k8s.io", Version: "v1", Resource: "tokenreviews"})
121123
g.Expect(err).NotTo(gmg.HaveOccurred())
122124
g.Expect(len(kinds)).To(gmg.Equal(1))
123125
g.Expect(kinds[0].Kind).To(gmg.Equal("TokenReview"))
124-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
126+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
125127

126128
resource, err := lazyRestMapper.ResourceFor(schema.GroupVersionResource{Group: "scheduling.k8s.io", Version: "v1", Resource: "priorityclasses"})
127129
g.Expect(err).NotTo(gmg.HaveOccurred())
128130
g.Expect(resource.Resource).To(gmg.Equal("priorityclasses"))
129-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(7))
131+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(5))
130132

131133
resources, err := lazyRestMapper.ResourcesFor(schema.GroupVersionResource{Group: "policy", Version: "v1", Resource: "poddisruptionbudgets"})
132134
g.Expect(err).NotTo(gmg.HaveOccurred())
133135
g.Expect(len(resources)).To(gmg.Equal(1))
134136
g.Expect(resources[0].Resource).To(gmg.Equal("poddisruptionbudgets"))
135-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(8))
137+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
136138
})
137139

138140
t.Run("LazyRESTMapper should cache fetched data and doesn't perform any additional requests", func(t *testing.T) {
@@ -327,7 +329,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
327329
t.Run("LazyRESTMapper should return an error if a resource doesn't exist", func(t *testing.T) {
328330
g := gmg.NewWithT(t)
329331

330-
// After initialization for each invalid resource the mapper performs just 1 request to the API server.
332+
// For each invalid resource the mapper performs just 1 request to the API server.
331333

332334
httpClient, err := rest.HTTPClientFor(restCfg)
333335
g.Expect(err).NotTo(gmg.HaveOccurred())
@@ -338,29 +340,29 @@ func TestLazyRestMapperProvider(t *testing.T) {
338340
lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
339341
g.Expect(err).NotTo(gmg.HaveOccurred())
340342

341-
_, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "INVALID"})
343+
_, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "INVALID"}, "v1")
342344
g.Expect(err).To(gmg.HaveOccurred())
343-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(3))
345+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(1))
344346

345-
_, err = lazyRestMapper.RESTMappings(schema.GroupKind{Group: "", Kind: "INVALID"})
347+
_, err = lazyRestMapper.RESTMappings(schema.GroupKind{Group: "", Kind: "INVALID"}, "v1")
346348
g.Expect(err).To(gmg.HaveOccurred())
347-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
349+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(2))
348350

349351
_, err = lazyRestMapper.KindFor(schema.GroupVersionResource{Group: "networking.k8s.io", Version: "v1", Resource: "INVALID"})
350352
g.Expect(err).To(gmg.HaveOccurred())
351-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(5))
353+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(3))
352354

353355
_, err = lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "authentication.k8s.io", Version: "v1", Resource: "INVALID"})
354356
g.Expect(err).To(gmg.HaveOccurred())
355-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
357+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
356358

357359
_, err = lazyRestMapper.ResourceFor(schema.GroupVersionResource{Group: "scheduling.k8s.io", Version: "v1", Resource: "INVALID"})
358360
g.Expect(err).To(gmg.HaveOccurred())
359-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(7))
361+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(5))
360362

361363
_, err = lazyRestMapper.ResourcesFor(schema.GroupVersionResource{Group: "policy", Version: "v1", Resource: "INVALID"})
362364
g.Expect(err).To(gmg.HaveOccurred())
363-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(8))
365+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
364366
})
365367

366368
t.Run("LazyRESTMapper should return an error if the version doesn't exist", func(t *testing.T) {
@@ -401,4 +403,81 @@ func TestLazyRestMapperProvider(t *testing.T) {
401403
g.Expect(err).To(gmg.HaveOccurred())
402404
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
403405
})
406+
407+
t.Run("LazyRESTMapper can fetch CRDs if they were created at runtime", func(t *testing.T) {
408+
g := gmg.NewWithT(t)
409+
410+
// To fetch all versions mapper does 2 requests:
411+
// GET https://host/api
412+
// GET https://host/apis
413+
// Then, for each version it performs just one request to the API server as usual:
414+
// GET https://host/apis/<group>/<version>
415+
416+
httpClient, err := rest.HTTPClientFor(restCfg)
417+
g.Expect(err).NotTo(gmg.HaveOccurred())
418+
419+
crt := newCountingRoundTripper(httpClient.Transport)
420+
httpClient.Transport = crt
421+
422+
lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
423+
g.Expect(err).NotTo(gmg.HaveOccurred())
424+
425+
// There are no requests before any call
426+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(0))
427+
428+
// Since we don't specify what version we expect, restmapper will fetch them all and search there.
429+
// To fetch a list of available versions
430+
// #1: GET https://host/api
431+
// #2: GET https://host/apis
432+
// Then, for each currently registered version:
433+
// #3: GET https://host/apis/crew.example.com/v1
434+
// #4: GET https://host/apis/crew.example.com/v2
435+
mapping, err := lazyRestMapper.RESTMapping(schema.GroupKind{Group: "crew.example.com", Kind: "driver"})
436+
g.Expect(err).NotTo(gmg.HaveOccurred())
437+
g.Expect(mapping.GroupVersionKind.Kind).To(gmg.Equal("driver"))
438+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
439+
440+
s := scheme.Scheme
441+
err = apiextensionsv1.AddToScheme(s)
442+
g.Expect(err).NotTo(gmg.HaveOccurred())
443+
444+
c, err := client.New(restCfg, client.Options{Scheme: s})
445+
g.Expect(err).NotTo(gmg.HaveOccurred())
446+
447+
// Register another CRD in runtime - "riders.crew.example.com".
448+
449+
crd := &apiextensionsv1.CustomResourceDefinition{}
450+
err = c.Get(context.TODO(), types.NamespacedName{Name: "drivers.crew.example.com"}, crd)
451+
g.Expect(err).NotTo(gmg.HaveOccurred())
452+
g.Expect(crd.Spec.Names.Kind).To(gmg.Equal("Driver"))
453+
454+
newCRD := &apiextensionsv1.CustomResourceDefinition{}
455+
crd.DeepCopyInto(newCRD)
456+
newCRD.Name = "riders.crew.example.com"
457+
newCRD.Spec.Names = apiextensionsv1.CustomResourceDefinitionNames{
458+
Kind: "Rider",
459+
Plural: "riders",
460+
}
461+
newCRD.ResourceVersion = ""
462+
463+
// Create the new CRD.
464+
g.Expect(c.Create(context.TODO(), newCRD)).To(gmg.Succeed())
465+
466+
// Wait a bit until the CRD is registered.
467+
g.Eventually(func() error {
468+
_, err := lazyRestMapper.RESTMapping(schema.GroupKind{Group: "crew.example.com", Kind: "rider"})
469+
return err
470+
}).Should(gmg.Succeed())
471+
472+
// Since we don't specify what version we expect, restmapper will fetch them all and search there.
473+
// To fetch a list of available versions
474+
// #1: GET https://host/api
475+
// #2: GET https://host/apis
476+
// Then, for each currently registered version:
477+
// #3: GET https://host/apis/crew.example.com/v1
478+
// #4: GET https://host/apis/crew.example.com/v2
479+
mapping, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "crew.example.com", Kind: "rider"})
480+
g.Expect(err).NotTo(gmg.HaveOccurred())
481+
g.Expect(mapping.GroupVersionKind.Kind).To(gmg.Equal("rider"))
482+
})
404483
}

0 commit comments

Comments
 (0)