Skip to content

Commit 5941f84

Browse files
FedosinFedosin
committed
Allow lazy restmapper to work with CRDs created in runtime
Now lazy restmapper fetches all API resources once at start and then caches them. It prevents it from discovery of new CRDs created after the controller has started. This commit allows lazy restmapper to work with such CRDs.
1 parent f6f37e6 commit 5941f84

File tree

2 files changed

+136
-39
lines changed

2 files changed

+136
-39
lines changed

‎pkg/client/apiutil/lazyrestmapper.go‎

Lines changed: 36 additions & 18 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

@@ -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.
@@ -199,26 +211,32 @@ func (m *lazyRESTMapper) addKnownGroupAndReload(groupName string, versions ...st
199211

200212
// findAPIGroupByName returns API group by its name.
201213
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")
214+
// Looking in the cache first.
215+
for i := range m.apiGroups {
216+
if groupName == (&m.apiGroups[i]).Name {
217+
return m.apiGroups[i], 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 i := range m.apiGroups {
234+
if groupName == (&m.apiGroups[i]).Name {
235+
return m.apiGroups[i], 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)