Skip to content

Commit

Permalink
No modify annotation when injecting (#902)
Browse files Browse the repository at this point in the history 
Signed-off-by: Ruben Vargas <[email protected]>
  • Loading branch information
@rubenvp8510
rubenvp8510 authored Mar 16, 2020
1 parent c01e4b6 commit 9e2cffa
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 50 deletions.
2 changes: 1 addition & 1 deletion pkg/autodetect/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
33 changes: 17 additions & 16 deletions pkg/inject/sidecar.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
48 changes: 20 additions & 28 deletions pkg/inject/sidecar_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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",
})
Expand All @@ -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",
Expand All @@ -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
Expand All @@ -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",
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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",
})
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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")
Expand All @@ -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)

Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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{})
Expand All @@ -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")
Expand Down
43 changes: 42 additions & 1 deletion test/e2e/sidecar_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 11 additions & 4 deletions test/e2e/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 9e2cffa

Please sign in to comment.