From 710990a6795b82a1961d11bdcf30b1f3b01b22f8 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Wed, 4 Dec 2019 19:31:43 -0600 Subject: [PATCH] Use APIReader for Get/List resources on the autodetect functions Signed-off-by: Ruben Vargas --- go.mod | 2 +- go.sum | 2 ++ pkg/autodetect/main.go | 21 ++++++++++---------- pkg/autodetect/main_test.go | 38 ++++++++++++++++++------------------- pkg/cmd/start/bootstrap.go | 2 +- pkg/upgrade/upgrade.go | 4 ++-- pkg/upgrade/upgrade_test.go | 8 ++++---- pkg/upgrade/v1_15_0_test.go | 2 +- 8 files changed, 41 insertions(+), 38 deletions(-) diff --git a/go.mod b/go.mod index cb5f53ef6..706ea6c15 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( go.opentelemetry.io/otel/exporter/trace/jaeger v0.1.2 golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f // indirect golang.org/x/net v0.0.0-20191028085509-fe3aa8a45271 - golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d // indirect + golang.org/x/tools v0.0.0-20191205002649-427c522ce2c7 // indirect google.golang.org/grpc v1.24.0 k8s.io/api v0.0.0 k8s.io/apimachinery v0.0.0 diff --git a/go.sum b/go.sum index 0f7d88f28..20f6ca0cd 100644 --- a/go.sum +++ b/go.sum @@ -777,6 +777,8 @@ golang.org/x/tools v0.0.0-20191101200257-8dbcdeb83d3f/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.0.0-20191125144606-a911d9008d1f/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d h1:/iIZNFGxc/a7C3yWjGcnboV+Tkc7mxr+p6fDztwoxuM= golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20191205002649-427c522ce2c7 h1:R+waiss/kC/yzpcyEFoesbEJzgvWtLC4zRdYjBNKf1g= +golang.org/x/tools v0.0.0-20191205002649-427c522ce2c7/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.0.1 h1:xyiBuvkD2g5n7cYzx6u2sxQvsAy4QJsZFCzGVdzOXZ0= diff --git a/pkg/autodetect/main.go b/pkg/autodetect/main.go index c2c7a1cb7..5b62397e6 100644 --- a/pkg/autodetect/main.go +++ b/pkg/autodetect/main.go @@ -22,9 +22,10 @@ import ( // Background represents a procedure that runs in the background, periodically auto-detecting features type Background struct { - cl client.Client - dcl discovery.DiscoveryInterface - ticker *time.Ticker + cl client.Client + clReader client.Reader + dcl discovery.DiscoveryInterface + ticker *time.Ticker retryDetectKafka bool retryDetectEs bool @@ -37,16 +38,16 @@ func New(mgr manager.Manager) (*Background, error) { return nil, err } - return WithClients(mgr.GetClient(), dcl), nil + return WithClients(mgr.GetClient(), dcl, mgr.GetAPIReader()), nil } // WithClients builds a new Background with the provided clients -func WithClients(cl client.Client, dcl discovery.DiscoveryInterface) *Background { +func WithClients(cl client.Client, dcl discovery.DiscoveryInterface, clr client.Reader) *Background { // whether we should keep adjusting depending on the environment retryDetectEs := viper.GetString("es-provision") == v1.FlagProvisionElasticsearchAuto retryDetectKafka := viper.GetString("kafka-provision") == v1.FlagProvisionKafkaAuto - return &Background{cl: cl, dcl: dcl, retryDetectKafka: retryDetectKafka, retryDetectEs: retryDetectEs} + return &Background{cl: cl, dcl: dcl, clReader: clr, retryDetectKafka: retryDetectKafka, retryDetectEs: retryDetectEs} } // Start initializes the auto-detection process that runs in the background @@ -79,7 +80,7 @@ func (b *Background) Stop() { func (b *Background) requireUpdates(deps *appsv1.DeploymentList) []*appsv1.Deployment { instances := &v1.JaegerList{} - if err := b.cl.List(context.Background(), instances); err != nil { + if err := b.clReader.List(context.Background(), instances); err != nil { log.WithError(err).Info("failed to retrieve the list of Jaeger instances") return nil } @@ -122,7 +123,7 @@ func (b *Background) requireUpdates(deps *appsv1.DeploymentList) []*appsv1.Deplo func (b *Background) detectDeploymentUpdates() error { deps := &appsv1.DeploymentList{} - if err := b.cl.List(context.Background(), deps); err != nil { + if err := b.clReader.List(context.Background(), deps); err != nil { return err } injectedDeps := b.requireUpdates(deps) @@ -257,12 +258,12 @@ func (b *Background) cleanDeployments() { instancesMap := make(map[string]*v1.Jaeger) - if err := b.cl.List(context.Background(), deployments, deployOpts...); err != nil { + if err := b.clReader.List(context.Background(), deployments, deployOpts...); err != nil { log.WithError(err).Error("error cleaning orphaned deployment") } // get all jaeger instances - if err := b.cl.List(context.Background(), instances, jaegerOpts...); err != nil { + if err := b.clReader.List(context.Background(), instances, jaegerOpts...); err != nil { log.WithError(err).Error("error cleaning orphaned deployment") } diff --git a/pkg/autodetect/main_test.go b/pkg/autodetect/main_test.go index 43e059c88..21fc4441e 100644 --- a/pkg/autodetect/main_test.go +++ b/pkg/autodetect/main_test.go @@ -36,7 +36,7 @@ func TestStart(t *testing.T) { // prepare dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) done := make(chan bool) go func() { @@ -71,7 +71,7 @@ func TestStartContinuesInBackground(t *testing.T) { cl.CreateFunc = func(ctx context.Context, obj runtime.Object, opts ...client.CreateOption) error { return fmt.Errorf("faked error") } - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) done := make(chan bool) go func() { @@ -123,7 +123,7 @@ func TestAutoDetectFallback(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // sanity check assert.False(t, viper.IsSet("platform")) @@ -149,7 +149,7 @@ func TestAutoDetectOpenShift(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) dcl.ServerGroupsFunc = func() (apiGroupList *metav1.APIGroupList, err error) { return &metav1.APIGroupList{ @@ -173,7 +173,7 @@ func TestAutoDetectKubernetes(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.autoDetectCapabilities() @@ -189,7 +189,7 @@ func TestExplicitPlatform(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.autoDetectCapabilities() @@ -205,7 +205,7 @@ func TestAutoDetectEsProvisionNoEsOperator(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.autoDetectCapabilities() @@ -221,7 +221,7 @@ func TestAutoDetectEsProvisionWithEsOperator(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) dcl.ServerGroupsFunc = func() (apiGroupList *metav1.APIGroupList, err error) { return &metav1.APIGroupList{ @@ -245,7 +245,7 @@ func TestAutoDetectKafkaProvisionNoKafkaOperator(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.autoDetectCapabilities() @@ -261,7 +261,7 @@ func TestAutoDetectKafkaProvisionWithKafkaOperator(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) dcl.ServerGroupsFunc = func() (apiGroupList *metav1.APIGroupList, err error) { return &metav1.APIGroupList{ @@ -285,7 +285,7 @@ func TestAutoDetectKafkaExplicitYes(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.autoDetectCapabilities() @@ -301,7 +301,7 @@ func TestAutoDetectKafkaExplicitNo(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.autoDetectCapabilities() @@ -317,7 +317,7 @@ func TestAutoDetectKafkaDefaultNoOperator(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.autoDetectCapabilities() @@ -333,7 +333,7 @@ func TestAutoDetectKafkaDefaultWithOperator(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := fake.NewFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) dcl.ServerGroupsFunc = func() (apiGroupList *metav1.APIGroupList, err error) { return &metav1.APIGroupList{ @@ -359,7 +359,7 @@ func TestNoAuthDelegatorAvailable(t *testing.T) { cl.CreateFunc = func(ctx context.Context, obj runtime.Object, opts ...client.CreateOption) error { return fmt.Errorf("faked error") } - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.detectClusterRoles() @@ -377,7 +377,7 @@ func TestAuthDelegatorBecomesAvailable(t *testing.T) { cl.CreateFunc = func(ctx context.Context, obj runtime.Object, opts ...client.CreateOption) error { return fmt.Errorf("faked error") } - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.detectClusterRoles() @@ -394,7 +394,7 @@ func TestAuthDelegatorBecomesUnavailable(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := customFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) // test b.detectClusterRoles() @@ -474,7 +474,7 @@ func TestCleanDeployments(t *testing.T) { err = cl.Create(context.TODO(), jaeger2) require.NoError(t, err) - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) b.cleanDeployments() persisted1 := &appsv1.Deployment{} err = cl.Get(context.Background(), types.NamespacedName{ @@ -501,7 +501,7 @@ func TestRequireUpdates(t *testing.T) { dcl := &fakeDiscoveryClient{} cl := customFakeClient() - b := WithClients(cl, dcl) + b := WithClients(cl, dcl, cl) deps := &appsv1.DeploymentList{ Items: []appsv1.Deployment{{ diff --git a/pkg/cmd/start/bootstrap.go b/pkg/cmd/start/bootstrap.go index 4a2eaf3ca..6dcf6a6d1 100644 --- a/pkg/cmd/start/bootstrap.go +++ b/pkg/cmd/start/bootstrap.go @@ -203,7 +203,7 @@ func performUpgrades(ctx context.Context, mgr manager.Manager) { defer span.End() // upgrades all the instances managed by this operator - if err := upgrade.ManagedInstances(ctx, mgr.GetClient()); err != nil { + if err := upgrade.ManagedInstances(ctx, mgr.GetClient(), mgr.GetAPIReader()); err != nil { log.WithError(err).Warn("failed to upgrade managed instances") } } diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go index 1e0202e36..39fa79996 100644 --- a/pkg/upgrade/upgrade.go +++ b/pkg/upgrade/upgrade.go @@ -14,7 +14,7 @@ import ( ) // ManagedInstances finds all the Jaeger instances for the current operator and upgrades them, if necessary -func ManagedInstances(ctx context.Context, c client.Client) error { +func ManagedInstances(ctx context.Context, c client.Client, reader client.Reader) error { tracer := global.TraceProvider().GetTracer(v1.ReconciliationTracer) ctx, span := tracer.Start(ctx, "ManagedInstances") defer span.End() @@ -24,7 +24,7 @@ func ManagedInstances(ctx context.Context, c client.Client) error { opts := client.MatchingLabels(map[string]string{ v1.LabelOperatedBy: identity, }) - if err := c.List(ctx, list, opts); err != nil { + if err := reader.List(ctx, list, opts); err != nil { return tracing.HandleError(err, span) } diff --git a/pkg/upgrade/upgrade_test.go b/pkg/upgrade/upgrade_test.go index 73776d0c8..d5636cd9f 100644 --- a/pkg/upgrade/upgrade_test.go +++ b/pkg/upgrade/upgrade_test.go @@ -28,7 +28,7 @@ func TestVersionUpgradeToLatest(t *testing.T) { cl := fake.NewFakeClient(objs...) // test - assert.NoError(t, ManagedInstances(context.Background(), cl)) + assert.NoError(t, ManagedInstances(context.Background(), cl, cl)) // verify persisted := &v1.Jaeger{} @@ -56,7 +56,7 @@ func TestVersionUpgradeToLatestOwnedResource(t *testing.T) { cl := fake.NewFakeClient(objs...) // test - assert.NoError(t, ManagedInstances(context.Background(), cl)) + assert.NoError(t, ManagedInstances(context.Background(), cl, cl)) // verify persisted := &v1.Jaeger{} @@ -78,7 +78,7 @@ func TestUnknownVersion(t *testing.T) { cl := fake.NewFakeClient(objs...) // test - assert.NoError(t, ManagedInstances(context.Background(), cl)) + assert.NoError(t, ManagedInstances(context.Background(), cl, cl)) // verify persisted := &v1.Jaeger{} @@ -106,7 +106,7 @@ func TestSkipForNonOwnedInstances(t *testing.T) { cl := fake.NewFakeClient(objs...) // test - assert.NoError(t, ManagedInstances(context.Background(), cl)) + assert.NoError(t, ManagedInstances(context.Background(), cl, cl)) // verify persisted := &v1.Jaeger{} diff --git a/pkg/upgrade/v1_15_0_test.go b/pkg/upgrade/v1_15_0_test.go index 366bd2085..6f01c26bd 100644 --- a/pkg/upgrade/v1_15_0_test.go +++ b/pkg/upgrade/v1_15_0_test.go @@ -28,7 +28,7 @@ func TestUpgradeDeprecatedOptions(t *testing.T) { cl := fake.NewFakeClient(objs...) // test - assert.NoError(t, ManagedInstances(context.Background(), cl)) + assert.NoError(t, ManagedInstances(context.Background(), cl, cl)) // verify persisted := &v1.Jaeger{}