From d7c13273ecd9753a82daaf95d6353fd4739aee43 Mon Sep 17 00:00:00 2001 From: Liang Deng <283304489@qq.com> Date: Mon, 16 Oct 2023 22:02:46 +0800 Subject: [PATCH] update restmapper Signed-off-by: Liang Deng <283304489@qq.com> --- pkg/admiralctl/federalize/federalize.go | 4 +- pkg/admiralctl/util/restmapper/restmapper.go | 132 +---------- .../util/restmapper/restmapper_test.go | 213 ------------------ 3 files changed, 13 insertions(+), 336 deletions(-) delete mode 100644 pkg/admiralctl/util/restmapper/restmapper_test.go diff --git a/pkg/admiralctl/federalize/federalize.go b/pkg/admiralctl/federalize/federalize.go index 71c275a22..33ec8a86f 100644 --- a/pkg/admiralctl/federalize/federalize.go +++ b/pkg/admiralctl/federalize/federalize.go @@ -258,12 +258,12 @@ func (o *CommandFederalizeOption) Preflight(f util.Factory, args []string) error o.resourceObj = obj o.gvk = obj.GetObjectKind().GroupVersionKind() - o.mapper, err = restmapper.MapperProvider(o.controlPlaneRestConfig) + o.mapper, err = restmapper.GetRESTMapper(o.controlPlaneRestConfig) if err != nil { return fmt.Errorf("failed to create restmapper: %w", err) } - o.gvr, err = restmapper.GetGroupVersionResource(o.mapper, o.gvk) + o.gvr, err = restmapper.ConvertGVKToGVR(o.mapper, o.gvk) if err != nil { return fmt.Errorf("failed to get gvr from %q: %w", o.gvk, err) } diff --git a/pkg/admiralctl/util/restmapper/restmapper.go b/pkg/admiralctl/util/restmapper/restmapper.go index 1b51901a3..c6a094994 100644 --- a/pkg/admiralctl/util/restmapper/restmapper.go +++ b/pkg/admiralctl/util/restmapper/restmapper.go @@ -17,8 +17,6 @@ limitations under the License. package restmapper import ( - "sync" - "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" @@ -26,133 +24,25 @@ import ( "k8s.io/client-go/restmapper" ) -// GetGroupVersionResource is a helper to map GVK(schema.GroupVersionKind) to GVR(schema.GroupVersionResource). -func GetGroupVersionResource(restMapper meta.RESTMapper, gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) { - restMapping, err := restMapper.RESTMapping(gvk.GroupKind(), gvk.Version) +func GetRESTMapper(restConfig *rest.Config) (meta.RESTMapper, error) { + dc, err := discovery.NewDiscoveryClientForConfig(restConfig) if err != nil { - return schema.GroupVersionResource{}, err - } - return restMapping.Resource, nil -} - -// cachedRESTMapper caches the previous result to accelerate subsequent queries. -type cachedRESTMapper struct { - restMapper meta.RESTMapper - discoveryClient discovery.DiscoveryInterface - gvkToGVR sync.Map - // mu is used to provide thread-safe mapper reloading. - mu sync.RWMutex -} - -func (g *cachedRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) { - return g.getMapper().KindFor(resource) -} - -func (g *cachedRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) { - return g.getMapper().KindsFor(resource) -} - -func (g *cachedRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) { - return g.getMapper().ResourceFor(input) -} - -func (g *cachedRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) { - return g.getMapper().ResourcesFor(input) -} - -func (g *cachedRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) { - return g.getMapper().RESTMappings(gk, versions...) -} - -func (g *cachedRESTMapper) ResourceSingularizer(resource string) (singular string, err error) { - return g.getMapper().ResourceSingularizer(resource) -} - -func (g *cachedRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { - // in case of multi-versions or no versions, cachedRESTMapper don't know which is the preferred version, - // so just bypass the cache and consult the underlying mapper. - if len(versions) != 1 { - return g.getMapper().RESTMapping(gk, versions...) - } - - gvk := gk.WithVersion(versions[0]) - value, ok := g.gvkToGVR.Load(gvk) - if ok { // hit cache, just return - return value.(*meta.RESTMapping), nil - } - - // consult underlying mapper and then update cache - restMapping, err := g.getMapper().RESTMapping(gk, versions...) - if meta.IsNoMatchError(err) { - // hit here means a resource might be missing from the current rest mapper, - // probably because a new resource(CRD) has been added, we have to reload - // resource and rebuild the rest mapper. - - var groupResources []*restmapper.APIGroupResources - groupResources, err = restmapper.GetAPIGroupResources(g.discoveryClient) - if err != nil { - return nil, err - } - - newMapper := restmapper.NewDiscoveryRESTMapper(groupResources) - restMapping, err = newMapper.RESTMapping(gk, versions...) - if err == nil { - // hit here means after reloading, the new rest mapper can recognize - // the resource, we have to replace the mapper and clear cache. - g.mu.Lock() - g.restMapper = newMapper - g.mu.Unlock() - g.gvkToGVR.Range(func(key, value any) bool { - g.gvkToGVR.Delete(key) - return true - }) - } + panic(err.Error()) } + grl, err := restmapper.GetAPIGroupResources(dc) if err != nil { - return restMapping, err + panic(err.Error()) } - g.gvkToGVR.Store(gvk, restMapping) - return restMapping, nil + return restmapper.NewDiscoveryRESTMapper(grl), nil } -func (g *cachedRESTMapper) getMapper() meta.RESTMapper { - g.mu.RLock() - defer g.mu.RUnlock() - return g.restMapper -} - -// NewCachedRESTMapper builds a cachedRESTMapper with a customized underlyingMapper. -// If underlyingMapper is nil, defaults to DiscoveryRESTMapper. -func NewCachedRESTMapper(cfg *rest.Config, underlyingMapper meta.RESTMapper) (meta.RESTMapper, error) { - cachedMapper := cachedRESTMapper{} - - // short path, build with customized underlying mapper. - if underlyingMapper != nil { - cachedMapper.restMapper = underlyingMapper - return &cachedMapper, nil - } - - client, err := discovery.NewDiscoveryClientForConfig(cfg) - if err != nil { - return nil, err - } - - // loading current resources for building a base rest mapper. - groupResources, err := restmapper.GetAPIGroupResources(client) +// ConvertGVKToGVR convert GVK to GVR +func ConvertGVKToGVR(restMapper meta.RESTMapper, gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) { + restMapping, err := restMapper.RESTMapping(gvk.GroupKind(), gvk.Version) if err != nil { - return nil, err + return schema.GroupVersionResource{}, err } - - cachedMapper.restMapper = restmapper.NewDiscoveryRESTMapper(groupResources) - cachedMapper.discoveryClient = client - - return &cachedMapper, nil -} - -// MapperProvider is a wrapper of cachedRESTMapper which is typically used -// to generate customized RESTMapper for controller-runtime framework. -func MapperProvider(c *rest.Config) (meta.RESTMapper, error) { - return NewCachedRESTMapper(c, nil) + return restMapping.Resource, nil } diff --git a/pkg/admiralctl/util/restmapper/restmapper_test.go b/pkg/admiralctl/util/restmapper/restmapper_test.go deleted file mode 100644 index a5482e264..000000000 --- a/pkg/admiralctl/util/restmapper/restmapper_test.go +++ /dev/null @@ -1,213 +0,0 @@ -/* -Copyright 2023 The KubeAdmiral Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package restmapper - -import ( - "testing" - - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - discoveryfake "k8s.io/client-go/discovery/fake" - "k8s.io/client-go/rest" - "k8s.io/client-go/restmapper" - coretesting "k8s.io/client-go/testing" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" -) - -var fakeResources = []*metav1.APIResourceList{ - { - GroupVersion: "apps/v1", - APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}, - }, - { - GroupVersion: "v1", - APIResources: []metav1.APIResource{{Name: "pods", Namespaced: true, Kind: "Pod"}}, - }, - { - GroupVersion: "v2", - APIResources: []metav1.APIResource{{Name: "pods", Namespaced: true, Kind: "Pod"}}, - }, - { - GroupVersion: "extensions/v1beta", - APIResources: []metav1.APIResource{{Name: "jobs", Namespaced: true, Kind: "Job"}}, - }, -} - -// getGVRTestCases organizes the test cases for GetGroupVersionResource. -// It can be shared by both benchmark and unit test. -var getGVRTestCases = []struct { - name string - inputGVK schema.GroupVersionKind - expectedGVR schema.GroupVersionResource - expectErr bool -}{ - { - name: "v1,Pod cache miss", - inputGVK: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, - expectedGVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - expectErr: false, - }, - { - name: "v2,Pod cache miss", - inputGVK: schema.GroupVersionKind{Group: "", Version: "v2", Kind: "Pod"}, - expectedGVR: schema.GroupVersionResource{Group: "", Version: "v2", Resource: "pods"}, - expectErr: false, - }, - { - name: "extensions/v1beta,Job cache miss", - inputGVK: schema.GroupVersionKind{Group: "extensions", Version: "v1beta", Kind: "Job"}, - expectedGVR: schema.GroupVersionResource{Group: "extensions", Version: "v1beta", Resource: "jobs"}, - expectErr: false, - }, - { - name: "v1,Pod cache hit once", - inputGVK: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, - expectedGVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - expectErr: false, - }, - { - name: "v1,Pod cache hit twice", - inputGVK: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, - expectedGVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - expectErr: false, - }, - { - name: "v2,Pod cache hit once", - inputGVK: schema.GroupVersionKind{Group: "", Version: "v2", Kind: "Pod"}, - expectedGVR: schema.GroupVersionResource{Group: "", Version: "v2", Resource: "pods"}, - expectErr: false, - }, - { - name: "v2,Pod cache hit twice", - inputGVK: schema.GroupVersionKind{Group: "", Version: "v2", Kind: "Pod"}, - expectedGVR: schema.GroupVersionResource{Group: "", Version: "v2", Resource: "pods"}, - expectErr: false, - }, - { - name: "extensions/v1beta,Job cache hit once", - inputGVK: schema.GroupVersionKind{Group: "extensions", Version: "v1beta", Kind: "Job"}, - expectedGVR: schema.GroupVersionResource{Group: "extensions", Version: "v1beta", Resource: "jobs"}, - expectErr: false, - }, - { - name: "extensions/v1beta,Job cache hit twice", - inputGVK: schema.GroupVersionKind{Group: "extensions", Version: "v1beta", Kind: "Job"}, - expectedGVR: schema.GroupVersionResource{Group: "extensions", Version: "v1beta", Resource: "jobs"}, - expectErr: false, - }, - { - name: "cache miss and invalidate the cache", - inputGVK: schema.GroupVersionKind{Group: "non-existence", Version: "non-existence", Kind: "non-existence"}, - expectErr: true, - }, -} - -var discoveryClient = &discoveryfake.FakeDiscovery{Fake: &coretesting.Fake{Resources: fakeResources}} - -func BenchmarkGetGroupVersionResource(b *testing.B) { - option := apiutil.WithCustomMapper(func() (meta.RESTMapper, error) { - groupResources, err := restmapper.GetAPIGroupResources(discoveryClient) - if err != nil { - return nil, err - } - return restmapper.NewDiscoveryRESTMapper(groupResources), nil - }) - - mapper, err := apiutil.NewDynamicRESTMapper(&rest.Config{}, option) - if err != nil { - b.Error(err) - } - - b.ResetTimer() - for i := 0; i < b.N; i++ { - for _, tc := range getGVRTestCases { - _, err := GetGroupVersionResource(mapper, tc.inputGVK) - if (err != nil && !tc.expectErr) || (err == nil && tc.expectErr) { - b.Errorf("GetGroupVersionResource For %#v Error: %v, wantErr: %v", tc.inputGVK, err, tc.expectErr) - } - } - } -} - -func BenchmarkGetGroupVersionResourceWithoutCache(b *testing.B) { - groupResources, err := restmapper.GetAPIGroupResources(discoveryClient) - if err != nil { - b.Fatalf("Failed to load resources: %v", err) - } - - mapper := restmapper.NewDiscoveryRESTMapper(groupResources) - - b.ResetTimer() - for i := 0; i < b.N; i++ { - for _, tc := range getGVRTestCases { - _, err := GetGroupVersionResource(mapper, tc.inputGVK) - if (err != nil && !tc.expectErr) || (err == nil && tc.expectErr) { - b.Errorf("GetGroupVersionResource For %#v Error: %v, wantErr: %v", tc.inputGVK, err, tc.expectErr) - } - } - } -} - -func BenchmarkGetGroupVersionResourceWithCache(b *testing.B) { - cachedMapper := &cachedRESTMapper{} - - groupResources, err := restmapper.GetAPIGroupResources(discoveryClient) - if err != nil { - b.Fatalf("Failed to load resources: %v", err) - } - - newMapper := restmapper.NewDiscoveryRESTMapper(groupResources) - cachedMapper.restMapper = newMapper - cachedMapper.discoveryClient = discoveryClient - - b.ResetTimer() - for i := 0; i < b.N; i++ { - for _, tc := range getGVRTestCases { - _, err := GetGroupVersionResource(cachedMapper, tc.inputGVK) - if (err != nil && !tc.expectErr) || (err == nil && tc.expectErr) { - b.Errorf("GetGroupVersionResource For %#v Error: %v, wantErr: %v", tc.inputGVK, err, tc.expectErr) - } - } - } -} - -func TestGetGroupVersionResourceWithCache(t *testing.T) { - cachedMapper := &cachedRESTMapper{} - - groupResources, err := restmapper.GetAPIGroupResources(discoveryClient) - if err != nil { - t.Fatalf("Failed to load resources: %v", err) - } - - newMapper := restmapper.NewDiscoveryRESTMapper(groupResources) - cachedMapper.restMapper = newMapper - cachedMapper.discoveryClient = discoveryClient - - for _, tc := range getGVRTestCases { - t.Run(tc.name, func(t *testing.T) { - got, err := GetGroupVersionResource(cachedMapper, tc.inputGVK) - if (err != nil && !tc.expectErr) || (err == nil && tc.expectErr) { - t.Fatalf("GetGroupVersionResource (%#v) error: %v, wantErr: %v", tc.inputGVK, err, tc.expectErr) - } - - if got != tc.expectedGVR { - t.Fatalf("GetGroupVersionResource(%#v) = %#v, want: %#v", tc.inputGVK, got, tc.expectedGVR) - } - }) - } -}