-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try to resolve container.name from the injected agent args #1319
Changes from 3 commits
65a8d62
fd27c42
4324e78
4aab7d1
6fbe2ec
436bcc5
aef839e
980af69
fa80037
de6d80d
5a14178
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package inject | |
|
||
import ( | ||
"fmt" | ||
"reflect" | ||
"strings" | ||
"testing" | ||
|
||
|
@@ -508,8 +509,9 @@ func TestSidecarOrderOfArguments(t *testing.T) { | |
containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--c-option") | ||
containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--jaeger.tags") | ||
containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--reporter.grpc.host-port") | ||
agentTags := agentTags(dep.Spec.Template.Spec.Containers[1].Args) | ||
assert.Contains(t, agentTags, "container.name=only_container") | ||
agentTagsMap := parseAgentTags(dep.Spec.Template.Spec.Containers[1].Args) | ||
assert.Contains(t, agentTagsMap, "container.name") | ||
assert.Equal(t, agentTagsMap["container.name"], "only_container") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you are changing this PR for some other reason, you might want to remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you're right. Unnecessary |
||
} | ||
|
||
func TestSidecarExplicitTags(t *testing.T) { | ||
|
@@ -523,8 +525,8 @@ func TestSidecarExplicitTags(t *testing.T) { | |
|
||
// verify | ||
assert.Len(t, dep.Spec.Template.Spec.Containers, 2) | ||
agentTags := agentTags(dep.Spec.Template.Spec.Containers[1].Args) | ||
assert.Equal(t, []string{"key=val"}, agentTags) | ||
agentTags := parseAgentTags(dep.Spec.Template.Spec.Containers[1].Args) | ||
assert.True(t, reflect.DeepEqual(agentTags, map[string]string{"key": "val"})) | ||
jpkrohling marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
func TestSidecarCustomReporterPort(t *testing.T) { | ||
|
@@ -671,8 +673,31 @@ func TestSidecarAgentTagsWithMultipleContainers(t *testing.T) { | |
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") | ||
agentTags := agentTags(dep.Spec.Template.Spec.Containers[2].Args) | ||
assert.Equal(t, "", util.FindItem("container.name=", agentTags)) | ||
agentTagsMap := parseAgentTags(dep.Spec.Template.Spec.Containers[2].Args) | ||
assert.NotContains(t, agentTagsMap, "container.name") | ||
} | ||
|
||
func TestSidecarAgentContainerNameTagWithDoubleInjectedContainer(t *testing.T) { | ||
jaeger := v1.NewJaeger(types.NamespacedName{Name: "my-instance"}) | ||
dep := Sidecar(jaeger, dep(map[string]string{}, map[string]string{})) | ||
|
||
// inject - 1st time | ||
assert.Equal(t, dep.Labels[Label], jaeger.Name) | ||
assert.Len(t, dep.Spec.Template.Spec.Containers, 2, "Expected 2 containers") | ||
assert.Equal(t, "jaeger-agent", dep.Spec.Template.Spec.Containers[1].Name) | ||
containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--jaeger.tags") | ||
agentTagsMap := parseAgentTags(dep.Spec.Template.Spec.Containers[1].Args) | ||
assert.Contains(t, agentTagsMap, "container.name") | ||
assert.Equal(t, agentTagsMap["container.name"], "only_container") | ||
|
||
// inject - 2nd time due to deployment/namespace reconciliation | ||
dep = Sidecar(jaeger, dep) | ||
assert.Len(t, dep.Spec.Template.Spec.Containers, 2, "Expected 2 containers") | ||
assert.Equal(t, "jaeger-agent", dep.Spec.Template.Spec.Containers[1].Name) | ||
containsOptionWithPrefix(t, dep.Spec.Template.Spec.Containers[1].Args, "--jaeger.tags") | ||
agentTagsMap = parseAgentTags(dep.Spec.Template.Spec.Containers[1].Args) | ||
assert.Contains(t, agentTagsMap, "container.name") | ||
assert.Equal(t, agentTagsMap["container.name"], "only_container") | ||
} | ||
|
||
func ns(annotations map[string]string) *corev1.Namespace { | ||
|
@@ -734,15 +759,6 @@ func containsOptionWithPrefix(t *testing.T, args []string, prefix string) bool { | |
return false | ||
} | ||
|
||
func agentTags(args []string) []string { | ||
tagsArg := util.FindItem("--jaeger.tags=", args) | ||
if tagsArg == "" { | ||
return []string{} | ||
} | ||
tagsParam := strings.SplitN(tagsArg, "=", 2)[1] | ||
return strings.Split(tagsParam, ",") | ||
} | ||
|
||
func TestSidecarArgumentsOpenshiftTLS(t *testing.T) { | ||
viper.Set("platform", v1.FlagPlatformOpenShift) | ||
defer viper.Reset() | ||
|
@@ -765,8 +781,9 @@ func TestSidecarArgumentsOpenshiftTLS(t *testing.T) { | |
assert.Greater(t, len(util.FindItem("--reporter.grpc.host-port=dns:///my-instance-collector-headless.test.svc:14250", dep.Spec.Template.Spec.Containers[1].Args)), 0) | ||
assert.Greater(t, len(util.FindItem("--reporter.grpc.tls.enabled=true", dep.Spec.Template.Spec.Containers[1].Args)), 0) | ||
assert.Greater(t, len(util.FindItem("--reporter.grpc.tls.ca="+ca.ServiceCAPath, dep.Spec.Template.Spec.Containers[1].Args)), 0) | ||
agentTags := agentTags(dep.Spec.Template.Spec.Containers[1].Args) | ||
assert.Contains(t, agentTags, "container.name=only_container") | ||
agentTagsMap := parseAgentTags(dep.Spec.Template.Spec.Containers[1].Args) | ||
assert.Contains(t, agentTagsMap, "container.name") | ||
assert.Equal(t, agentTagsMap["container.name"], "only_container") | ||
} | ||
|
||
func TestEqualSidecar(t *testing.T) { | ||
|
@@ -863,3 +880,37 @@ func TestSidecarWithSecurityContext(t *testing.T) { | |
assert.Len(t, dep.Spec.Template.Spec.Containers, 2) | ||
assert.Equal(t, dep.Spec.Template.Spec.Containers[1].SecurityContext, expectedSecurityContext) | ||
} | ||
|
||
func TestSortedTags(t *testing.T) { | ||
defaultAgentTagsMap := make(map[string]string) | ||
defaultAgentTagsMap["cluster"] = "undefined" // this value isn't currently available | ||
defaultAgentTagsMap["deployment.name"] = "deploy" | ||
defaultAgentTagsMap["pod.namespace"] = "ns" | ||
defaultAgentTagsMap["pod.name"] = "pod_name" | ||
defaultAgentTagsMap["host.ip"] = "0.0.0.0" | ||
assert.Equal(t, joinTags(defaultAgentTagsMap), fmt.Sprintf("%s=%s,%s=%s,%s=%s,%s=%s,%s=%s", | ||
"cluster", "undefined", // this value isn't currently available | ||
"deployment.name", "deploy", | ||
"host.ip", "0.0.0.0", | ||
"pod.name", "pod_name", | ||
"pod.namespace", "ns", | ||
)) | ||
} | ||
|
||
func TestSortedTagsWithContainer(t *testing.T) { | ||
defaultAgentTagsMap := make(map[string]string) | ||
defaultAgentTagsMap["cluster"] = "undefined" // this value isn't currently available | ||
defaultAgentTagsMap["deployment.name"] = "deploy" | ||
defaultAgentTagsMap["pod.namespace"] = "ns" | ||
defaultAgentTagsMap["pod.name"] = "pod_name" | ||
defaultAgentTagsMap["host.ip"] = "0.0.0.0" | ||
defaultAgentTagsMap["container.name"] = "only_container" | ||
assert.Equal(t, joinTags(defaultAgentTagsMap), fmt.Sprintf("%s=%s,%s=%s,%s=%s,%s=%s,%s=%s,%s=%s", | ||
"cluster", "undefined", // this value isn't currently available | ||
"container.name", "only_container", | ||
"deployment.name", "deploy", | ||
"host.ip", "0.0.0.0", | ||
"pod.name", "pod_name", | ||
"pod.namespace", "ns", | ||
)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's up with the changes to go.mod and go.sum ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have reverted all changes to
go.mod
andgo.sum
. But may be this is an indirect dependency, so it will be cleaned bygo mod tidy
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be. Once this gets merged, would you like to run a
go mod tidy
against master and seeing if there are differences in the go.mod?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I would like to have a try.