diff --git a/pkg/autodetect/main.go b/pkg/autodetect/main.go index 296baa4a2..590c75c2b 100644 --- a/pkg/autodetect/main.go +++ b/pkg/autodetect/main.go @@ -243,7 +243,7 @@ func (b *Background) cleanDeployments(ctx context.Context) { // check deployments to see which one needs to be cleaned. for _, dep := range deployments.Items { - if instanceName, ok := dep.Annotations[inject.Annotation]; ok { + if instanceName, ok := dep.Labels[inject.Label]; ok { _, instanceExists := instancesMap[instanceName] if !instanceExists { // Jaeger instance not exist anymore, we need to clean this up. inject.CleanSidecar(&dep) diff --git a/pkg/inject/sidecar.go b/pkg/inject/sidecar.go index 541d171f9..eed1535df 100644 --- a/pkg/inject/sidecar.go +++ b/pkg/inject/sidecar.go @@ -43,20 +43,25 @@ func Sidecar(jaeger *v1.Jaeger, dep *appsv1.Deployment) *appsv1.Deployment { deployment.NewAgent(jaeger) // we need some initialization from that, but we don't actually need the agent's instance here logFields := jaeger.Logger().WithField("deployment", dep.Name) - if jaeger == nil || (dep.Annotations[Annotation] != jaeger.Name && dep.Annotations[AnnotationLegacy] != jaeger.Name) { - logFields.Trace("skipping sidecar injection") - } else { - decorate(dep) + if jaeger == nil { + logFields.Trace("no Jaeger instance found, skipping sidecar injection") + return dep + } - logFields.Debug("injecting sidecar") - dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, container(jaeger, dep)) + if val, ok := dep.Labels[Label]; ok && val != jaeger.Name { + logFields.Trace("deployment is assigned to a different Jaeger instance, skipping sidecar injection") + return dep + } - jaegerName := util.Truncate(jaeger.Name, 63) - if dep.Labels == nil { - dep.Labels = map[string]string{Label: jaegerName} - } else { - dep.Labels[Label] = jaegerName - } + decorate(dep) + logFields.Debug("injecting sidecar") + dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, container(jaeger, dep)) + jaegerName := util.Truncate(jaeger.Name, 63) + + if dep.Labels == nil { + dep.Labels = map[string]string{Label: jaegerName} + } else { + dep.Labels[Label] = jaegerName } return dep @@ -104,10 +109,6 @@ func Select(target *appsv1.Deployment, ns *corev1.Namespace, availableJaegerPods // jaeger instance to use! // first, we make sure we normalize the name: jaeger := &availableJaegerPods.Items[0] - if target.Annotations == nil { - target.Annotations = map[string]string{} - } - target.Annotations[Annotation] = jaeger.Name return jaeger } return nil diff --git a/pkg/inject/sidecar_test.go b/pkg/inject/sidecar_test.go index 8b32948b6..9cf9f0257 100644 --- a/pkg/inject/sidecar_test.go +++ b/pkg/inject/sidecar_test.go @@ -32,17 +32,9 @@ func reset() { func TestInjectSidecar(t *testing.T) { jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestInjectSidecar"}) - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{}) - dep = Sidecar(jaeger, dep) - 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 TestInjectSidecarWithLegacyAnnotation(t *testing.T) { - jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestInjectSidecarWithLegacyAnnotation"}) - dep := dep(map[string]string{AnnotationLegacy: jaeger.Name}, map[string]string{}) + dep := dep(map[string]string{}, map[string]string{}) dep = Sidecar(jaeger, dep) + assert.Equal(t, dep.Labels[Label], jaeger.Name) 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) @@ -70,7 +62,7 @@ func TestInjectSidecarWithEnvVars(t *testing.T) { func TestInjectSidecarWithEnvVarsK8sAppName(t *testing.T) { // prepare jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestInjectSidecarWithEnvVarsK8sAppName"}) - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{ + dep := dep(map[string]string{}, map[string]string{ "app": "noapp", "app.kubernetes.io/name": "testapp", }) @@ -87,7 +79,7 @@ func TestInjectSidecarWithEnvVarsK8sAppName(t *testing.T) { func TestInjectSidecarWithEnvVarsK8sAppInstance(t *testing.T) { // prepare jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestInjectSidecarWithEnvVarsK8sAppInstance"}) - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{ + dep := dep(map[string]string{}, map[string]string{ "app": "noapp", "app.kubernetes.io/name": "noname", "app.kubernetes.io/instance": "testapp", @@ -105,7 +97,7 @@ func TestInjectSidecarWithEnvVarsK8sAppInstance(t *testing.T) { func TestInjectSidecarWithEnvVarsWithNamespace(t *testing.T) { // prepare jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestInjectSidecarWithEnvVarsWithNamespace"}) - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) + dep := dep(map[string]string{}, map[string]string{"app": "testapp"}) dep.Namespace = "mynamespace" // test @@ -123,7 +115,7 @@ func TestInjectSidecarWithEnvVarsWithNamespace(t *testing.T) { func TestInjectSidecarWithEnvVarsOverrideName(t *testing.T) { // prepare jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestInjectSidecarWithEnvVarsOverrideName"}) - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) + dep := dep(map[string]string{}, map[string]string{"app": "testapp"}) envVar := corev1.EnvVar{ Name: envVarServiceName, Value: "otherapp", @@ -149,7 +141,7 @@ func TestInjectSidecarWithEnvVarsOverridePropagation(t *testing.T) { Name: envVarPropagation, Value: "tracecontext", } - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) + dep := dep(map[string]string{}, map[string]string{"app": "testapp"}) dep.Spec.Template.Spec.Containers[0].Env = append(dep.Spec.Template.Spec.Containers[0].Env, traceContextEnvVar) // test @@ -167,7 +159,7 @@ func TestInjectSidecarWithEnvVarsOverridePropagation(t *testing.T) { func TestSidecarDefaultPorts(t *testing.T) { // prepare jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestSidecarPorts"}) - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{"app": "testapp"}) + dep := dep(map[string]string{}, map[string]string{"app": "testapp"}) // test dep = Sidecar(jaeger, dep) @@ -187,7 +179,7 @@ func TestSidecarDefaultPorts(t *testing.T) { func TestSkipInjectSidecar(t *testing.T) { // prepare jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestSkipInjectSidecar"}) - dep := dep(map[string]string{Annotation: "non-existing-operator"}, map[string]string{}) + dep := dep(map[string]string{}, map[string]string{Label: "non-existing-operator"}) // test dep = Sidecar(jaeger, dep) @@ -198,7 +190,7 @@ func TestSkipInjectSidecar(t *testing.T) { } func TestSidecarNeeded(t *testing.T) { - depWithAgent := dep(map[string]string{Annotation: "some-jaeger-instance"}, map[string]string{}) + depWithAgent := dep(map[string]string{}, map[string]string{}) depWithAgent.Spec.Template.Spec.Containers = append(depWithAgent.Spec.Template.Spec.Containers, corev1.Container{ Name: "jaeger-agent", }) @@ -375,7 +367,7 @@ func TestSidecarOrderOfArguments(t *testing.T) { "c-option": "c-value", }) - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{}) + dep := dep(map[string]string{}, map[string]string{}) dep = Sidecar(jaeger, dep) assert.Len(t, dep.Spec.Template.Spec.Containers, 2) @@ -394,7 +386,7 @@ func TestSidecarExplicitTags(t *testing.T) { // prepare jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestSidecarExplicitTags"}) jaeger.Spec.Agent.Options = v1.NewOptions(map[string]interface{}{"jaeger.tags": "key=val"}) - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{}) + dep := dep(map[string]string{}, map[string]string{}) // test dep = Sidecar(jaeger, dep) @@ -412,7 +404,7 @@ func TestSidecarOverrideReporter(t *testing.T) { "reporter.thrift.host-port": "collector:14267", }) - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{}) + dep := dep(map[string]string{}, map[string]string{}) dep = Sidecar(jaeger, dep) assert.Len(t, dep.Spec.Template.Spec.Containers, 2) @@ -444,7 +436,7 @@ func TestSidecarAgentResources(t *testing.T) { }, } - dep := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{}) + dep := dep(map[string]string{}, map[string]string{}) dep = Sidecar(jaeger, dep) assert.Len(t, dep.Spec.Template.Spec.Containers, 2, "Expected 2 containers") @@ -463,7 +455,7 @@ func TestCleanSidecars(t *testing.T) { Namespace: "Test", } jaeger := v1.NewJaeger(nsn) - dep1 := Sidecar(jaeger, dep(map[string]string{Annotation: jaeger.Name}, map[string]string{})) + dep1 := Sidecar(jaeger, dep(map[string]string{}, map[string]string{})) CleanSidecar(dep1) assert.Equal(t, len(dep1.Spec.Template.Spec.Containers), 1) @@ -475,10 +467,10 @@ func TestSidecarWithLabel(t *testing.T) { Namespace: "Test", } jaeger := v1.NewJaeger(nsn) - dep1 := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{}) + dep1 := dep(map[string]string{}, map[string]string{}) dep1 = Sidecar(jaeger, dep1) assert.Equal(t, dep1.Labels[Label], "TestSidecarWithLabel") - dep2 := dep(map[string]string{Annotation: jaeger.Name}, map[string]string{}) + dep2 := dep(map[string]string{}, map[string]string{}) dep2.Labels = map[string]string{"anotherLabel": "anotherValue"} dep2 = Sidecar(jaeger, dep2) assert.Equal(t, len(dep2.Labels), 2) @@ -489,7 +481,7 @@ func TestSidecarWithLabel(t *testing.T) { func TestSidecarWithoutPrometheusAnnotations(t *testing.T) { // prepare jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestSidecarWithoutPrometheusAnnotations"}) - dep := Sidecar(jaeger, dep(map[string]string{Annotation: jaeger.Name}, map[string]string{})) + dep := Sidecar(jaeger, dep(map[string]string{}, map[string]string{})) // test dep = Sidecar(jaeger, dep) @@ -503,7 +495,6 @@ func TestSidecarWithPrometheusAnnotations(t *testing.T) { // prepare jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestSidecarWithPrometheusAnnotations"}) dep := dep(map[string]string{ - Annotation: jaeger.Name, "prometheus.io/scrape": "false", "prometheus.io/port": "9090", }, map[string]string{}) @@ -518,8 +509,9 @@ func TestSidecarWithPrometheusAnnotations(t *testing.T) { func TestSidecarAgentTagsWithMultipleContainers(t *testing.T) { jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestSidecarAgentTagsWithMultipleContainers"}) - dep := Sidecar(jaeger, depWithTwoContainers(map[string]string{Annotation: jaeger.Name}, map[string]string{})) + dep := Sidecar(jaeger, depWithTwoContainers(map[string]string{}, map[string]string{})) + assert.Equal(t, dep.Labels[Label], jaeger.Name) assert.Len(t, dep.Spec.Template.Spec.Containers, 3, "Expected 3 containers") assert.Equal(t, "jaeger-agent", dep.Spec.Template.Spec.Containers[2].Name) containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[2].Args, "--jaeger.tags") diff --git a/test/e2e/sidecar_test.go b/test/e2e/sidecar_test.go index 2b690cc8f..eafb7d83c 100644 --- a/test/e2e/sidecar_test.go +++ b/test/e2e/sidecar_test.go @@ -16,6 +16,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" @@ -64,13 +65,14 @@ func (suite *SidecarTestSuite) AfterTest(suiteName, testName string) { // Sidecar runs a test with the agent as sidecar func (suite *SidecarTestSuite) TestSidecar() { + cleanupOptions := &framework.CleanupOptions{TestContext: ctx, Timeout: timeout, RetryInterval: retryInterval} jaegerInstanceName := "agent-as-sidecar" j := getJaegerAgentAsSidecarDefinition(jaegerInstanceName, namespace) + undeployJaegerInstance(j) err := fw.Client.Create(goctx.TODO(), j, cleanupOptions) require.NoError(t, err, "Failed to create jaeger instance") - defer undeployJaegerInstance(j) err = e2eutil.WaitForDeployment(t, fw.KubeClient, namespace, jaegerInstanceName, 1, retryInterval, timeout) require.NoError(t, err, "Error waiting for Jaeger instance deployment") @@ -99,6 +101,45 @@ func (suite *SidecarTestSuite) TestSidecar() { return len(resp.Data) > 0, nil }) require.NoError(t, err, "Failed waiting for expected content") + + /* Testing other instance */ + + otherJaegerInstanceName := "agent-as-sidecar2" + j2 := getJaegerAgentAsSidecarDefinition(otherJaegerInstanceName, namespace) + defer undeployJaegerInstance(j2) + + err = fw.Client.Create(goctx.TODO(), j2, cleanupOptions) + err = e2eutil.WaitForDeployment(t, fw.KubeClient, namespace, jaegerInstanceName, 1, retryInterval, timeout) + require.NoError(t, err, "Error waiting for Jaeger instance deployment") + + persisted := &appsv1.Deployment{} + err = fw.Client.Get(goctx.TODO(), types.NamespacedName{ + Name: "vertx-create-span-sidecar", + Namespace: namespace, + }, persisted) + require.NoError(t, err, "Error getting jaeger instance") + require.Equal(t, "agent-as-sidecar", persisted.Labels[inject.Label]) + + err = fw.Client.Delete(goctx.TODO(), j) + require.NoError(t, err, "Error deleting instance") + + url, httpClient = getQueryURLAndHTTPClient(otherJaegerInstanceName, "%s/api/traces?service=order", true) + req, err = http.NewRequest(http.MethodGet, url, nil) + + err = wait.Poll(retryInterval, timeout, func() (done bool, err error) { + res, err := httpClient.Do(req) + require.NoError(t, err) + + body, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + + resp := &resp{} + err = json.Unmarshal(body, &resp) + require.NoError(t, err) + + return len(resp.Data) > 0, nil + }) + require.NoError(t, err, "Failed waiting for expected content") } func getVertxDefinition(annotations map[string]string) *appsv1.Deployment { diff --git a/test/e2e/utils.go b/test/e2e/utils.go index c432ad468..1f465a1d9 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -243,13 +243,20 @@ func printTestStackTrace() { } } -func undeployJaegerInstance(jaeger *v1.Jaeger) { +func undeployJaegerInstance(jaeger *v1.Jaeger) bool { if !debugMode || !t.Failed() { err := fw.Client.Delete(goctx.TODO(), jaeger) - require.NoError(t, err, "Error undeploying Jaeger") - err = e2eutil.WaitForDeletion(t, fw.Client.Client, jaeger, retryInterval, timeout) - require.NoError(t, err) + if err := fw.Client.Delete(goctx.TODO(), jaeger); err != nil { + return false + } + + if err = e2eutil.WaitForDeletion(t, fw.Client.Client, jaeger, retryInterval, timeout); err != nil { + return false + } + return true } + // Always return true, we don't care + return true } func getJaegerInstance(name, namespace string) *v1.Jaeger {