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

Remove OwnerReferences from CA configmaps #1467

Merged
merged 2 commits into from
Jun 10, 2021
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
16 changes: 0 additions & 16 deletions pkg/config/ca/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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": "",
Expand All @@ -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{
Expand All @@ -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,
}},
},
}
}
Expand Down
34 changes: 33 additions & 1 deletion pkg/controller/jaeger/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)})
jpkrohling marked this conversation as resolved.
Show resolved Hide resolved
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
}
64 changes: 64 additions & 0 deletions pkg/controller/jaeger/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package jaeger

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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)

}
4 changes: 3 additions & 1 deletion pkg/controller/jaeger/jaeger_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down