Skip to content

Commit

Permalink
Use aggregated discovery API to discovery API groups and resources
Browse files Browse the repository at this point in the history
Use aggregated discovery API to discovery API groups and resources

Fixes #7526

Signed-off-by: Wenkai Yin(尹文开) <[email protected]>
  • Loading branch information
ywk253100 committed Oct 28, 2024
1 parent 8320df4 commit 7738f5a
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 26 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8352-ywk253100
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use aggregated discovery API to discovery API groups and resources
12 changes: 12 additions & 0 deletions pkg/client/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
"k8s.io/client-go/discovery"
k8scheme "k8s.io/client-go/kubernetes/scheme"
kbclient "sigs.k8s.io/controller-runtime/pkg/client"

Expand Down Expand Up @@ -56,6 +57,9 @@ type Factory interface {
// It adds Kubernetes and Velero types to its scheme. It uses the following priority to specify the cluster
// configuration: --kubeconfig flag, KUBECONFIG environment variable, in-cluster configuration.
KubebuilderWatchClient() (kbclient.WithWatch, error)
// DiscoveryClient returns a Kubernetes discovery client. It uses the following priority to specify the cluster
// configuration: --kubeconfig flag, KUBECONFIG environment variable, in-cluster configuration.
DiscoveryClient() (discovery.AggregatedDiscoveryInterface, error)
// SetBasename changes the basename for an already-constructed client.
// This is useful for generating clients that require a different user-agent string below the root `velero`
// command, such as the server subcommand.
Expand Down Expand Up @@ -209,6 +213,14 @@ func (f *factory) KubebuilderWatchClient() (kbclient.WithWatch, error) {
return kubebuilderWatchClient, nil
}

func (f *factory) DiscoveryClient() (discovery.AggregatedDiscoveryInterface, error) {
clientConfig, err := f.ClientConfig()
if err != nil {
return nil, err
}
return discovery.NewDiscoveryClientForConfig(clientConfig)
}

func (f *factory) SetBasename(name string) {
f.baseName = name
}
Expand Down
30 changes: 29 additions & 1 deletion pkg/client/mocks/Factory.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 14 additions & 4 deletions pkg/cmd/server/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,12 @@ func newServiceAccountBackupItemAction(f client.Factory) plugincommon.HandlerIni
return nil, err
}

discoveryHelper, err := velerodiscovery.NewHelper(clientset.Discovery(), logger)
discoveryClient, err := f.DiscoveryClient()
if err != nil {
return nil, err
}

discoveryHelper, err := velerodiscovery.NewHelper(discoveryClient, logger)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -253,11 +258,11 @@ func newRemapCRDVersionAction(f client.Factory) plugincommon.HandlerInitializer
return nil, err
}

clientset, err := f.KubeClient()
discoveryClient, err := f.DiscoveryClient()
if err != nil {
return nil, err
}
discoveryHelper, err := velerodiscovery.NewHelper(clientset.Discovery(), logger)
discoveryHelper, err := velerodiscovery.NewHelper(discoveryClient, logger)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -470,7 +475,12 @@ func newServiceAccountItemBlockAction(f client.Factory) plugincommon.HandlerInit
return nil, err
}

discoveryHelper, err := velerodiscovery.NewHelper(clientset.Discovery(), logger)
discoveryClient, err := f.DiscoveryClient()
if err != nil {
return nil, err
}

discoveryHelper, err := velerodiscovery.NewHelper(discoveryClient, logger)
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ type server struct {
metricsAddress string
kubeClientConfig *rest.Config
kubeClient kubernetes.Interface
discoveryClient discovery.DiscoveryInterface
discoveryClient discovery.AggregatedDiscoveryInterface
discoveryHelper velerodiscovery.Helper
dynamicClient dynamic.Interface
// controller-runtime client. the difference from the controller-manager's client
Expand Down Expand Up @@ -269,8 +269,8 @@ func newServer(f client.Factory, config *config.Config, logger *logrus.Logger) (
return nil, err
}

var discoveryClient *discovery.DiscoveryClient
if discoveryClient, err = discovery.NewDiscoveryClientForConfig(clientConfig); err != nil {
var discoveryClient discovery.AggregatedDiscoveryInterface
if discoveryClient, err = f.DiscoveryClient(); err != nil {
cancelFunc()
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/discovery/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ type serverResourcesInterface interface {
}

type helper struct {
discoveryClient discovery.DiscoveryInterface
discoveryClient discovery.AggregatedDiscoveryInterface
logger logrus.FieldLogger

// lock guards mapper, resources and resourcesMap
Expand All @@ -89,7 +89,7 @@ type helper struct {

var _ Helper = &helper{}

func NewHelper(discoveryClient discovery.DiscoveryInterface, logger logrus.FieldLogger) (Helper, error) {
func NewHelper(discoveryClient discovery.AggregatedDiscoveryInterface, logger logrus.FieldLogger) (Helper, error) {
h := &helper{
discoveryClient: discoveryClient,
logger: logger,
Expand Down
39 changes: 23 additions & 16 deletions pkg/discovery/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,15 @@ func TestHelper_ResourceFor(t *testing.T) {
},
},
}

h := &helper{
discoveryClient: fakeDiscoveryClient,
lock: sync.RWMutex{},
mapper: nil,
resources: fakeDiscoveryClient.Resources,
resourcesMap: make(map[schema.GroupVersionResource]metav1.APIResource),
serverVersion: &version.Info{Major: "1", Minor: "22", GitVersion: "v1.22.1"},
discoveryClient: &velerotest.DiscoveryClient{
FakeDiscovery: fakeDiscoveryClient,
},
lock: sync.RWMutex{},
mapper: nil,
resources: fakeDiscoveryClient.Resources,
resourcesMap: make(map[schema.GroupVersionResource]metav1.APIResource),
serverVersion: &version.Info{Major: "1", Minor: "22", GitVersion: "v1.22.1"},
}

for _, resourceList := range h.resources {
Expand Down Expand Up @@ -352,11 +353,13 @@ func TestHelper_KindFor(t *testing.T) {
}

h := &helper{
discoveryClient: fakeDiscoveryClient,
lock: sync.RWMutex{},
resources: fakeDiscoveryClient.Resources,
resourcesMap: make(map[schema.GroupVersionResource]metav1.APIResource),
serverVersion: &version.Info{Major: "1", Minor: "22", GitVersion: "v1.22.1"},
discoveryClient: &velerotest.DiscoveryClient{
FakeDiscovery: fakeDiscoveryClient,
},
lock: sync.RWMutex{},
resources: fakeDiscoveryClient.Resources,
resourcesMap: make(map[schema.GroupVersionResource]metav1.APIResource),
serverVersion: &version.Info{Major: "1", Minor: "22", GitVersion: "v1.22.1"},
}

h.kindMap = map[schema.GroupVersionKind]metav1.APIResource{pvGVK: pvAPIRes}
Expand Down Expand Up @@ -517,9 +520,11 @@ func TestHelper_Refresh(t *testing.T) {
for _, testCase := range testCases {
t.Run(testCase.description, func(t *testing.T) {
h := &helper{
lock: sync.RWMutex{},
discoveryClient: fakeDiscoveryClient,
logger: logrus.New(),
lock: sync.RWMutex{},
discoveryClient: &velerotest.DiscoveryClient{
FakeDiscovery: fakeDiscoveryClient,
},
logger: logrus.New(),
}
// Set feature flags
if testCase.features != "" {
Expand Down Expand Up @@ -637,7 +642,9 @@ func TestHelper(t *testing.T) {
fakeDiscoveryClient := &fake.FakeDiscovery{
Fake: &clientgotesting.Fake{},
}
h, err := NewHelper(fakeDiscoveryClient, logrus.New())
h, err := NewHelper(&velerotest.DiscoveryClient{
FakeDiscovery: fakeDiscoveryClient,
}, logrus.New())
assert.NoError(t, err)
// All below calls put together for the implementation are empty or just very simple, and just want to cover testing
// If wanting to write unit tests for some functions could remove it and with writing new function alone
Expand Down
9 changes: 9 additions & 0 deletions pkg/test/discovery_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/discovery"
discoveryfake "k8s.io/client-go/discovery/fake"
)
Expand Down Expand Up @@ -80,3 +81,11 @@ func (c *DiscoveryClient) WithAPIResource(resource *APIResource) *DiscoveryClien

return c
}

func (c *DiscoveryClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) {
apiGroupList, err := c.ServerGroups()
if err != nil {
return nil, nil, nil, err
}
return apiGroupList, nil, nil, nil
}

0 comments on commit 7738f5a

Please sign in to comment.