From 66490eb30b5d0c3f5ba44a28cca59955136b6662 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Mon, 7 Jun 2021 20:47:04 -0500 Subject: [PATCH] Remove OwnerReferences from CA configmaps Signed-off-by: Ruben Vargas --- pkg/config/ca/ca.go | 16 ------ pkg/controller/jaeger/configmap.go | 34 +++++++++++- pkg/controller/jaeger/configmap_test.go | 64 ++++++++++++++++++++++ pkg/controller/jaeger/jaeger_controller.go | 4 +- 4 files changed, 100 insertions(+), 18 deletions(-) diff --git a/pkg/config/ca/ca.go b/pkg/config/ca/ca.go index 7f621b042..66ec46ac3 100644 --- a/pkg/config/ca/ca.go +++ b/pkg/config/ca/ca.go @@ -34,7 +34,6 @@ func GetTrustedCABundle(jaeger *v1.Jaeger) *corev1.ConfigMap { } jaeger.Logger().Debug("CA: Creating the trustedCABundle configmap") - trueVar := true name := TrustedCAName(jaeger) labels := util.Labels(name, "ca-configmap", *jaeger) @@ -50,13 +49,6 @@ func GetTrustedCABundle(jaeger *v1.Jaeger) *corev1.ConfigMap { Name: name, Namespace: jaeger.Namespace, Labels: labels, - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: jaeger.APIVersion, - Kind: jaeger.Kind, - Name: jaeger.Name, - UID: jaeger.UID, - Controller: &trueVar, - }}, }, Data: map[string]string{ "ca-bundle.crt": "", @@ -77,7 +69,6 @@ func GetServiceCABundle(jaeger *v1.Jaeger) *corev1.ConfigMap { } jaeger.Logger().Debug("CA: Creating the service CA configmap") - trueVar := true name := ServiceCAName(jaeger) annotations := map[string]string{ @@ -94,13 +85,6 @@ func GetServiceCABundle(jaeger *v1.Jaeger) *corev1.ConfigMap { Namespace: jaeger.Namespace, Labels: util.Labels(name, "service-ca-configmap", *jaeger), Annotations: annotations, - OwnerReferences: []metav1.OwnerReference{{ - APIVersion: jaeger.APIVersion, - Kind: jaeger.Kind, - Name: jaeger.Name, - UID: jaeger.UID, - Controller: &trueVar, - }}, }, } } diff --git a/pkg/controller/jaeger/configmap.go b/pkg/controller/jaeger/configmap.go index a69ad7f24..0d63d4b38 100644 --- a/pkg/controller/jaeger/configmap.go +++ b/pkg/controller/jaeger/configmap.go @@ -6,11 +6,14 @@ import ( log "github.com/sirupsen/logrus" "go.opentelemetry.io/otel" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" "sigs.k8s.io/controller-runtime/pkg/client" - v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" + "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" "github.com/jaegertracing/jaeger-operator/pkg/inventory" "github.com/jaegertracing/jaeger-operator/pkg/tracing" + "github.com/jaegertracing/jaeger-operator/pkg/util" ) func (r *ReconcileJaeger) applyConfigMaps(ctx context.Context, jaeger v1.Jaeger, desired []corev1.ConfigMap) error { @@ -63,3 +66,32 @@ func (r *ReconcileJaeger) applyConfigMaps(ctx context.Context, jaeger v1.Jaeger, return nil } + +func (r *ReconcileJaeger) getSelectorsForConfigMaps(instanceName string, components []string) labels.Selector { + // We are swallowing the errors here because we are sure that the label requirements are valid + // e.g. "DoubleEqual" has one single value, "In" has at least 1 value, etc... so don't require to check for errors + nameReq, _ := labels.NewRequirement("app.kubernetes.io/name", selection.DoubleEquals, []string{util.Truncate(instanceName, 63)}) + componentReq, _ := labels.NewRequirement("app.kubernetes.io/component", selection.In, components) + managerReq, _ := labels.NewRequirement("app.kubernetes.io/managed-by", selection.Equals, []string{"jaeger-operator"}) + selector := labels.Everything() + return selector.Add(*nameReq, *componentReq, *managerReq) +} + +func (r *ReconcileJaeger) cleanConfigMaps(ctx context.Context, instanceName string) error { + configmaps := corev1.ConfigMapList{} + if err := r.rClient.List(ctx, &configmaps, &client.ListOptions{ + LabelSelector: r.getSelectorsForConfigMaps(instanceName, []string{"ca-configmap", "service-ca-configmap"}), + }); err != nil { + return err + } + + for _, cfgMap := range configmaps.Items { + if err := r.client.Delete(ctx, &cfgMap); err != nil { + log.WithFields(log.Fields{ + "configMapName": cfgMap.Name, + "configMapNamespace": cfgMap.Namespace, + }).WithError(err).Error("error cleaning configmap deployment") + } + } + return nil +} diff --git a/pkg/controller/jaeger/configmap_test.go b/pkg/controller/jaeger/configmap_test.go index 8d153cd20..67cca2ff6 100644 --- a/pkg/controller/jaeger/configmap_test.go +++ b/pkg/controller/jaeger/configmap_test.go @@ -2,6 +2,7 @@ package jaeger import ( "context" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -194,3 +195,66 @@ func TestConfigMapCreateExistingNameInAnotherNamespace(t *testing.T) { assert.Equal(t, nsnExisting.Name, persistedExisting.Name) assert.Equal(t, nsnExisting.Namespace, persistedExisting.Namespace) } + +func TestConfigMapsClean(t *testing.T) { + // prepare + nsnNonExist := types.NamespacedName{ + Name: "deleted-jaeger", + } + + nsnExisting := types.NamespacedName{ + Name: "existing-jaeger", + } + + // Create trusted CA config maps for non existing jaeger + trustedCAConfig := &corev1.ConfigMap{} + trustedCAConfig.Name = fmt.Sprintf("%s-trusted-ca", nsnNonExist.Name) + trustedCAConfig.Labels = map[string]string{ + "app.kubernetes.io/name": nsnNonExist.Name, + "app.kubernetes.io/component": "ca-configmap", + "app.kubernetes.io/managed-by": "jaeger-operator", + } + + serviceCAConfig := &corev1.ConfigMap{} + serviceCAConfig.Name = fmt.Sprintf("%s-service-ca", nsnNonExist.Name) + serviceCAConfig.Labels = map[string]string{ + "app.kubernetes.io/name": nsnNonExist.Name, + "app.kubernetes.io/component": "service-ca-configmap", + "app.kubernetes.io/managed-by": "jaeger-operator", + } + + // Create trusted CA config maps for existing jaeger + serviceCAConfigExist := &corev1.ConfigMap{} + serviceCAConfigExist.Name = fmt.Sprintf("%s-service-ca", nsnExisting.Name) + serviceCAConfigExist.Labels = map[string]string{ + "app.kubernetes.io/name": nsnExisting.Name, + "app.kubernetes.io/component": "service-ca-configmap", + "app.kubernetes.io/managed-by": "jaeger-operator", + } + + objs := []runtime.Object{ + trustedCAConfig, + serviceCAConfig, + serviceCAConfigExist, + v1.NewJaeger(nsnExisting), + } + + r, cl := getReconciler(objs) + + // The three defined ConfigMaps exist + configMaps := &corev1.ConfigMapList{} + err := cl.List(context.Background(), configMaps) + assert.NoError(t, err) + assert.Len(t, configMaps.Items, 3) + + // Reconcile non-exist jaeger + _, err = r.Reconcile(reconcile.Request{NamespacedName: nsnNonExist}) + assert.NoError(t, err) + + // Check that configmaps were clean up. + err = cl.List(context.Background(), configMaps) + assert.NoError(t, err) + assert.Len(t, configMaps.Items, 1) + assert.Equal(t, fmt.Sprintf("%s-service-ca", nsnExisting.Name), configMaps.Items[0].Name) + +} diff --git a/pkg/controller/jaeger/jaeger_controller.go b/pkg/controller/jaeger/jaeger_controller.go index 912292678..9ce66055a 100644 --- a/pkg/controller/jaeger/jaeger_controller.go +++ b/pkg/controller/jaeger/jaeger_controller.go @@ -108,7 +108,9 @@ func (r *ReconcileJaeger) Reconcile(request reconcile.Request) (reconcile.Result // Request object not found, could have been deleted after reconcile request. // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. // Return and don't requeue - span.SetStatus(codes.Error, err.Error()) + if err := r.cleanConfigMaps(ctx, request.Name); err != nil { + return reconcile.Result{}, tracing.HandleError(err, span) + } return reconcile.Result{}, nil } // Error reading the object - requeue the request.