Add OTEL config to all-in-one (#1080)

* Add OTEL config to all-in-one Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Remove yaml Signed-off-by: Pavol Loffay <ploffay@redhat.com> * exclude openapi from coverage Signed-off-by: Pavol Loffay <ploffay@redhat.com> * Add test for error path Signed-off-by: Pavol Loffay <ploffay@redhat.com>
jaegertracing · Jun 4, 2020 · e48d7d3 · e48d7d3
1 parent c4418b6
commit e48d7d3
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 15 deletions.
diff --git a/.codecov.yml b/.codecov.yml
@@ -3,5 +3,6 @@ ignore:
   - "pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go"
   - "pkg/apis/io/v1alpha1/zz_generated.defaults.go"
   - "pkg/apis/jaegertracing/v1/zz_generated.defaults.go"
+  - "pkg/apis/jaegertracing/v1/zz_generated.openapi.go"
   - "pkg/apis/kafka/v1beta1/zz_generated.deepcopy.go"
   - "pkg/apis/kafka/v1beta1/zz_generated.openapi.go"
diff --git a/deploy/crds/jaegertracing.io_jaegers_crd.yaml b/deploy/crds/jaegertracing.io_jaegers_crd.yaml
@@ -1542,6 +1542,8 @@ spec:
                     type: string
                   nullable: true
                   type: object
+                config:
+                  type: object
                 image:
                   type: string
                 labels:

diff --git a/pkg/apis/jaegertracing/v1/jaeger_types.go b/pkg/apis/jaegertracing/v1/jaeger_types.go
@@ -285,6 +285,9 @@ type JaegerAllInOneSpec struct {
 	// +optional
 	Options Options `json:"options,omitempty"`
 
+	// +optional
+	Config FreeForm `json:"config,omitempty"`
+
 	// +optional
 	JaegerCommonSpec `json:",inline,omitempty"`
 }

diff --git a/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go b/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go
diff --git a/pkg/apis/jaegertracing/v1/zz_generated.openapi.go b/pkg/apis/jaegertracing/v1/zz_generated.openapi.go
diff --git a/pkg/config/otelconfig/otelconfig.go b/pkg/config/otelconfig/otelconfig.go
@@ -26,11 +26,7 @@ func ShouldCreate(jaeger *v1.Jaeger, opts v1.Options, otelCfg map[string]interfa
 		jaeger.Logger().Info("OpenTelemetry config will not be created. The config is explicitly provided in the options.")
 		return false
 	}
-	if len(otelCfg) == 0 {
-		return false
-	}
-	return true
-
+	return len(otelCfg) > 0
 }
 
 // Get returns a OTEL config maps for a Jaeger instance.
@@ -48,6 +44,10 @@ func Get(jaeger *v1.Jaeger) []corev1.ConfigMap {
 	if c != nil {
 		cms = append(cms, *c)
 	}
+	c = createIfNeeded(jaeger, "all-in-one", jaeger.Spec.AllInOne.Options, jaeger.Spec.AllInOne.Config)
+	if c != nil {
+		cms = append(cms, *c)
+	}
 	return cms
 }
 

diff --git a/pkg/deployment/all_in_one.go b/pkg/deployment/all_in_one.go
@@ -5,6 +5,8 @@ import (
 	"sort"
 	"strconv"
 
+	"github.com/jaegertracing/jaeger-operator/pkg/config/otelconfig"
+
 	"github.com/spf13/viper"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -60,21 +62,26 @@ func (a *AllInOne) Get() *appsv1.Deployment {
 	sampling.Update(a.jaeger, commonSpec, &options)
 	tls.Update(a.jaeger, commonSpec, &options)
 
-	if len(util.FindItem("--reporter.type=", options)) == 0 {
-		options = append(options, "--reporter.type=grpc")
-	}
-
 	// Enable tls by default for openshift platform
 	// even though the agent is in the same process as the collector, they communicate via gRPC, and the collector has TLS enabled,
 	// as it might receive connections from external agents
 	if viper.GetString("platform") == v1.FlagPlatformOpenShift {
-		if len(util.FindItem("--reporter.type=grpc", options)) > 0 && len(util.FindItem("--reporter.grpc.tls.enabled=true", options)) == 0 {
+		if len(util.FindItem("--reporter.grpc.tls.enabled=true", options)) == 0 {
 			options = append(options, "--reporter.grpc.tls.enabled=true")
 			options = append(options, "--reporter.grpc.tls.ca=/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt")
 			options = append(options, fmt.Sprintf("--reporter.grpc.tls.server-name=%s.%s.svc.cluster.local", service.GetNameForHeadlessCollectorService(a.jaeger), a.jaeger.Namespace))
 		}
 	}
 
+	otelConf, err := a.jaeger.Spec.AllInOne.Config.GetMap()
+	if err != nil {
+		a.jaeger.Logger().WithField("error", err).
+			WithField("component", "all-in-one").
+			Errorf("Could not parse OTEL config, config map will not be created")
+	} else if otelconfig.ShouldCreate(a.jaeger, a.jaeger.Spec.AllInOne.Options, otelConf) {
+		otelconfig.Update(a.jaeger, "all-in-one", commonSpec, &options)
+	}
+
 	// ensure we have a consistent order of the arguments
 	// see https://github.com/jaegertracing/jaeger-operator/issues/334
 	sort.Strings(options)

diff --git a/pkg/deployment/all_in_one_test.go b/pkg/deployment/all_in_one_test.go
@@ -3,6 +3,8 @@ package deployment
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
 	"github.com/jaegertracing/jaeger-operator/pkg/version"
 
@@ -277,14 +279,13 @@ func TestAllInOneOrderOfArguments(t *testing.T) {
 	dep := a.Get()
 
 	assert.Len(t, dep.Spec.Template.Spec.Containers, 1)
-	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 5)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 4)
 	assert.NotEmpty(t, util.FindItem("--a-option", dep.Spec.Template.Spec.Containers[0].Args))
 	assert.NotEmpty(t, util.FindItem("--b-option", dep.Spec.Template.Spec.Containers[0].Args))
 	assert.NotEmpty(t, util.FindItem("--c-option", dep.Spec.Template.Spec.Containers[0].Args))
 
 	// the following are added automatically
 	assert.NotEmpty(t, util.FindItem("--sampling.strategies-file", dep.Spec.Template.Spec.Containers[0].Args))
-	assert.NotEmpty(t, util.FindItem("--reporter.type=grpc", dep.Spec.Template.Spec.Containers[0].Args))
 }
 
 func TestAllInOneArgumentsOpenshiftTLS(t *testing.T) {
@@ -303,7 +304,7 @@ func TestAllInOneArgumentsOpenshiftTLS(t *testing.T) {
 
 	// verify
 	assert.Len(t, dep.Spec.Template.Spec.Containers, 1)
-	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 9)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 8)
 	assert.NotEmpty(t, util.FindItem("--a-option=a-value", dep.Spec.Template.Spec.Containers[0].Args))
 	assert.NotEmpty(t, util.FindItem("--collector.grpc.tls.enabled=true", dep.Spec.Template.Spec.Containers[0].Args))
 	assert.NotEmpty(t, util.FindItem("--collector.grpc.tls.cert=/etc/tls-config/tls.crt", dep.Spec.Template.Spec.Containers[0].Args))
@@ -313,5 +314,29 @@ func TestAllInOneArgumentsOpenshiftTLS(t *testing.T) {
 	assert.NotEmpty(t, util.FindItem("--reporter.grpc.tls.ca", dep.Spec.Template.Spec.Containers[0].Args))
 	assert.NotEmpty(t, util.FindItem("--reporter.grpc.tls.enabled", dep.Spec.Template.Spec.Containers[0].Args))
 	assert.NotEmpty(t, util.FindItem("--reporter.grpc.tls.server-name", dep.Spec.Template.Spec.Containers[0].Args))
-	assert.Equal(t, "--reporter.type=grpc", util.FindItem("--reporter.type", dep.Spec.Template.Spec.Containers[0].Args))
+}
+
+func TestAllInOneOTELConfig(t *testing.T) {
+	jaeger := v1.NewJaeger(types.NamespacedName{Name: "instance"})
+	jaeger.Spec.AllInOne.Config = v1.NewFreeForm(map[string]interface{}{"foo": "bar"})
+
+	c := NewAllInOne(jaeger)
+	d := c.Get()
+	assert.True(t, hasArgument("--config=/etc/jaeger/otel/config.yaml", d.Spec.Template.Spec.Containers[0].Args))
+	assert.True(t, hasVolume("instance-all-in-one-otel-config", d.Spec.Template.Spec.Volumes))
+	assert.True(t, hasVolumeMount("instance-all-in-one-otel-config", d.Spec.Template.Spec.Containers[0].VolumeMounts))
+}
+
+func TestAllInOneOTELConfig_error(t *testing.T) {
+	jaeger := v1.NewJaeger(types.NamespacedName{Name: "instance"})
+	jaeger.Spec.AllInOne.Config = v1.NewFreeForm(map[string]interface{}{})
+	jaeger.Spec.AllInOne.Config.UnmarshalJSON([]byte(""))
+	_, err := jaeger.Spec.AllInOne.Config.GetMap()
+	require.Error(t, err)
+
+	c := NewAllInOne(jaeger)
+	d := c.Get()
+	assert.False(t, hasArgument("--config=/etc/jaeger/otel/config.yaml", d.Spec.Template.Spec.Containers[0].Args))
+	assert.False(t, hasVolume("instance-all-in-one-otel-config", d.Spec.Template.Spec.Volumes))
+	assert.False(t, hasVolumeMount("instance-all-in-one-otel-config", d.Spec.Template.Spec.Containers[0].VolumeMounts))
 }