Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use APIReader for Get/List resources for autodetect #814

Merged
merged 1 commit into from
Dec 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
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
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
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
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
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
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
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
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