Create missing CA config maps on deployment controller (#1347)

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
 	github.com/go-openapi/spec v0.19.8
 	github.com/googleapis/gnostic v0.3.1
+	github.com/kr/pretty v0.2.0 // indirect
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/openshift/api v0.0.0-20200701144905-de5b010b2b38
 	github.com/opentracing/opentracing-go v1.1.1-0.20190913142402-a7454ce5950e

go.sum

@@ -646,6 +646,8 @@ github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxv
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
 github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
+github.com/kr/pretty v0.2.0 h1:s5hAObm+yFO5uHYt5dYjxi2rXrsnmRpJx4OYvIWUaQs=
+github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA=
 github.com/kr/pty v1.1.8/go.mod h1:O1sed60cT9XZ5uDucP5qwvh+TE3NnUj51EiZO/lmSfw=

pkg/controller/deployment/deployment_controller.go

@@ -81,13 +81,14 @@ func (r *ReconcileDeployment) Reconcile(request reconcile.Request) (reconcile.Re
 
 	tracer := global.TraceProvider().GetTracer(v1.ReconciliationTracer)
 	ctx, span := tracer.Start(ctx, "reconcileDeployment")
+	span.SetAttributes(key.String("name", request.Name), key.String("namespace", request.Namespace))
 	defer span.End()
 
-	span.SetAttributes(key.String("name", request.Name), key.String("namespace", request.Namespace))
-	log.WithFields(log.Fields{
+	logger := log.WithFields(log.Fields{
 		"namespace": request.Namespace,
 		"name":      request.Name,
-	}).Trace("Reconciling Deployment")
+	})
+	logger.Debug("Reconciling Deployment")
 
 	// Fetch the Deployment instance
 	dep := &appsv1.Deployment{}
@@ -108,74 +109,61 @@ func (r *ReconcileDeployment) Reconcile(request reconcile.Request) (reconcile.Re
 	err = r.rClient.Get(ctx, types.NamespacedName{Name: request.Namespace}, ns)
 	// we shouldn't fail if the namespace object can't be obtained
 	if err != nil {
-		log.WithField("namespace", request.Namespace).WithError(err).Trace("failed to get the namespace for the deployment, skipping injection based on namespace annotation")
-		tracing.HandleError(err, span)
+		msg := "failed to get the namespace for the deployment, skipping injection based on namespace annotation"
+		logger.WithError(err).Debug(msg)
+		span.AddEvent(ctx, msg, key.String("error", err.Error()))
 	}
 
-	if inject.Needed(dep, ns) {
-		jaegers := &v1.JaegerList{}
-		opts := []client.ListOption{}
+	if !inject.Desired(dep, ns) {
+		// sidecar isn't desired for this deployment, skip remaining of the reconciliation
+		return reconcile.Result{}, nil
+	}
 
-		if viper.GetString(v1.ConfigOperatorScope) == v1.OperatorScopeNamespace {
-			opts = append(opts, client.InNamespace(viper.GetString(v1.ConfigWatchNamespace)))
-		}
+	jaegers := &v1.JaegerList{}
+	opts := []client.ListOption{}
 
-		if err := r.rClient.List(ctx, jaegers, opts...); err != nil {
-			log.WithError(err).Error("failed to get the available Jaeger pods")
-			return reconcile.Result{}, tracing.HandleError(err, span)
+	if viper.GetString(v1.ConfigOperatorScope) == v1.OperatorScopeNamespace {
+		opts = append(opts, client.InNamespace(viper.GetString(v1.ConfigWatchNamespace)))
+	}
+
+	if err := r.rClient.List(ctx, jaegers, opts...); err != nil {
+		logger.WithError(err).Error("failed to get the available Jaeger pods")
+		return reconcile.Result{}, tracing.HandleError(err, span)
+	}
+
+	jaeger := inject.Select(dep, ns, jaegers)
+	if jaeger != nil && jaeger.GetDeletionTimestamp() == nil {
+		logger := logger.WithFields(log.Fields{
+			"jaeger":           jaeger.Name,
+			"jaeger-namespace": jaeger.Namespace,
+		})
+		if jaeger.Namespace != dep.Namespace {
+			if err := r.reconcileConfigMaps(ctx, jaeger, dep); err != nil {
+				msg := "failed to reconcile config maps for the namespace"
+				logger.WithError(err).Error(msg)
+				span.AddEvent(ctx, msg)
+			}
 		}
 
-		jaeger := inject.Select(dep, ns, jaegers)
-		if jaeger != nil && jaeger.GetDeletionTimestamp() == nil {
-			if jaeger.Namespace != request.Namespace {
-				log.WithFields(log.Fields{
-					"jaeger-namespace": jaeger.Namespace,
-					"app-namespace":    request.Namespace,
-				}).Debug("different namespaces, so check whether trusted CA bundle configmap should be created")
-				if cm := ca.GetTrustedCABundle(jaeger); cm != nil {
-					// Update the namespace to be the same as the Deployment being injected
-					cm.Namespace = request.Namespace
-					jaeger.Logger().WithFields(log.Fields{
-						"configMap": cm.Name,
-						"namespace": cm.Namespace,
-					}).Debug("creating Trusted CA bundle config maps")
-					if err := r.client.Create(ctx, cm); err != nil && !errors.IsAlreadyExists(err) {
-						log.WithField("namespace", request.Namespace).WithError(err).Error("failed to create trusted CA bundle")
-						return reconcile.Result{}, tracing.HandleError(err, span)
-					}
-				}
-
-				if cm := ca.GetServiceCABundle(jaeger); cm != nil {
-					// Update the namespace to be the same as the Deployment being injected
-					cm.Namespace = request.Namespace
-					jaeger.Logger().WithFields(log.Fields{
-						"configMap": cm.Name,
-						"namespace": cm.Namespace,
-					}).Debug("creating service CA config map")
-					if err := r.client.Create(ctx, cm); err != nil && !errors.IsAlreadyExists(err) {
-						log.WithField("namespace", request.Namespace).WithError(err).Error("failed to create trusted CA bundle")
-						return reconcile.Result{}, tracing.HandleError(err, span)
-					}
-				}
+		// a suitable jaeger instance was found! let's inject a sidecar pointing to it then
+		// Verified that jaeger instance was found and is not marked for deletion.
+		if inject.Needed(dep, ns) {
+			{
+				msg := "injecting Jaeger Agent sidecar"
+				logger.Info(msg)
+				span.AddEvent(ctx, msg)
 			}
 
-			patch := client.MergeFrom(dep.DeepCopy())
-			// a suitable jaeger instance was found! let's inject a sidecar pointing to it then
-			// Verified that jaeger instance was found and is not marked for deletion.
-			log.WithFields(log.Fields{
-				"deployment":       dep.Name,
-				"namespace":        dep.Namespace,
-				"jaeger":           jaeger.Name,
-				"jaeger-namespace": jaeger.Namespace,
-			}).Info("Injecting Jaeger Agent sidecar")
-			injectedDep := inject.Sidecar(jaeger, dep)
-			if err := r.client.Patch(ctx, injectedDep, patch); err != nil {
-				log.WithField("deployment", injectedDep).WithError(err).Error("failed to update")
+			dep = inject.Sidecar(jaeger, dep)
+			if err := r.client.Update(ctx, dep); err != nil {
+				logger.WithError(err).Error("failed to update deployment with sidecar")
 				return reconcile.Result{}, tracing.HandleError(err, span)
 			}
-		} else {
-			log.WithField("deployment", dep.Name).Info("No suitable Jaeger instances found to inject a sidecar")
 		}
+	} else {
+		msg := "no suitable Jaeger instances found to inject a sidecar"
+		span.AddEvent(ctx, msg)
+		logger.Debug(msg)
 	}
 
 	return reconcile.Result{}, nil
@@ -231,3 +219,46 @@ func (r *ReconcileDeployment) syncOnJaegerChanges(event handler.MapObject) []rec
 	}
 	return reconciliations
 }
+
+func (r *ReconcileDeployment) reconcileConfigMaps(ctx context.Context, jaeger *v1.Jaeger, dep *appsv1.Deployment) error {
+	tracer := global.TraceProvider().GetTracer(v1.ReconciliationTracer)
+	ctx, span := tracer.Start(ctx, "reconcileConfigMaps")
+	defer span.End()
+
+	cms := []*corev1.ConfigMap{}
+	if cm := ca.GetTrustedCABundle(jaeger); cm != nil {
+		cms = append(cms, cm)
+	}
+	if cm := ca.GetServiceCABundle(jaeger); cm != nil {
+		cms = append(cms, cm)
+	}
+
+	for _, cm := range cms {
+		if err := r.reconcileConfigMap(ctx, cm, dep); err != nil {
+			return tracing.HandleError(err, span)
+		}
+	}
+
+	return nil
+}
+
+func (r *ReconcileDeployment) reconcileConfigMap(ctx context.Context, cm *corev1.ConfigMap, dep *appsv1.Deployment) error {
+	tracer := global.TraceProvider().GetTracer(v1.ReconciliationTracer)
+	ctx, span := tracer.Start(ctx, "reconcileConfigMap")
+	defer span.End()
+
+	// Update the namespace to be the same as the Deployment being injected
+	cm.Namespace = dep.Namespace
+	span.SetAttribute(key.String("name", cm.Name))
+	span.SetAttribute(key.String("namespace", cm.Namespace))
+
+	if err := r.client.Create(ctx, cm); err != nil {
+		if errors.IsAlreadyExists(err) {
+			span.AddEvent(ctx, "config map exists already")
+		} else {
+			return tracing.HandleError(err, span)
+		}
+	}
+
+	return nil
+}

pkg/controller/deployment/deployment_controller_test.go

@@ -1,12 +1,13 @@
 package deployment
 
 import (
+	"context"
 	"sort"
 	"testing"
 
-	v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
-	"github.com/jaegertracing/jaeger-operator/pkg/inject"
+	"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"
@@ -16,6 +17,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
+	"github.com/jaegertracing/jaeger-operator/pkg/inject"
 )
 
 func TestSyncOnJaegerChanges(t *testing.T) {
@@ -134,3 +138,69 @@ func TestSyncOnJaegerChanges(t *testing.T) {
 
 	assert.Equal(t, expected, requests)
 }
+
+func TestReconcileConfigMaps(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		existing []runtime.Object
+	}{
+		{
+			desc: "all config maps missing",
+		},
+		{
+			desc: "none missing",
+			existing: []runtime.Object{
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "ns1",
+						Name:      "my-instance-trusted-ca",
+					},
+				},
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "ns1",
+						Name:      "my-instance-service-ca",
+					},
+				},
+			},
+		},
+	}
+	for _, tC := range testCases {
+		t.Run(tC.desc, func(t *testing.T) {
+			// prepare
+			jaeger := v1.NewJaeger(types.NamespacedName{
+				Namespace: "observability",
+				Name:      "my-instance",
+			})
+			dep := appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "ns1",
+					Name:      "my-dep",
+				},
+			}
+
+			s := scheme.Scheme
+			cl := fake.NewFakeClient(tC.existing...)
+			r := &ReconcileDeployment{
+				client:  cl,
+				rClient: cl,
+				scheme:  s,
+			}
+
+			viper.Set("platform", v1.FlagPlatformOpenShift)
+			defer viper.Reset()
+
+			// test
+			err := r.reconcileConfigMaps(context.Background(), jaeger, &dep)
+
+			// verify
+			assert.NoError(t, err)
+
+			cms := corev1.ConfigMapList{}
+			err = cl.List(context.Background(), &cms)
+			require.NoError(t, err)
+
+			assert.Len(t, cms.Items, 2)
+		})
+	}
+}

pkg/inject/sidecar.go

@@ -76,17 +76,34 @@ func Sidecar(jaeger *v1.Jaeger, dep *appsv1.Deployment) *appsv1.Deployment {
 	return dep
 }
 
-// Needed determines whether a pod needs to get a sidecar injected or not
-func Needed(dep *appsv1.Deployment, ns *corev1.Namespace) bool {
+// Desired determines whether a sidecar is desired, based on the annotation from both the deployment and the namespace
+func Desired(dep *appsv1.Deployment, ns *corev1.Namespace) bool {
+	logger := log.WithFields(log.Fields{
+		"namespace":  dep.Namespace,
+		"deployment": dep.Name,
+	})
 	_, depExist := dep.Annotations[Annotation]
 	_, nsExist := ns.Annotations[Annotation]
-	if !depExist && !nsExist {
-		log.WithFields(log.Fields{
-			"namespace":  dep.Namespace,
-			"deployment": dep.Name,
-		}).Trace("annotation not present, not injecting")
+
+	if depExist {
+		logger.Debug("annotation present on deployment")
+		return true
+	}
+
+	if nsExist {
+		logger.Debug("annotation present on namespace")
+		return true
+	}
+
+	return false
+}
+
+// Needed determines whether a pod needs to get a sidecar injected or not
+func Needed(dep *appsv1.Deployment, ns *corev1.Namespace) bool {
+	if !Desired(dep, ns) {
 		return false
 	}
+
 	// do not inject jaeger due to port collision
 	// do not inject if deployment's Annotation value is false
 	if dep.Labels["app"] == "jaeger" {