Skip to content

Commit

Permalink
Use APIReader for Get/List resources on the autodetect functions (#814)
Browse files Browse the repository at this point in the history 
Signed-off-by: Ruben Vargas <[email protected]>
  • Loading branch information
@rubenvp8510 @jpkrohling
rubenvp8510 authored and jpkrohling committed Dec 5, 2019
1 parent 6b2327e commit 61a627b
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 38 deletions.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
21 changes: 11 additions & 10 deletions pkg/autodetect/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
}

Expand Down
38 changes: 19 additions & 19 deletions pkg/autodetect/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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"))
Expand All @@ -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{
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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{
Expand All @@ -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()
Expand All @@ -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{
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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{
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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{
Expand All @@ -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{{
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/start/bootstrap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/upgrade/upgrade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/upgrade/upgrade_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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{}
Expand All @@ -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{}
Expand Down Expand Up @@ -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{}
Expand Down
2 changes: 1 addition & 1 deletion pkg/upgrade/v1_15_0_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down

0 comments on commit 61a627b

Please sign in to comment.