From 1e4b9a6a2773a7af9a026ab3c7c7529fcf591391 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Thu, 27 Jul 2023 18:04:50 +0200 Subject: [PATCH 1/3] Refactor the autodetect module to reduce the number of writes/reads in viper Signed-off-by: Israel Blancas --- pkg/autodetect/main.go | 163 ++++++++++++++++++++---------------- pkg/autodetect/main_test.go | 1 - 2 files changed, 92 insertions(+), 72 deletions(-) diff --git a/pkg/autodetect/main.go b/pkg/autodetect/main.go index 80794ffd7..ec6d30f25 100644 --- a/pkg/autodetect/main.go +++ b/pkg/autodetect/main.go @@ -95,20 +95,22 @@ func (b *Background) autoDetectCapabilities() { b.firstRun.Do(func() { // the platform won't change during the execution of the operator, need to run it only once b.detectPlatform(ctx, apiList) - }) + // the version of the APIs provided by the platform will not change + b.detectCronjobsVersion(ctx) + b.detectAutoscalingVersion(ctx) + }) b.detectElasticsearch(ctx, apiList) b.detectKafka(ctx, apiList) - b.detectCronjobsVersion(ctx) - b.detectAutoscalingVersion(ctx) } - b.detectClusterRoles(ctx) b.cleanDeployments(ctx) } func (b *Background) detectCronjobsVersion(ctx context.Context) { apiGroupVersions := []string{v1.FlagCronJobsVersionBatchV1, v1.FlagCronJobsVersionBatchV1Beta1} + detectedVersion := "" + for _, apiGroupVersion := range apiGroupVersions { groupAPIList, err := b.dcl.ServerResourcesForGroupVersion(apiGroupVersion) if err != nil { @@ -119,20 +121,26 @@ func (b *Background) detectCronjobsVersion(ctx context.Context) { } for _, api := range groupAPIList.APIResources { if api.Name == "cronjobs" { - viper.Set(v1.FlagCronJobsVersion, apiGroupVersion) - log.Log.V(-1).Info(fmt.Sprintf("found the cronjobs api in %s", apiGroupVersion)) - return + detectedVersion = apiGroupVersion + break } } } - log.Log.V(2).Info( - fmt.Sprintf("did not find the cronjobs api in %s", strings.Join(apiGroupVersions, " or ")), - ) + if detectedVersion == "" { + log.Log.V(2).Info( + fmt.Sprintf("did not find the cronjobs api in %s", strings.Join(apiGroupVersions, " or ")), + ) + } else { + viper.Set(v1.FlagCronJobsVersion, detectedVersion) + log.Log.V(-1).Info(fmt.Sprintf("found the cronjobs api in %s", detectedVersion)) + } } func (b *Background) detectAutoscalingVersion(ctx context.Context) { apiGroupVersions := []string{v1.FlagAutoscalingVersionV2, v1.FlagAutoscalingVersionV2Beta2} + detectedVersion := "" + for _, apiGroupVersion := range apiGroupVersions { groupAPIList, err := b.dcl.ServerResourcesForGroupVersion(apiGroupVersion) if err != nil { @@ -143,16 +151,21 @@ func (b *Background) detectAutoscalingVersion(ctx context.Context) { } for _, api := range groupAPIList.APIResources { if api.Name == "horizontalpodautoscalers" { - viper.Set(v1.FlagAutoscalingVersion, apiGroupVersion) - log.Log.V(-1).Info(fmt.Sprintf("found the horizontalpodautoscalers api in %s", apiGroupVersion)) - return + detectedVersion = apiGroupVersion + break } } } - log.Log.V(2).Info( - fmt.Sprintf("did not find the autoscaling api in %s", strings.Join(apiGroupVersions, " or ")), - ) + if detectedVersion == "" { + log.Log.V(2).Info( + fmt.Sprintf("did not find the autoscaling api in %s", strings.Join(apiGroupVersions, " or ")), + ) + } else { + viper.Set(v1.FlagAutoscalingVersion, detectedVersion) + log.Log.V(-1).Info(fmt.Sprintf("found the horizontalpodautoscalers api in %s", detectedVersion)) + } + } // AvailableAPIs returns available list of CRDs from the cluster. @@ -179,78 +192,81 @@ func AvailableAPIs(discovery discovery.DiscoveryInterface, groups map[string]boo func (b *Background) detectPlatform(ctx context.Context, apiList []*metav1.APIResourceList) { // detect the platform, we run this only once, as the platform can't change between runs ;) - if strings.EqualFold(viper.GetString("platform"), v1.FlagPlatformAutoDetect) { - log.Log.V(-1).Info("Attempting to auto-detect the platform") - if isOpenShift(apiList) { - viper.Set("platform", v1.FlagPlatformOpenShift) - } else { - viper.Set("platform", v1.FlagPlatformKubernetes) - } + platform := viper.GetString("platform") + detectedPlatform := "" - log.Log.Info( - "Auto-detected the platform", - "platform", viper.GetString("platform"), - ) - } else { + if !strings.EqualFold(platform, v1.FlagPlatformAutoDetect) { log.Log.V(-1).Info( "The 'platform' option is explicitly set", - "platform", viper.GetString("platform"), + "platform", platform, ) + return } + + log.Log.V(-1).Info("Attempting to auto-detect the platform") + if isOpenShift(apiList) { + detectedPlatform = v1.FlagPlatformOpenShift + } else { + detectedPlatform = v1.FlagPlatformKubernetes + } + + viper.Set("platform", detectedPlatform) + log.Log.Info( + "Auto-detected the platform", + "platform", detectedPlatform, + ) } func (b *Background) detectElasticsearch(ctx context.Context, apiList []*metav1.APIResourceList) { // detect whether the Elasticsearch operator is available - if b.retryDetectEs { + currentESProvision := viper.GetString("es-provision") + if !b.retryDetectEs { log.Log.V(-1).Info( - "Determining whether we should enable the Elasticsearch Operator integration", + "The 'es-provision' option is explicitly set", + "es-provision", currentESProvision, ) - previous := viper.GetString("es-provision") - if IsElasticsearchOperatorAvailable(apiList) { - viper.Set("es-provision", v1.FlagProvisionElasticsearchYes) - } else { - viper.Set("es-provision", v1.FlagProvisionElasticsearchNo) - } + } - if previous != viper.GetString("es-provision") { - log.Log.Info( - "Automatically adjusted the 'es-provision' flag", - "es-provision", viper.GetString("es-provision"), - ) - } - } else { - log.Log.V(-1).Info( - "The 'es-provision' option is explicitly set", - "es-provision", viper.GetString("es-provision"), + log.Log.V(-1).Info("Determining whether we should enable the Elasticsearch Operator integration") + + esProvision := v1.FlagProvisionElasticsearchNo + if IsElasticsearchOperatorAvailable(apiList) { + esProvision = v1.FlagProvisionElasticsearchYes + } + + if currentESProvision != esProvision { + log.Log.Info( + "Automatically adjusted the 'es-provision' flag", + "es-provision", esProvision, ) + viper.Set("es-provision", esProvision) } } // detectKafka checks whether the Kafka Operator is available func (b *Background) detectKafka(_ context.Context, apiList []*metav1.APIResourceList) { - // viper has a "IsSet" method that we could use, except that it returns "true" even - // when nothing is set but it finds a 'Default' value... - if b.retryDetectKafka { - log.Log.V(-1).Info("Determining whether we should enable the Kafka Operator integration") - - previous := viper.GetString("kafka-provision") - if isKafkaOperatorAvailable(apiList) { - viper.Set("kafka-provision", v1.FlagProvisionKafkaYes) - } else { - viper.Set("kafka-provision", v1.FlagProvisionKafkaNo) - } - - if previous != viper.GetString("kafka-provision") { - log.Log.Info( - "Automatically adjusted the 'kafka-provision' flag", - "kafka-provision", viper.GetString("kafka-provision"), - ) - } - } else { + currentKafkaProvision := viper.GetString("kafka-provision") + if !b.retryDetectKafka { log.Log.V(-1).Info( "The 'kafka-provision' option is explicitly set", - "kafka-provision", viper.GetString("kafka-provision"), + "kafka-provision", currentKafkaProvision, ) + return + } + + log.Log.V(-1).Info("Determining whether we should enable the Kafka Operator integration") + + kafkaProvision := v1.FlagProvisionKafkaNo + if isKafkaOperatorAvailable(apiList) { + kafkaProvision = v1.FlagProvisionKafkaYes + } + + if currentKafkaProvision != kafkaProvision { + log.Log.Info( + "Automatically adjusted the 'kafka-provision' flag", + "kafka-provision", kafkaProvision, + ) + viper.Set("kafka-provision", kafkaProvision) } } @@ -264,26 +280,31 @@ func (b *Background) detectClusterRoles(ctx context.Context) { Token: "TEST", }, } + currentAuthDelegator := viper.GetBool("auth-delegator-available") + var newAuthDelegator bool if err := b.cl.Create(ctx, tr); err != nil { - if !viper.IsSet("auth-delegator-available") || (viper.IsSet("auth-delegator-available") && viper.GetBool("auth-delegator-available")) { + if !viper.IsSet("auth-delegator-available") || (viper.IsSet("auth-delegator-available") && currentAuthDelegator) { // for the first run, we log this info, or when the previous value was true log.Log.Info( "The service account running this operator does not have the role 'system:auth-delegator', consider granting it for additional capabilities", ) } - viper.Set("auth-delegator-available", false) + newAuthDelegator = false } else { // this isn't technically correct, as we only ensured that we can create token reviews (which is what the OAuth Proxy does) // but it might be the case that we have *another* cluster role that includes this access and still not have // the "system:auth-delegator". This is an edge case, and it's more complicated to check that, so, we'll keep it simple for now // and deal with the edge case if it ever manifests in the real world - if !viper.IsSet("auth-delegator-available") || (viper.IsSet("auth-delegator-available") && !viper.GetBool("auth-delegator-available")) { + if !viper.IsSet("auth-delegator-available") || (viper.IsSet("auth-delegator-available") && !currentAuthDelegator) { // for the first run, we log this info, or when the previous value was 'false' log.Log.Info( "The service account running this operator has the role 'system:auth-delegator', enabling OAuth Proxy's 'delegate-urls' option", ) } - viper.Set("auth-delegator-available", true) + newAuthDelegator = true + } + if currentAuthDelegator != newAuthDelegator || !viper.IsSet("auth-delegator-available") { + viper.Set("auth-delegator-available", newAuthDelegator) } } diff --git a/pkg/autodetect/main_test.go b/pkg/autodetect/main_test.go index 12f95b5f2..c3ea1752d 100644 --- a/pkg/autodetect/main_test.go +++ b/pkg/autodetect/main_test.go @@ -453,7 +453,6 @@ func TestAutoDetectCronJobsVersion(t *testing.T) { // verify assert.Equal(t, apiGroup, viper.GetString(v1.FlagCronJobsVersion)) - fmt.Printf("Test finished on [%s]\n", apiGroup) } } From 43a070649c6879a6f16e6cefefacbcf0ee74d0f3 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Thu, 27 Jul 2023 18:53:04 +0200 Subject: [PATCH 2/3] Fix linting Signed-off-by: Israel Blancas --- pkg/autodetect/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/autodetect/main.go b/pkg/autodetect/main.go index ec6d30f25..a744ee802 100644 --- a/pkg/autodetect/main.go +++ b/pkg/autodetect/main.go @@ -165,7 +165,6 @@ func (b *Background) detectAutoscalingVersion(ctx context.Context) { viper.Set(v1.FlagAutoscalingVersion, detectedVersion) log.Log.V(-1).Info(fmt.Sprintf("found the horizontalpodautoscalers api in %s", detectedVersion)) } - } // AvailableAPIs returns available list of CRDs from the cluster. From 9bfbb36e459728609a984bfcd521fd8107723972 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Fri, 28 Jul 2023 12:28:42 +0200 Subject: [PATCH 3/3] Move the cleaning tasks outside the autodetection Signed-off-by: Israel Blancas --- pkg/autoclean/main.go | 158 +++++++++++++++++++++++++++++++++ pkg/autoclean/main_test.go | 168 ++++++++++++++++++++++++++++++++++++ pkg/autodetect/main.go | 94 -------------------- pkg/autodetect/main_test.go | 117 ------------------------- pkg/cmd/start/bootstrap.go | 10 +++ 5 files changed, 336 insertions(+), 211 deletions(-) create mode 100644 pkg/autoclean/main.go create mode 100644 pkg/autoclean/main_test.go diff --git a/pkg/autoclean/main.go b/pkg/autoclean/main.go new file mode 100644 index 000000000..c35ce66e9 --- /dev/null +++ b/pkg/autoclean/main.go @@ -0,0 +1,158 @@ +package autoclean + +import ( + "context" + "strings" + "time" + + "github.com/spf13/viper" + + appsv1 "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + "k8s.io/client-go/discovery" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + + v1 "github.com/jaegertracing/jaeger-operator/apis/v1" + "github.com/jaegertracing/jaeger-operator/pkg/inject" +) + +type Background struct { + cl client.Client + clReader client.Reader + dcl discovery.DiscoveryInterface + ticker *time.Ticker +} + +// New creates a new auto-clean runner +func New(mgr manager.Manager) (*Background, error) { + dcl, err := discovery.NewDiscoveryClientForConfig(mgr.GetConfig()) + if err != nil { + return nil, err + } + + 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, clr client.Reader) *Background { + return &Background{ + cl: cl, + dcl: dcl, + clReader: clr, + } +} + +// Start initializes the auto-clean process that runs in the background +func (b *Background) Start() { + b.ticker = time.NewTicker(5 * time.Second) + b.autoClean() + + go func() { + for { + <-b.ticker.C + b.autoClean() + } + }() +} + +// Stop causes the background process to stop auto clean capabilities +func (b *Background) Stop() { + b.ticker.Stop() +} + +func (b *Background) autoClean() { + ctx := context.Background() + b.cleanDeployments(ctx) +} + +func (b *Background) cleanDeployments(ctx context.Context) { + log.Log.V(-1).Info("cleaning orphaned deployments.") + + instancesMap := make(map[string]*v1.Jaeger) + deployments := &appsv1.DeploymentList{} + deployOpts := []client.ListOption{ + matchingLabelKeys(map[string]string{inject.Label: ""}), + } + + // if we are not watching all namespaces, we have to get items from each namespace being watched + if namespaces := viper.GetString(v1.ConfigWatchNamespace); namespaces != v1.WatchAllNamespaces { + for _, ns := range strings.Split(namespaces, ",") { + nsDeps := &appsv1.DeploymentList{} + if err := b.clReader.List(ctx, nsDeps, append(deployOpts, client.InNamespace(ns))...); err != nil { + log.Log.Error( + err, + "error getting a list of deployments to analyze in namespace", + "namespace", ns, + ) + } + deployments.Items = append(deployments.Items, nsDeps.Items...) + + instances := &v1.JaegerList{} + if err := b.clReader.List(ctx, instances, client.InNamespace(ns)); err != nil { + log.Log.Error( + err, + "error getting a list of existing jaeger instances in namespace", + "namespace", ns, + ) + } + for i := range instances.Items { + instancesMap[instances.Items[i].Name] = &instances.Items[i] + } + } + } else { + if err := b.clReader.List(ctx, deployments, deployOpts...); err != nil { + log.Log.Error( + err, + "error getting a list of deployments to analyze", + ) + } + + instances := &v1.JaegerList{} + if err := b.clReader.List(ctx, instances); err != nil { + log.Log.Error( + err, + "error getting a list of existing jaeger instances", + ) + } + for i := range instances.Items { + instancesMap[instances.Items[i].Name] = &instances.Items[i] + } + } + + // check deployments to see which one needs to be cleaned. + for i := range deployments.Items { + dep := deployments.Items[i] + if instanceName, ok := dep.Labels[inject.Label]; ok { + _, instanceExists := instancesMap[instanceName] + if !instanceExists { // Jaeger instance not exist anymore, we need to clean this up. + inject.CleanSidecar(instanceName, &dep) + if err := b.cl.Update(ctx, &dep); err != nil { + log.Log.Error( + err, + "error cleaning orphaned deployment", + "deploymentName", dep.Name, + "deploymentNamespace", dep.Namespace, + ) + } + } + } + } +} + +type matchingLabelKeys map[string]string + +func (m matchingLabelKeys) ApplyToList(opts *client.ListOptions) { + sel := labels.NewSelector() + for k := range map[string]string(m) { + req, err := labels.NewRequirement(k, selection.Exists, []string{}) + if err != nil { + log.Log.Error(err, "failed to build label selector") + return + } + sel.Add(*req) + } + opts.LabelSelector = sel +} diff --git a/pkg/autoclean/main_test.go b/pkg/autoclean/main_test.go new file mode 100644 index 000000000..d7e714795 --- /dev/null +++ b/pkg/autoclean/main_test.go @@ -0,0 +1,168 @@ +package autoclean + +import ( + "context" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/version" + "k8s.io/client-go/discovery" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + v1 "github.com/jaegertracing/jaeger-operator/apis/v1" + "github.com/jaegertracing/jaeger-operator/pkg/inject" +) + +func TestCleanDeployments(t *testing.T) { + for _, tt := range []struct { + cap string // caption for the test + watchNamespace string // the value for WATCH_NAMESPACE + jaegerNamespace string // in which namespace the jaeger exists, empty for non existing + deleted bool // whether the sidecar should have been deleted + }{ + { + cap: "existing-same-namespace", + watchNamespace: "observability", + jaegerNamespace: "observability", + deleted: false, + }, + { + cap: "not-existing-same-namespace", + watchNamespace: "observability", + jaegerNamespace: "", + deleted: true, + }, + { + cap: "existing-watched-namespace", + watchNamespace: "observability,other-observability", + jaegerNamespace: "other-observability", + deleted: false, + }, + { + cap: "existing-non-watched-namespace", + watchNamespace: "observability", + jaegerNamespace: "other-observability", + deleted: true, + }, + { + cap: "existing-watching-all-namespaces", + watchNamespace: v1.WatchAllNamespaces, + jaegerNamespace: "other-observability", + deleted: false, + }, + } { + t.Run(tt.cap, func(t *testing.T) { + // prepare the test data + viper.Set(v1.ConfigWatchNamespace, tt.watchNamespace) + defer viper.Reset() + + jaeger := v1.NewJaeger(types.NamespacedName{ + Name: "my-instance", + Namespace: "observability", // at first, it exists in the same namespace as the deployment + }) + + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mydep", + Namespace: "observability", + Annotations: map[string]string{inject.Annotation: jaeger.Name}, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "C1", + Image: "image1", + }, + }, + }, + }, + }, + } + dep = inject.Sidecar(jaeger, dep) + + // sanity check + require.Equal(t, 2, len(dep.Spec.Template.Spec.Containers)) + + // prepare the list of existing objects + objs := []runtime.Object{dep} + if len(tt.jaegerNamespace) > 0 { + jaeger.Namespace = tt.jaegerNamespace // now, it exists only in this namespace + objs = append(objs, jaeger) + } + + // prepare the client + s := scheme.Scheme + s.AddKnownTypes(v1.GroupVersion, &v1.Jaeger{}) + s.AddKnownTypes(v1.GroupVersion, &v1.JaegerList{}) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + b := WithClients(cl, &fakeDiscoveryClient{}, cl) + + // test + b.cleanDeployments(context.Background()) + + // verify + persisted := &appsv1.Deployment{} + err := cl.Get(context.Background(), types.NamespacedName{ + Namespace: dep.Namespace, + Name: dep.Name, + }, persisted) + require.NoError(t, err) + + // should the sidecar have been deleted? + if tt.deleted { + assert.Equal(t, 1, len(persisted.Spec.Template.Spec.Containers)) + assert.NotContains(t, persisted.Labels, inject.Label) + } else { + assert.Equal(t, 2, len(persisted.Spec.Template.Spec.Containers)) + assert.Contains(t, persisted.Labels, inject.Label) + } + }) + } +} + +type fakeDiscoveryClient struct { + discovery.DiscoveryInterface + ServerGroupsFunc func() (apiGroupList *metav1.APIGroupList, err error) + ServerResourcesForGroupVersionFunc func(groupVersion string) (resources *metav1.APIResourceList, err error) +} + +func (d *fakeDiscoveryClient) ServerGroups() (apiGroupList *metav1.APIGroupList, err error) { + if d.ServerGroupsFunc == nil { + return &metav1.APIGroupList{}, nil + } + return d.ServerGroupsFunc() +} + +func (d *fakeDiscoveryClient) ServerResourcesForGroupVersion(groupVersion string) (resources *metav1.APIResourceList, err error) { + if d.ServerGroupsFunc == nil { + return &metav1.APIResourceList{}, nil + } + return d.ServerResourcesForGroupVersionFunc(groupVersion) +} + +func (d *fakeDiscoveryClient) ServerResources() ([]*metav1.APIResourceList, error) { + return []*metav1.APIResourceList{}, nil +} + +func (d *fakeDiscoveryClient) ServerPreferredResources() ([]*metav1.APIResourceList, error) { + return []*metav1.APIResourceList{}, nil +} + +func (d *fakeDiscoveryClient) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) { + return []*metav1.APIResourceList{}, nil +} + +func (d *fakeDiscoveryClient) ServerVersion() (*version.Info, error) { + return &version.Info{}, nil +} diff --git a/pkg/autodetect/main.go b/pkg/autodetect/main.go index a744ee802..7445de5d5 100644 --- a/pkg/autodetect/main.go +++ b/pkg/autodetect/main.go @@ -8,18 +8,14 @@ import ( "time" "github.com/spf13/viper" - appsv1 "k8s.io/api/apps/v1" authenticationapi "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" "k8s.io/client-go/discovery" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" v1 "github.com/jaegertracing/jaeger-operator/apis/v1" - "github.com/jaegertracing/jaeger-operator/pkg/inject" ) var listenedGroupsMap = map[string]bool{"logging.openshift.io": true, "kafka.strimzi.io": true, "route.openshift.io": true} @@ -104,7 +100,6 @@ func (b *Background) autoDetectCapabilities() { b.detectKafka(ctx, apiList) } b.detectClusterRoles(ctx) - b.cleanDeployments(ctx) } func (b *Background) detectCronjobsVersion(ctx context.Context) { @@ -307,80 +302,6 @@ func (b *Background) detectClusterRoles(ctx context.Context) { } } -func (b *Background) cleanDeployments(ctx context.Context) { - log.Log.V(-1).Info("detecting orphaned deployments.") - - instancesMap := make(map[string]*v1.Jaeger) - deployments := &appsv1.DeploymentList{} - deployOpts := []client.ListOption{ - matchingLabelKeys(map[string]string{inject.Label: ""}), - } - - // if we are not watching all namespaces, we have to get items from each namespace being watched - if namespaces := viper.GetString(v1.ConfigWatchNamespace); namespaces != v1.WatchAllNamespaces { - for _, ns := range strings.Split(namespaces, ",") { - nsDeps := &appsv1.DeploymentList{} - if err := b.clReader.List(ctx, nsDeps, append(deployOpts, client.InNamespace(ns))...); err != nil { - log.Log.Error( - err, - "error getting a list of deployments to analyze in namespace", - "namespace", ns, - ) - } - deployments.Items = append(deployments.Items, nsDeps.Items...) - - instances := &v1.JaegerList{} - if err := b.clReader.List(ctx, instances, client.InNamespace(ns)); err != nil { - log.Log.Error( - err, - "error getting a list of existing jaeger instances in namespace", - "namespace", ns, - ) - } - for i := range instances.Items { - instancesMap[instances.Items[i].Name] = &instances.Items[i] - } - } - } else { - if err := b.clReader.List(ctx, deployments, deployOpts...); err != nil { - log.Log.Error( - err, - "error getting a list of deployments to analyze", - ) - } - - instances := &v1.JaegerList{} - if err := b.clReader.List(ctx, instances); err != nil { - log.Log.Error( - err, - "error getting a list of existing jaeger instances", - ) - } - for i := range instances.Items { - instancesMap[instances.Items[i].Name] = &instances.Items[i] - } - } - - // check deployments to see which one needs to be cleaned. - for i := range deployments.Items { - dep := deployments.Items[i] - if instanceName, ok := dep.Labels[inject.Label]; ok { - _, instanceExists := instancesMap[instanceName] - if !instanceExists { // Jaeger instance not exist anymore, we need to clean this up. - inject.CleanSidecar(instanceName, &dep) - if err := b.cl.Update(ctx, &dep); err != nil { - log.Log.Error( - err, - "error cleaning orphaned deployment", - "deploymentName", dep.Name, - "deploymentNamespace", dep.Namespace, - ) - } - } - } - } -} - func isOpenShift(apiList []*metav1.APIResourceList) bool { for _, r := range apiList { if strings.HasPrefix(r.GroupVersion, "route.openshift.io") { @@ -412,18 +333,3 @@ func isKafkaOperatorAvailable(apiList []*metav1.APIResourceList) bool { } return false } - -type matchingLabelKeys map[string]string - -func (m matchingLabelKeys) ApplyToList(opts *client.ListOptions) { - sel := labels.NewSelector() - for k := range map[string]string(m) { - req, err := labels.NewRequirement(k, selection.Exists, []string{}) - if err != nil { - log.Log.Error(err, "failed to build label selector") - return - } - sel.Add(*req) - } - opts.LabelSelector = sel -} diff --git a/pkg/autodetect/main_test.go b/pkg/autodetect/main_test.go index c3ea1752d..6376877cf 100644 --- a/pkg/autodetect/main_test.go +++ b/pkg/autodetect/main_test.go @@ -9,22 +9,15 @@ import ( openapi_v2 "github.com/google/gnostic/openapiv2" "github.com/spf13/viper" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/version" "k8s.io/client-go/discovery" - "k8s.io/client-go/kubernetes/scheme" restclient "k8s.io/client-go/rest" fakeRest "k8s.io/client-go/rest/fake" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" v1 "github.com/jaegertracing/jaeger-operator/apis/v1" - "github.com/jaegertracing/jaeger-operator/pkg/inject" ) func TestStart(t *testing.T) { @@ -580,116 +573,6 @@ func TestAuthDelegatorBecomesUnavailable(t *testing.T) { assert.False(t, viper.GetBool("auth-delegator-available")) } -func TestCleanDeployments(t *testing.T) { - for _, tt := range []struct { - cap string // caption for the test - watchNamespace string // the value for WATCH_NAMESPACE - jaegerNamespace string // in which namespace the jaeger exists, empty for non existing - deleted bool // whether the sidecar should have been deleted - }{ - { - cap: "existing-same-namespace", - watchNamespace: "observability", - jaegerNamespace: "observability", - deleted: false, - }, - { - cap: "not-existing-same-namespace", - watchNamespace: "observability", - jaegerNamespace: "", - deleted: true, - }, - { - cap: "existing-watched-namespace", - watchNamespace: "observability,other-observability", - jaegerNamespace: "other-observability", - deleted: false, - }, - { - cap: "existing-non-watched-namespace", - watchNamespace: "observability", - jaegerNamespace: "other-observability", - deleted: true, - }, - { - cap: "existing-watching-all-namespaces", - watchNamespace: v1.WatchAllNamespaces, - jaegerNamespace: "other-observability", - deleted: false, - }, - } { - t.Run(tt.cap, func(t *testing.T) { - // prepare the test data - viper.Set(v1.ConfigWatchNamespace, tt.watchNamespace) - defer viper.Reset() - - jaeger := v1.NewJaeger(types.NamespacedName{ - Name: "my-instance", - Namespace: "observability", // at first, it exists in the same namespace as the deployment - }) - - dep := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mydep", - Namespace: "observability", - Annotations: map[string]string{inject.Annotation: jaeger.Name}, - }, - Spec: appsv1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{}, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "C1", - Image: "image1", - }, - }, - }, - }, - }, - } - dep = inject.Sidecar(jaeger, dep) - - // sanity check - require.Equal(t, 2, len(dep.Spec.Template.Spec.Containers)) - - // prepare the list of existing objects - objs := []runtime.Object{dep} - if len(tt.jaegerNamespace) > 0 { - jaeger.Namespace = tt.jaegerNamespace // now, it exists only in this namespace - objs = append(objs, jaeger) - } - - // prepare the client - s := scheme.Scheme - s.AddKnownTypes(v1.GroupVersion, &v1.Jaeger{}) - s.AddKnownTypes(v1.GroupVersion, &v1.JaegerList{}) - cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() - b := WithClients(cl, &fakeDiscoveryClient{}, cl) - - // test - b.cleanDeployments(context.Background()) - - // verify - persisted := &appsv1.Deployment{} - err := cl.Get(context.Background(), types.NamespacedName{ - Namespace: dep.Namespace, - Name: dep.Name, - }, persisted) - require.NoError(t, err) - - // should the sidecar have been deleted? - if tt.deleted { - assert.Equal(t, 1, len(persisted.Spec.Template.Spec.Containers)) - assert.NotContains(t, persisted.Labels, inject.Label) - } else { - assert.Equal(t, 2, len(persisted.Spec.Template.Spec.Containers)) - assert.Contains(t, persisted.Labels, inject.Label) - } - }) - } -} - type fakeClient struct { client.Client CreateFunc func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error diff --git a/pkg/cmd/start/bootstrap.go b/pkg/cmd/start/bootstrap.go index 692f199bb..198a2e2ce 100644 --- a/pkg/cmd/start/bootstrap.go +++ b/pkg/cmd/start/bootstrap.go @@ -42,6 +42,7 @@ import ( appsv1controllers "github.com/jaegertracing/jaeger-operator/controllers/appsv1" esv1controllers "github.com/jaegertracing/jaeger-operator/controllers/elasticsearch" jaegertracingcontrollers "github.com/jaegertracing/jaeger-operator/controllers/jaegertracing" + "github.com/jaegertracing/jaeger-operator/pkg/autoclean" "github.com/jaegertracing/jaeger-operator/pkg/autodetect" kafkav1beta2 "github.com/jaegertracing/jaeger-operator/pkg/kafka/v1beta2" opmetrics "github.com/jaegertracing/jaeger-operator/pkg/metrics" @@ -125,6 +126,15 @@ func bootstrap(ctx context.Context) manager.Manager { d.Start() } + if c, err := autoclean.New(mgr); err != nil { + log.Log.Error( + err, + "failed to start the background process to auto-clean the operator objects", + ) + } else { + c.Start() + } + detectNamespacePermissions(ctx, mgr) performUpgrades(ctx, mgr) setupControllers(ctx, mgr)