Use DynamicRESTMapperOption to enable Lazy Restmapper

Instead of creating the instance of the mapper directly, we will use WithExperimentalLazyMapper option for Dynamic Restmapper.
kubernetes-sigs · Feb 6, 2023 · f4d10a3 · f4d10a3
1 parent b3f4882
commit f4d10a3
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 59 deletions.
diff --git a/pkg/client/apiutil/dynamicrestmapper.go b/pkg/client/apiutil/dynamicrestmapper.go
@@ -40,6 +40,8 @@ type dynamicRESTMapper struct {
 	// Used for lazy init.
 	inited  uint32
 	initMtx sync.Mutex
+
+	useLazyRestmapper bool
 }
 
 // DynamicRESTMapperOption is a functional option on the dynamicRESTMapper.
@@ -60,6 +62,12 @@ var WithLazyDiscovery DynamicRESTMapperOption = func(drm *dynamicRESTMapper) err
 	return nil
 }
 
+// WithExperimentalLazyMapper enables experimental more advanced Lazy Restmapping mechanism.
+var WithExperimentalLazyMapper DynamicRESTMapperOption = func(drm *dynamicRESTMapper) error {
+	drm.useLazyRestmapper = true
+	return nil
+}
+
 // WithCustomMapper supports setting a custom RESTMapper refresher instead of
 // the default method, which uses a discovery client.
 //
@@ -95,6 +103,9 @@ func NewDynamicRESTMapper(cfg *rest.Config, opts ...DynamicRESTMapperOption) (me
 			return nil, err
 		}
 	}
+	if drm.useLazyRestmapper {
+		return newLazyRESTMapperWithClient(client)
+	}
 	if !drm.lazy {
 		if err := drm.setStaticMapper(); err != nil {
 			return nil, err

diff --git a/pkg/client/apiutil/lazyrestmapper.go b/pkg/client/apiutil/lazyrestmapper.go
@@ -24,13 +24,12 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
-	"k8s.io/client-go/rest"
 	"k8s.io/client-go/restmapper"
 )
 
-// LazyRESTMapper is a RESTMapper that will lazily query the provided
+// lazyRESTMapper is a RESTMapper that will lazily query the provided
 // client for discovery information to do REST mappings.
-type LazyRESTMapper struct {
+type lazyRESTMapper struct {
 	mapper      meta.RESTMapper
 	client      *discovery.DiscoveryClient
 	knownGroups map[string]*restmapper.APIGroupResources
@@ -40,27 +39,17 @@ type LazyRESTMapper struct {
 	mu sync.Mutex
 }
 
-// NewLazyRESTMapper initializes a LazyRESTMapper.
-func NewLazyRESTMapper(c *rest.Config) (meta.RESTMapper, error) {
-	discoveryClient, err := discovery.NewDiscoveryClientForConfig(c)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create discovery client: %w", err)
-	}
-
-	return NewLazyRESTMapperWithClient(discoveryClient)
-}
-
-// NewLazyRESTMapperWithClient initializes a LazyRESTMapper with a custom discovery client.
-func NewLazyRESTMapperWithClient(discoveryClient *discovery.DiscoveryClient) (meta.RESTMapper, error) {
-	return &LazyRESTMapper{
+// newLazyRESTMapperWithClient initializes a LazyRESTMapper with a custom discovery client.
+func newLazyRESTMapperWithClient(discoveryClient *discovery.DiscoveryClient) (meta.RESTMapper, error) {
+	return &lazyRESTMapper{
 		mapper:      restmapper.NewDiscoveryRESTMapper([]*restmapper.APIGroupResources{}),
 		client:      discoveryClient,
 		knownGroups: map[string]*restmapper.APIGroupResources{},
 	}, nil
 }
 
 // KindFor implements Mapper.KindFor.
-func (m *LazyRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
+func (m *lazyRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
 	res, err := m.mapper.KindFor(resource)
 	if meta.IsNoMatchError(err) {
 		if err = m.addKnownGroupAndReload(resource.Group, resource.Version); err != nil {
@@ -74,7 +63,7 @@ func (m *LazyRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.G
 }
 
 // KindsFor implements Mapper.KindsFor.
-func (m *LazyRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
+func (m *lazyRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
 	res, err := m.mapper.KindsFor(resource)
 	if meta.IsNoMatchError(err) {
 		if err = m.addKnownGroupAndReload(resource.Group, resource.Version); err != nil {
@@ -88,7 +77,7 @@ func (m *LazyRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]schem
 }
 
 // ResourceFor implements Mapper.ResourceFor.
-func (m *LazyRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) {
+func (m *lazyRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) {
 	res, err := m.mapper.ResourceFor(input)
 	if meta.IsNoMatchError(err) {
 		if err = m.addKnownGroupAndReload(input.Group, input.Version); err != nil {
@@ -102,7 +91,7 @@ func (m *LazyRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.
 }
 
 // ResourcesFor implements Mapper.ResourcesFor.
-func (m *LazyRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) {
+func (m *lazyRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) {
 	res, err := m.mapper.ResourcesFor(input)
 	if meta.IsNoMatchError(err) {
 		if err = m.addKnownGroupAndReload(input.Group, input.Version); err != nil {
@@ -116,7 +105,7 @@ func (m *LazyRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]sche
 }
 
 // RESTMapping implements Mapper.RESTMapping.
-func (m *LazyRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
+func (m *lazyRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
 	res, err := m.mapper.RESTMapping(gk, versions...)
 	if meta.IsNoMatchError(err) {
 		if err = m.addKnownGroupAndReload(gk.Group, versions...); err != nil {
@@ -130,7 +119,7 @@ func (m *LazyRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*
 }
 
 // RESTMappings implements Mapper.RESTMappings.
-func (m *LazyRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
+func (m *lazyRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
 	res, err := m.mapper.RESTMappings(gk, versions...)
 	if meta.IsNoMatchError(err) {
 		if err = m.addKnownGroupAndReload(gk.Group, versions...); err != nil {
@@ -144,13 +133,13 @@ func (m *LazyRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) (
 }
 
 // ResourceSingularizer implements Mapper.ResourceSingularizer.
-func (m *LazyRESTMapper) ResourceSingularizer(resource string) (string, error) {
+func (m *lazyRESTMapper) ResourceSingularizer(resource string) (string, error) {
 	return m.mapper.ResourceSingularizer(resource)
 }
 
 // addKnownGroupAndReload reloads the mapper with updated information about missing API group.
 // versions can be specified for partial updates, for instance for v1beta1 version only.
-func (m *LazyRESTMapper) addKnownGroupAndReload(groupName string, versions ...string) error {
+func (m *lazyRESTMapper) addKnownGroupAndReload(groupName string, versions ...string) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
@@ -209,7 +198,7 @@ func (m *LazyRESTMapper) addKnownGroupAndReload(groupName string, versions ...st
 }
 
 // findAPIGroupByName returns API group by its name.
-func (m *LazyRESTMapper) findAPIGroupByName(groupName string) (metav1.APIGroup, error) {
+func (m *lazyRESTMapper) findAPIGroupByName(groupName string) (metav1.APIGroup, error) {
 	// Ensure that required info about existing API groups is received and stored in the mapper.
 	// It will make 2 API calls to /api and /apis, but only once.
 	if m.apiGroups == nil {
@@ -234,7 +223,7 @@ func (m *LazyRESTMapper) findAPIGroupByName(groupName string) (metav1.APIGroup,
 }
 
 // fetchGroupVersionResources fetches the resources for the specified group and its versions.
-func (m *LazyRESTMapper) fetchGroupVersionResources(groupName string, versions ...string) (map[schema.GroupVersion]*metav1.APIResourceList, error) {
+func (m *lazyRESTMapper) fetchGroupVersionResources(groupName string, versions ...string) (map[schema.GroupVersion]*metav1.APIResourceList, error) {
 	groupVersionResources := make(map[schema.GroupVersion]*metav1.APIResourceList)
 	failedGroups := make(map[schema.GroupVersion]error)
 

diff --git a/pkg/client/apiutil/lazyrestmapper_test.go b/pkg/client/apiutil/lazyrestmapper_test.go
@@ -24,7 +24,6 @@ import (
 	gmg "github.com/onsi/gomega"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
@@ -96,10 +95,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
 		crt := newCountingRoundTripper(httpClient.Transport)
 		httpClient.Transport = crt
 
-		discoveryClient, err := discovery.NewDiscoveryClientForConfigAndClient(restCfg, httpClient)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		lazyRestMapper, err := apiutil.NewLazyRESTMapperWithClient(discoveryClient)
+		lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
 		g.Expect(err).NotTo(gmg.HaveOccurred())
 
 		// There are no requests before any call
@@ -148,10 +144,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
 		crt := newCountingRoundTripper(httpClient.Transport)
 		httpClient.Transport = crt
 
-		discoveryClient, err := discovery.NewDiscoveryClientForConfigAndClient(restCfg, httpClient)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		lazyRestMapper, err := apiutil.NewLazyRESTMapperWithClient(discoveryClient)
+		lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
 		g.Expect(err).NotTo(gmg.HaveOccurred())
 
 		g.Expect(crt.GetRequestCount()).To(gmg.Equal(0))
@@ -188,10 +181,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
 		crt := newCountingRoundTripper(httpClient.Transport)
 		httpClient.Transport = crt
 
-		discoveryClient, err := discovery.NewDiscoveryClientForConfigAndClient(restCfg, httpClient)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		lazyRestMapper, err := apiutil.NewLazyRESTMapperWithClient(discoveryClient)
+		lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
 		g.Expect(err).NotTo(gmg.HaveOccurred())
 
 		g.Expect(crt.GetRequestCount()).To(gmg.Equal(0))
@@ -227,10 +217,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
 		crt := newCountingRoundTripper(httpClient.Transport)
 		httpClient.Transport = crt
 
-		discoveryClient, err := discovery.NewDiscoveryClientForConfigAndClient(restCfg, httpClient)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		lazyRestMapper, err := apiutil.NewLazyRESTMapperWithClient(discoveryClient)
+		lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
 		g.Expect(err).NotTo(gmg.HaveOccurred())
 
 		g.Expect(crt.GetRequestCount()).To(gmg.Equal(0))
@@ -265,10 +252,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
 		crt := newCountingRoundTripper(httpClient.Transport)
 		httpClient.Transport = crt
 
-		discoveryClient, err := discovery.NewDiscoveryClientForConfigAndClient(restCfg, httpClient)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		lazyRestMapper, err := apiutil.NewLazyRESTMapperWithClient(discoveryClient)
+		lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
 		g.Expect(err).NotTo(gmg.HaveOccurred())
 
 		g.Expect(crt.GetRequestCount()).To(gmg.Equal(0))
@@ -312,10 +296,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
 		crt := newCountingRoundTripper(httpClient.Transport)
 		httpClient.Transport = crt
 
-		discoveryClient, err := discovery.NewDiscoveryClientForConfigAndClient(restCfg, httpClient)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		lazyRestMapper, err := apiutil.NewLazyRESTMapperWithClient(discoveryClient)
+		lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
 		g.Expect(err).NotTo(gmg.HaveOccurred())
 
 		_, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "INVALID1"}, "v1")
@@ -354,10 +335,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
 		crt := newCountingRoundTripper(httpClient.Transport)
 		httpClient.Transport = crt
 
-		discoveryClient, err := discovery.NewDiscoveryClientForConfigAndClient(restCfg, httpClient)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		lazyRestMapper, err := apiutil.NewLazyRESTMapperWithClient(discoveryClient)
+		lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
 		g.Expect(err).NotTo(gmg.HaveOccurred())
 
 		_, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "INVALID"})
@@ -396,10 +374,7 @@ func TestLazyRestMapperProvider(t *testing.T) {
 		crt := newCountingRoundTripper(httpClient.Transport)
 		httpClient.Transport = crt
 
-		discoveryClient, err := discovery.NewDiscoveryClientForConfigAndClient(restCfg, httpClient)
-		g.Expect(err).NotTo(gmg.HaveOccurred())
-
-		lazyRestMapper, err := apiutil.NewLazyRESTMapperWithClient(discoveryClient)
+		lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient, apiutil.WithExperimentalLazyMapper)
 		g.Expect(err).NotTo(gmg.HaveOccurred())
 
 		_, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "apps", Kind: "deployment"}, "INVALID")