Provide a truly lazy restmapper

Rework WithLazyDiscovery to use DeferredDiscoveryRESTMapper that
will lazily query the provided client for discovery information
to do REST mappings.

Author: Fedosin

pkg/client/apiutil/dynamicrestmapper.go

@@ -18,12 +18,12 @@ package apiutil
 
 import (
 	"sync"
-	"sync/atomic"
 
 	"golang.org/x/time/rate"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/discovery/cached/memory"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/restmapper"
 )
@@ -37,9 +37,6 @@ type dynamicRESTMapper struct {
 	newMapper    func() (meta.RESTMapper, error)
 
 	lazy bool
-	// Used for lazy init.
-	inited  uint32
-	initMtx sync.Mutex
 }
 
 // DynamicRESTMapperOption is a functional option on the dynamicRESTMapper.
@@ -76,30 +73,43 @@ func WithCustomMapper(newMapper func() (meta.RESTMapper, error)) DynamicRESTMapp
 // RESTMapper dynamically discovers resource types at runtime. opts
 // configure the RESTMapper.
 func NewDynamicRESTMapper(cfg *rest.Config, opts ...DynamicRESTMapperOption) (meta.RESTMapper, error) {
-	client, err := discovery.NewDiscoveryClientForConfig(cfg)
-	if err != nil {
-		return nil, err
-	}
 	drm := &dynamicRESTMapper{
 		limiter: rate.NewLimiter(rate.Limit(defaultRefillRate), defaultLimitSize),
-		newMapper: func() (meta.RESTMapper, error) {
-			groupResources, err := restmapper.GetAPIGroupResources(client)
-			if err != nil {
-				return nil, err
-			}
-			return restmapper.NewDiscoveryRESTMapper(groupResources), nil
-		},
 	}
 	for _, opt := range opts {
-		if err = opt(drm); err != nil {
+		if err := opt(drm); err != nil {
 			return nil, err
 		}
 	}
-	if !drm.lazy {
-		if err := drm.setStaticMapper(); err != nil {
+
+	if drm.newMapper == nil {
+		// If custom mapper is not set, we wse NewDeferredDiscoveryRESTMapper for lazy loading
+		// or standard NewDiscoveryRESTMapper for other cases.
+		client, err := discovery.NewDiscoveryClientForConfig(cfg)
+		if err != nil {
 			return nil, err
 		}
+
+		if !drm.lazy {
+			drm.newMapper = func() (meta.RESTMapper, error) {
+				groupResources, err := restmapper.GetAPIGroupResources(client)
+				if err != nil {
+					return nil, err
+				}
+
+				return restmapper.NewDiscoveryRESTMapper(groupResources), nil
+			}
+		} else {
+			drm.newMapper = func() (meta.RESTMapper, error) {
+				return restmapper.NewDeferredDiscoveryRESTMapper(memory.NewMemCacheClient(client)), nil
+			}
+		}
+	}
+
+	if err := drm.setStaticMapper(); err != nil {
+		return nil, err
 	}
+
 	return drm, nil
 }
 
@@ -124,23 +134,6 @@ func (drm *dynamicRESTMapper) setStaticMapper() error {
 	return nil
 }
 
-// init initializes drm only once if drm is lazy.
-func (drm *dynamicRESTMapper) init() (err error) {
-	// skip init if drm is not lazy or has initialized
-	if !drm.lazy || atomic.LoadUint32(&drm.inited) != 0 {
-		return nil
-	}
-
-	drm.initMtx.Lock()
-	defer drm.initMtx.Unlock()
-	if drm.inited == 0 {
-		if err = drm.setStaticMapper(); err == nil {
-			atomic.StoreUint32(&drm.inited, 1)
-		}
-	}
-	return err
-}
-
 // checkAndReload attempts to call the given callback, which is assumed to be dependent
 // on the data in the restmapper.
 //
@@ -198,9 +191,6 @@ func (drm *dynamicRESTMapper) checkAndReload(checkNeedsReload func() error) erro
 // TODO: wrap reload errors on NoKindMatchError with go 1.13 errors.
 
 func (drm *dynamicRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
-	if err := drm.init(); err != nil {
-		return schema.GroupVersionKind{}, err
-	}
 	var gvk schema.GroupVersionKind
 	err := drm.checkAndReload(func() error {
 		var err error
@@ -211,9 +201,6 @@ func (drm *dynamicRESTMapper) KindFor(resource schema.GroupVersionResource) (sch
 }
 
 func (drm *dynamicRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
-	if err := drm.init(); err != nil {
-		return nil, err
-	}
 	var gvks []schema.GroupVersionKind
 	err := drm.checkAndReload(func() error {
 		var err error
@@ -224,10 +211,6 @@ func (drm *dynamicRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]
 }
 
 func (drm *dynamicRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) {
-	if err := drm.init(); err != nil {
-		return schema.GroupVersionResource{}, err
-	}
-
 	var gvr schema.GroupVersionResource
 	err := drm.checkAndReload(func() error {
 		var err error
@@ -238,9 +221,6 @@ func (drm *dynamicRESTMapper) ResourceFor(input schema.GroupVersionResource) (sc
 }
 
 func (drm *dynamicRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) {
-	if err := drm.init(); err != nil {
-		return nil, err
-	}
 	var gvrs []schema.GroupVersionResource
 	err := drm.checkAndReload(func() error {
 		var err error
@@ -251,9 +231,6 @@ func (drm *dynamicRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([
 }
 
 func (drm *dynamicRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
-	if err := drm.init(); err != nil {
-		return nil, err
-	}
 	var mapping *meta.RESTMapping
 	err := drm.checkAndReload(func() error {
 		var err error
@@ -264,9 +241,6 @@ func (drm *dynamicRESTMapper) RESTMapping(gk schema.GroupKind, versions ...strin
 }
 
 func (drm *dynamicRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) {
-	if err := drm.init(); err != nil {
-		return nil, err
-	}
 	var mappings []*meta.RESTMapping
 	err := drm.checkAndReload(func() error {
 		var err error
@@ -277,9 +251,6 @@ func (drm *dynamicRESTMapper) RESTMappings(gk schema.GroupKind, versions ...stri
 }
 
 func (drm *dynamicRESTMapper) ResourceSingularizer(resource string) (string, error) {
-	if err := drm.init(); err != nil {
-		return "", err
-	}
 	var singular string
 	err := drm.checkAndReload(func() error {
 		var err error

pkg/client/apiutil/dynamicrestmapper_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package apiutil
 
 import (
-	"fmt"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -145,27 +144,6 @@ var _ = Describe("Dynamic REST Mapper", func() {
 			By("ensuring that it was only refreshed once")
 			Expect(count).To(Equal(1))
 		})
-
-		It("should lazily initialize if the lazy option is used", func() {
-			var err error
-			var failedOnce bool
-			mockErr := fmt.Errorf("mock failed once")
-			mapper, err = NewDynamicRESTMapper(cfg, WithLazyDiscovery, WithCustomMapper(func() (meta.RESTMapper, error) {
-				// Make newMapper fail once
-				if !failedOnce {
-					failedOnce = true
-					return nil, mockErr
-				}
-				baseMapper := meta.NewDefaultRESTMapper(nil)
-				addToMapper(baseMapper)
-				return baseMapper, nil
-			}))
-			Expect(err).NotTo(HaveOccurred())
-			Expect(mapper.(*dynamicRESTMapper).staticMapper).To(BeNil())
-
-			Expect(callWithTarget()).To(MatchError(mockErr))
-			Expect(callWithTarget()).To(Succeed())
-		})
 	}
 
 	Describe("KindFor", func() {