From 4947ec8e881a5f7b3272efc8cdc6446c48d30693 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Fri, 23 Nov 2018 19:59:03 +0000 Subject: [PATCH 1/2] Enable JAEGER_SERVICE_NAME and JAEGER_PROPAGATION env vars to be set on containers with injected agent Signed-off-by: Gary Brown --- pkg/inject/sidecar.go | 39 ++++++++++++++++++ pkg/inject/sidecar_test.go | 83 +++++++++++++++++++++++++++++++++----- 2 files changed, 113 insertions(+), 9 deletions(-) diff --git a/pkg/inject/sidecar.go b/pkg/inject/sidecar.go index 87f811f07..42274c876 100644 --- a/pkg/inject/sidecar.go +++ b/pkg/inject/sidecar.go @@ -18,6 +18,11 @@ var ( Annotation = "inject-jaeger-agent" ) +const ( + serviceName = "JAEGER_SERVICE_NAME" + propagation = "JAEGER_PROPAGATION" +) + // Sidecar adds a new container to the deployment, connecting to the given jaeger instance func Sidecar(dep *appsv1.Deployment, jaeger *v1alpha1.Jaeger) { deployment.NewAgent(jaeger) // we need some initialization from that, but we don't actually need the agent's instance here @@ -25,6 +30,7 @@ func Sidecar(dep *appsv1.Deployment, jaeger *v1alpha1.Jaeger) { if jaeger == nil || dep.Annotations[Annotation] != jaeger.Name { logrus.Debugf("Skipping sidecar injection for deployment %v", dep.Name) } else { + decorate(dep) logrus.Debugf("Injecting sidecar for pod %v", dep.Name) dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, container(jaeger)) } @@ -96,3 +102,36 @@ func container(jaeger *v1alpha1.Jaeger) v1.Container { }, } } + +func decorate(dep *appsv1.Deployment) { + if app, found := dep.Spec.Template.Labels["app"]; found { + if len(dep.Namespace) > 0 { + app += "." + dep.Namespace + } else { + app += ".default" + } + for i := 0; i < len(dep.Spec.Template.Spec.Containers); i++ { + if !hasEnv(serviceName, dep.Spec.Template.Spec.Containers[i].Env) { + dep.Spec.Template.Spec.Containers[i].Env = append(dep.Spec.Template.Spec.Containers[i].Env, v1.EnvVar{ + Name: serviceName, + Value: app, + }) + } + if !hasEnv(propagation, dep.Spec.Template.Spec.Containers[i].Env) { + dep.Spec.Template.Spec.Containers[i].Env = append(dep.Spec.Template.Spec.Containers[i].Env, v1.EnvVar{ + Name: propagation, + Value: "jaeger,b3", + }) + } + } + } +} + +func hasEnv(name string, vars []v1.EnvVar) bool { + for i := 0; i < len(vars); i++ { + if vars[i].Name == name { + return true + } + } + return false +} diff --git a/pkg/inject/sidecar_test.go b/pkg/inject/sidecar_test.go index 309c74550..e12873190 100644 --- a/pkg/inject/sidecar_test.go +++ b/pkg/inject/sidecar_test.go @@ -28,15 +28,77 @@ func reset() { func TestInjectSidecar(t *testing.T) { jaeger := v1alpha1.NewJaeger("TestInjectSidecar") - dep := dep(map[string]string{Annotation: jaeger.Name}) + dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{}) Sidecar(dep, jaeger) assert.Len(t, dep.Spec.Template.Spec.Containers, 2) assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Image, "jaeger-agent") + assert.Len(t, dep.Spec.Template.Spec.Containers[0].Env, 0) +} + +func TestInjectSidecarWithEnvVars(t *testing.T) { + jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVars") + dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) + Sidecar(dep, jaeger) + assert.Len(t, dep.Spec.Template.Spec.Containers, 2) + assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Image, "jaeger-agent") + assert.Len(t, dep.Spec.Template.Spec.Containers[0].Env, 2) + assert.Equal(t, serviceName, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + assert.Equal(t, "testapp.default", dep.Spec.Template.Spec.Containers[0].Env[0].Value) + assert.Equal(t, propagation, dep.Spec.Template.Spec.Containers[0].Env[1].Name) + assert.Equal(t, "jaeger,b3", dep.Spec.Template.Spec.Containers[0].Env[1].Value) +} + +func TestInjectSidecarWithEnvVarsWithNamespace(t *testing.T) { + jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVars") + dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) + dep.Namespace = "mynamespace" + Sidecar(dep, jaeger) + assert.Len(t, dep.Spec.Template.Spec.Containers, 2) + assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Image, "jaeger-agent") + assert.Len(t, dep.Spec.Template.Spec.Containers[0].Env, 2) + assert.Equal(t, serviceName, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + assert.Equal(t, "testapp.mynamespace", dep.Spec.Template.Spec.Containers[0].Env[0].Value) + assert.Equal(t, propagation, dep.Spec.Template.Spec.Containers[0].Env[1].Name) + assert.Equal(t, "jaeger,b3", dep.Spec.Template.Spec.Containers[0].Env[1].Value) +} + +func TestInjectSidecarWithEnvVarsNoOverrideName(t *testing.T) { + jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVarsNoOverrideName") + dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) + dep.Spec.Template.Spec.Containers[0].Env = append(dep.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{ + Name: serviceName, + Value: "otherapp", + }) + Sidecar(dep, jaeger) + assert.Len(t, dep.Spec.Template.Spec.Containers, 2) + assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Image, "jaeger-agent") + assert.Len(t, dep.Spec.Template.Spec.Containers[0].Env, 2) + assert.Equal(t, serviceName, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + assert.Equal(t, "otherapp", dep.Spec.Template.Spec.Containers[0].Env[0].Value) + assert.Equal(t, propagation, dep.Spec.Template.Spec.Containers[0].Env[1].Name) + assert.Equal(t, "jaeger,b3", dep.Spec.Template.Spec.Containers[0].Env[1].Value) +} + +func TestInjectSidecarWithEnvVarsNoOverridePropagation(t *testing.T) { + jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVarsNoOverridePropagation") + dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) + dep.Spec.Template.Spec.Containers[0].Env = append(dep.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{ + Name: propagation, + Value: "tracecontext", + }) + Sidecar(dep, jaeger) + assert.Len(t, dep.Spec.Template.Spec.Containers, 2) + assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Image, "jaeger-agent") + assert.Len(t, dep.Spec.Template.Spec.Containers[0].Env, 2) + assert.Equal(t, propagation, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + assert.Equal(t, "tracecontext", dep.Spec.Template.Spec.Containers[0].Env[0].Value) + assert.Equal(t, serviceName, dep.Spec.Template.Spec.Containers[0].Env[1].Name) + assert.Equal(t, "testapp.default", dep.Spec.Template.Spec.Containers[0].Env[1].Value) } func TestSkipInjectSidecar(t *testing.T) { jaeger := v1alpha1.NewJaeger("TestSkipInjectSidecar") - dep := dep(map[string]string{Annotation: "non-existing-operator"}) + dep := dep(map[string]string{Annotation: "non-existing-operator"}, map[string]string{}) Sidecar(dep, jaeger) assert.Len(t, dep.Spec.Template.Spec.Containers, 1) assert.NotContains(t, dep.Spec.Template.Spec.Containers[0].Image, "jaeger-agent") @@ -59,12 +121,12 @@ func TestSidecarNotNeeded(t *testing.T) { } func TestSidecarNeeded(t *testing.T) { - dep := dep(map[string]string{Annotation: "some-jaeger-instance"}) + dep := dep(map[string]string{Annotation: "some-jaeger-instance"}, map[string]string{}) assert.True(t, Needed(dep)) } func TestHasSidecarAlready(t *testing.T) { - dep := dep(map[string]string{Annotation: "TestHasSidecarAlready"}) + dep := dep(map[string]string{Annotation: "TestHasSidecarAlready"}, map[string]string{}) assert.True(t, Needed(dep)) jaeger := v1alpha1.NewJaeger("TestHasSidecarAlready") Sidecar(dep, jaeger) @@ -72,7 +134,7 @@ func TestHasSidecarAlready(t *testing.T) { } func TestSelectSingleJaegerPod(t *testing.T) { - dep := dep(map[string]string{Annotation: "true"}) + dep := dep(map[string]string{Annotation: "true"}, map[string]string{}) jaegerPods := &v1alpha1.JaegerList{ Items: []v1alpha1.Jaeger{ v1alpha1.Jaeger{ @@ -89,7 +151,7 @@ func TestSelectSingleJaegerPod(t *testing.T) { } func TestCannotSelectFromMultipleJaegerPods(t *testing.T) { - dep := dep(map[string]string{Annotation: "true"}) + dep := dep(map[string]string{Annotation: "true"}, map[string]string{}) jaegerPods := &v1alpha1.JaegerList{ Items: []v1alpha1.Jaeger{ v1alpha1.Jaeger{ @@ -110,13 +172,13 @@ func TestCannotSelectFromMultipleJaegerPods(t *testing.T) { } func TestNoAvailableJaegerPods(t *testing.T) { - dep := dep(map[string]string{Annotation: "true"}) + dep := dep(map[string]string{Annotation: "true"}, map[string]string{}) jaeger := Select(dep, &v1alpha1.JaegerList{}) assert.Nil(t, jaeger) } func TestSelectBasedOnName(t *testing.T) { - dep := dep(map[string]string{Annotation: "the-second-jaeger-instance-available"}) + dep := dep(map[string]string{Annotation: "the-second-jaeger-instance-available"}, map[string]string{}) jaegerPods := &v1alpha1.JaegerList{ Items: []v1alpha1.Jaeger{ @@ -138,13 +200,16 @@ func TestSelectBasedOnName(t *testing.T) { assert.Equal(t, "the-second-jaeger-instance-available", jaeger.Name) } -func dep(annotations map[string]string) *appsv1.Deployment { +func dep(annotations map[string]string, labels map[string]string) *appsv1.Deployment { return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Annotations: annotations, }, Spec: appsv1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: labels, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{}, From 163fa63d1285b2aecf4af22c31c2bc8a4968ecf8 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Mon, 26 Nov 2018 12:42:54 +0000 Subject: [PATCH 2/2] Addressed comments Signed-off-by: Gary Brown --- pkg/inject/sidecar.go | 14 ++++++++------ pkg/inject/sidecar_test.go | 32 +++++++++++++++++--------------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/pkg/inject/sidecar.go b/pkg/inject/sidecar.go index 42274c876..3d0a45f24 100644 --- a/pkg/inject/sidecar.go +++ b/pkg/inject/sidecar.go @@ -19,8 +19,8 @@ var ( ) const ( - serviceName = "JAEGER_SERVICE_NAME" - propagation = "JAEGER_PROPAGATION" + envVarServiceName = "JAEGER_SERVICE_NAME" + envVarPropagation = "JAEGER_PROPAGATION" ) // Sidecar adds a new container to the deployment, connecting to the given jaeger instance @@ -105,21 +105,23 @@ func container(jaeger *v1alpha1.Jaeger) v1.Container { func decorate(dep *appsv1.Deployment) { if app, found := dep.Spec.Template.Labels["app"]; found { + // Append the namespace to the app name. Using the DNS style "."" + // which also matches with the style used in Istio. if len(dep.Namespace) > 0 { app += "." + dep.Namespace } else { app += ".default" } for i := 0; i < len(dep.Spec.Template.Spec.Containers); i++ { - if !hasEnv(serviceName, dep.Spec.Template.Spec.Containers[i].Env) { + if !hasEnv(envVarServiceName, dep.Spec.Template.Spec.Containers[i].Env) { dep.Spec.Template.Spec.Containers[i].Env = append(dep.Spec.Template.Spec.Containers[i].Env, v1.EnvVar{ - Name: serviceName, + Name: envVarServiceName, Value: app, }) } - if !hasEnv(propagation, dep.Spec.Template.Spec.Containers[i].Env) { + if !hasEnv(envVarPropagation, dep.Spec.Template.Spec.Containers[i].Env) { dep.Spec.Template.Spec.Containers[i].Env = append(dep.Spec.Template.Spec.Containers[i].Env, v1.EnvVar{ - Name: propagation, + Name: envVarPropagation, Value: "jaeger,b3", }) } diff --git a/pkg/inject/sidecar_test.go b/pkg/inject/sidecar_test.go index e12873190..5b86c6e7c 100644 --- a/pkg/inject/sidecar_test.go +++ b/pkg/inject/sidecar_test.go @@ -42,57 +42,59 @@ func TestInjectSidecarWithEnvVars(t *testing.T) { assert.Len(t, dep.Spec.Template.Spec.Containers, 2) assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Image, "jaeger-agent") assert.Len(t, dep.Spec.Template.Spec.Containers[0].Env, 2) - assert.Equal(t, serviceName, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + assert.Equal(t, envVarServiceName, dep.Spec.Template.Spec.Containers[0].Env[0].Name) assert.Equal(t, "testapp.default", dep.Spec.Template.Spec.Containers[0].Env[0].Value) - assert.Equal(t, propagation, dep.Spec.Template.Spec.Containers[0].Env[1].Name) + assert.Equal(t, envVarPropagation, dep.Spec.Template.Spec.Containers[0].Env[1].Name) assert.Equal(t, "jaeger,b3", dep.Spec.Template.Spec.Containers[0].Env[1].Value) } func TestInjectSidecarWithEnvVarsWithNamespace(t *testing.T) { - jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVars") + jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVarsWithNamespace") dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) dep.Namespace = "mynamespace" Sidecar(dep, jaeger) assert.Len(t, dep.Spec.Template.Spec.Containers, 2) assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Image, "jaeger-agent") assert.Len(t, dep.Spec.Template.Spec.Containers[0].Env, 2) - assert.Equal(t, serviceName, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + assert.Equal(t, envVarServiceName, dep.Spec.Template.Spec.Containers[0].Env[0].Name) assert.Equal(t, "testapp.mynamespace", dep.Spec.Template.Spec.Containers[0].Env[0].Value) - assert.Equal(t, propagation, dep.Spec.Template.Spec.Containers[0].Env[1].Name) + assert.Equal(t, envVarPropagation, dep.Spec.Template.Spec.Containers[0].Env[1].Name) assert.Equal(t, "jaeger,b3", dep.Spec.Template.Spec.Containers[0].Env[1].Value) } -func TestInjectSidecarWithEnvVarsNoOverrideName(t *testing.T) { - jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVarsNoOverrideName") +func TestInjectSidecarWithEnvVarsOverrideName(t *testing.T) { + jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVarsOverrideName") dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) dep.Spec.Template.Spec.Containers[0].Env = append(dep.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{ - Name: serviceName, + Name: envVarServiceName, Value: "otherapp", }) Sidecar(dep, jaeger) assert.Len(t, dep.Spec.Template.Spec.Containers, 2) assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Image, "jaeger-agent") assert.Len(t, dep.Spec.Template.Spec.Containers[0].Env, 2) - assert.Equal(t, serviceName, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + assert.Equal(t, envVarServiceName, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + // Explicitly provided env var is used instead of injected "app.namespace" value assert.Equal(t, "otherapp", dep.Spec.Template.Spec.Containers[0].Env[0].Value) - assert.Equal(t, propagation, dep.Spec.Template.Spec.Containers[0].Env[1].Name) + assert.Equal(t, envVarPropagation, dep.Spec.Template.Spec.Containers[0].Env[1].Name) assert.Equal(t, "jaeger,b3", dep.Spec.Template.Spec.Containers[0].Env[1].Value) } -func TestInjectSidecarWithEnvVarsNoOverridePropagation(t *testing.T) { - jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVarsNoOverridePropagation") +func TestInjectSidecarWithEnvVarsOverridePropagation(t *testing.T) { + jaeger := v1alpha1.NewJaeger("TestInjectSidecarWithEnvVarsOverridePropagation") dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) dep.Spec.Template.Spec.Containers[0].Env = append(dep.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{ - Name: propagation, + Name: envVarPropagation, Value: "tracecontext", }) Sidecar(dep, jaeger) assert.Len(t, dep.Spec.Template.Spec.Containers, 2) assert.Contains(t, dep.Spec.Template.Spec.Containers[1].Image, "jaeger-agent") assert.Len(t, dep.Spec.Template.Spec.Containers[0].Env, 2) - assert.Equal(t, propagation, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + assert.Equal(t, envVarPropagation, dep.Spec.Template.Spec.Containers[0].Env[0].Name) + // Explicitly provided propagation env var used instead of injected "jaeger,b3" value assert.Equal(t, "tracecontext", dep.Spec.Template.Spec.Containers[0].Env[0].Value) - assert.Equal(t, serviceName, dep.Spec.Template.Spec.Containers[0].Env[1].Name) + assert.Equal(t, envVarServiceName, dep.Spec.Template.Spec.Containers[0].Env[1].Name) assert.Equal(t, "testapp.default", dep.Spec.Template.Spec.Containers[0].Env[1].Value) }