Skip to content

Commit 66fe1a0

Browse files
Fedosinsbueringer
Fedosin
authored and
sbueringer
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 0b49f2e commit 66fe1a0

File tree

2 files changed

+143
-37
lines changed

2 files changed

+143
-37
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: 104 additions & 16 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
)
@@ -102,38 +107,38 @@ func TestLazyRestMapperProvider(t *testing.T) {
102107
// There are no requests before any call
103108
g.Expect(crt.GetRequestCount()).To(gmg.Equal(0))
104109

105-
mapping, err := lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "deployment"})
110+
mapping, err := lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "deployment"}, "v1")
106111
g.Expect(err).NotTo(gmg.HaveOccurred())
107112
g.Expect(mapping.GroupVersionKind.Kind).To(gmg.Equal("deployment"))
108-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(3))
113+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(1))
109114

110-
mappings, err := lazyRestMapper.RESTMappings(schema.GroupKind{Group: "", Kind: "pod"})
115+
mappings, err := lazyRestMapper.RESTMappings(schema.GroupKind{Group: "", Kind: "pod"}, "v1")
111116
g.Expect(err).NotTo(gmg.HaveOccurred())
112117
g.Expect(len(mappings)).To(gmg.Equal(1))
113118
g.Expect(mappings[0].GroupVersionKind.Kind).To(gmg.Equal("pod"))
114-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
119+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(2))
115120

116121
kind, err := lazyRestMapper.KindFor(schema.GroupVersionResource{Group: "networking.k8s.io", Version: "v1", Resource: "ingresses"})
117122
g.Expect(err).NotTo(gmg.HaveOccurred())
118123
g.Expect(kind.Kind).To(gmg.Equal("Ingress"))
119-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(5))
124+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(3))
120125

121126
kinds, err := lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "authentication.k8s.io", Version: "v1", Resource: "tokenreviews"})
122127
g.Expect(err).NotTo(gmg.HaveOccurred())
123128
g.Expect(len(kinds)).To(gmg.Equal(1))
124129
g.Expect(kinds[0].Kind).To(gmg.Equal("TokenReview"))
125-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
130+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
126131

127132
resource, err := lazyRestMapper.ResourceFor(schema.GroupVersionResource{Group: "scheduling.k8s.io", Version: "v1", Resource: "priorityclasses"})
128133
g.Expect(err).NotTo(gmg.HaveOccurred())
129134
g.Expect(resource.Resource).To(gmg.Equal("priorityclasses"))
130-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(7))
135+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(5))
131136

132137
resources, err := lazyRestMapper.ResourcesFor(schema.GroupVersionResource{Group: "policy", Version: "v1", Resource: "poddisruptionbudgets"})
133138
g.Expect(err).NotTo(gmg.HaveOccurred())
134139
g.Expect(len(resources)).To(gmg.Equal(1))
135140
g.Expect(resources[0].Resource).To(gmg.Equal("poddisruptionbudgets"))
136-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(8))
141+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
137142
})
138143

139144
t.Run("LazyRESTMapper should cache fetched data and doesn't perform any additional requests", func(t *testing.T) {
@@ -344,29 +349,29 @@ func TestLazyRestMapperProvider(t *testing.T) {
344349
lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, apiutil.WithExperimentalLazyMapper)
345350
g.Expect(err).NotTo(gmg.HaveOccurred())
346351

347-
_, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "INVALID"})
352+
_, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "INVALID"}, "v1")
348353
g.Expect(err).To(gmg.HaveOccurred())
349-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(3))
354+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(1))
350355

351-
_, err = lazyRestMapper.RESTMappings(schema.GroupKind{Group: "", Kind: "INVALID"})
356+
_, err = lazyRestMapper.RESTMappings(schema.GroupKind{Group: "", Kind: "INVALID"}, "v1")
352357
g.Expect(err).To(gmg.HaveOccurred())
353-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
358+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(2))
354359

355360
_, err = lazyRestMapper.KindFor(schema.GroupVersionResource{Group: "networking.k8s.io", Version: "v1", Resource: "INVALID"})
356361
g.Expect(err).To(gmg.HaveOccurred())
357-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(5))
362+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(3))
358363

359364
_, err = lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "authentication.k8s.io", Version: "v1", Resource: "INVALID"})
360365
g.Expect(err).To(gmg.HaveOccurred())
361-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
366+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
362367

363368
_, err = lazyRestMapper.ResourceFor(schema.GroupVersionResource{Group: "scheduling.k8s.io", Version: "v1", Resource: "INVALID"})
364369
g.Expect(err).To(gmg.HaveOccurred())
365-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(7))
370+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(5))
366371

367372
_, err = lazyRestMapper.ResourcesFor(schema.GroupVersionResource{Group: "policy", Version: "v1", Resource: "INVALID"})
368373
g.Expect(err).To(gmg.HaveOccurred())
369-
g.Expect(crt.GetRequestCount()).To(gmg.Equal(8))
374+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
370375
})
371376

372377
t.Run("LazyRESTMapper should return an error if the version doesn't exist", func(t *testing.T) {
@@ -407,4 +412,87 @@ func TestLazyRestMapperProvider(t *testing.T) {
407412
g.Expect(err).To(gmg.HaveOccurred())
408413
g.Expect(crt.GetRequestCount()).To(gmg.Equal(6))
409414
})
415+
416+
t.Run("LazyRESTMapper can fetch CRDs if they were created at runtime", func(t *testing.T) {
417+
g := gmg.NewWithT(t)
418+
419+
// To fetch all versions mapper does 2 requests:
420+
// GET https://host/api
421+
// GET https://host/apis
422+
// Then, for each version it performs just one request to the API server as usual:
423+
// GET https://host/apis/<group>/<version>
424+
425+
// Note: We have to use a separate restCfg for the Client, otherwise we
426+
// get a race condition on the counting round tripper between the Client
427+
// and the lazy restmapper.
428+
clientRestCfg := rest.CopyConfig(restCfg)
429+
430+
var crt *countingRoundTripper
431+
restCfg := rest.CopyConfig(restCfg)
432+
restCfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
433+
crt = newCountingRoundTripper(rt)
434+
return crt
435+
}
436+
437+
lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, apiutil.WithExperimentalLazyMapper)
438+
g.Expect(err).NotTo(gmg.HaveOccurred())
439+
440+
// There are no requests before any call
441+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(0))
442+
443+
// Since we don't specify what version we expect, restmapper will fetch them all and search there.
444+
// To fetch a list of available versions
445+
// #1: GET https://host/api
446+
// #2: GET https://host/apis
447+
// Then, for each currently registered version:
448+
// #3: GET https://host/apis/crew.example.com/v1
449+
// #4: GET https://host/apis/crew.example.com/v2
450+
mapping, err := lazyRestMapper.RESTMapping(schema.GroupKind{Group: "crew.example.com", Kind: "driver"})
451+
g.Expect(err).NotTo(gmg.HaveOccurred())
452+
g.Expect(mapping.GroupVersionKind.Kind).To(gmg.Equal("driver"))
453+
g.Expect(crt.GetRequestCount()).To(gmg.Equal(4))
454+
455+
s := scheme.Scheme
456+
err = apiextensionsv1.AddToScheme(s)
457+
g.Expect(err).NotTo(gmg.HaveOccurred())
458+
459+
c, err := client.New(clientRestCfg, client.Options{Scheme: s})
460+
g.Expect(err).NotTo(gmg.HaveOccurred())
461+
462+
// Register another CRD in runtime - "riders.crew.example.com".
463+
464+
crd := &apiextensionsv1.CustomResourceDefinition{}
465+
err = c.Get(context.TODO(), types.NamespacedName{Name: "drivers.crew.example.com"}, crd)
466+
g.Expect(err).NotTo(gmg.HaveOccurred())
467+
g.Expect(crd.Spec.Names.Kind).To(gmg.Equal("Driver"))
468+
469+
newCRD := &apiextensionsv1.CustomResourceDefinition{}
470+
crd.DeepCopyInto(newCRD)
471+
newCRD.Name = "riders.crew.example.com"
472+
newCRD.Spec.Names = apiextensionsv1.CustomResourceDefinitionNames{
473+
Kind: "Rider",
474+
Plural: "riders",
475+
}
476+
newCRD.ResourceVersion = ""
477+
478+
// Create the new CRD.
479+
g.Expect(c.Create(context.TODO(), newCRD)).To(gmg.Succeed())
480+
481+
// Wait a bit until the CRD is registered.
482+
g.Eventually(func() error {
483+
_, err := lazyRestMapper.RESTMapping(schema.GroupKind{Group: "crew.example.com", Kind: "rider"})
484+
return err
485+
}).Should(gmg.Succeed())
486+
487+
// Since we don't specify what version we expect, restmapper will fetch them all and search there.
488+
// To fetch a list of available versions
489+
// #1: GET https://host/api
490+
// #2: GET https://host/apis
491+
// Then, for each currently registered version:
492+
// #3: GET https://host/apis/crew.example.com/v1
493+
// #4: GET https://host/apis/crew.example.com/v2
494+
mapping, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "crew.example.com", Kind: "rider"})
495+
g.Expect(err).NotTo(gmg.HaveOccurred())
496+
g.Expect(mapping.GroupVersionKind.Kind).To(gmg.Equal("rider"))
497+
})
410498
}

0 commit comments

Comments
 (0)