Skip to content

Commit

Permalink
Keep reporter options when explicitly specified
Browse files Browse the repository at this point in the history
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
  • Loading branch information
jpkrohling committed Mar 20, 2019
1 parent 4e65aa5 commit bd1518f
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 9 deletions.
14 changes: 11 additions & 3 deletions pkg/deployment/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,17 @@ func (a *Agent) Get() *appsv1.DaemonSet {
return nil
}

args := append(a.jaeger.Spec.Agent.Options.ToArgs(),
"--reporter.type=grpc",
fmt.Sprintf("--reporter.grpc.host-port=dns:///%s:14250", service.GetNameForCollectorService(a.jaeger)))
args := append(a.jaeger.Spec.Agent.Options.ToArgs())

if !util.HasArg("--reporter-type", args) {
args = append(args, "--reporter.type=grpc")

// we only add the grpc host if we are adding the reporter type and there's no explicit value yet
if !util.HasArg("--reporter.grpc.host-port", args) {
args = append(args, fmt.Sprintf("--reporter.grpc.host-port=dns:///%s.%s:14250", service.GetNameForCollectorService(a.jaeger), a.jaeger.Namespace))
}
}

trueVar := true
labels := a.labels()

Expand Down
15 changes: 11 additions & 4 deletions pkg/inject/sidecar.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
"github.com/jaegertracing/jaeger-operator/pkg/deployment"
"github.com/jaegertracing/jaeger-operator/pkg/service"
"github.com/jaegertracing/jaeger-operator/pkg/util"
)

var (
Expand Down Expand Up @@ -87,10 +88,16 @@ func Select(target *appsv1.Deployment, availableJaegerPods *v1.JaegerList) *v1.J
}

func container(jaeger *v1.Jaeger) corev1.Container {
args := append(jaeger.Spec.Agent.Options.ToArgs(),
"--reporter.type=grpc",
fmt.Sprintf("--reporter.grpc.host-port=dns:///%s.%s:14250", service.GetNameForCollectorService(jaeger), jaeger.Namespace),
)
args := append(jaeger.Spec.Agent.Options.ToArgs())

if !util.HasArg("--reporter-type", args) {
args = append(args, "--reporter.type=grpc")

// we only add the grpc host if we are adding the reporter type and there's no explicit value yet
if !util.HasArg("--reporter.grpc.host-port", args) {
args = append(args, fmt.Sprintf("--reporter.grpc.host-port=dns:///%s.%s:14250", service.GetNameForCollectorService(jaeger), jaeger.Namespace))
}
}

// ensure we have a consistent order of the arguments
// see https://github.com/jaegertracing/jaeger-operator/issues/334
Expand Down
5 changes: 3 additions & 2 deletions pkg/inject/sidecar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,12 @@ func TestSidecarOrderOfArguments(t *testing.T) {
dep = Sidecar(jaeger, dep)

assert.Len(t, dep.Spec.Template.Spec.Containers, 2)
assert.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 4)
assert.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 5)
assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[1].Args[0], "--a-option"))
assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[1].Args[1], "--b-option"))
assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[1].Args[2], "--c-option"))
assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[1].Args[3], "--collector.host-port"))
assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[1].Args[3], "--reporter.grpc.host-port"))
assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[1].Args[4], "--reporter.type"))
}

func dep(annotations map[string]string, labels map[string]string) *appsv1.Deployment {
Expand Down
11 changes: 11 additions & 0 deletions pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,14 @@ func GetEsHostname(opts map[string]string) string {
urlArr := strings.Split(urls, ",")
return urlArr[0]
}

// HasArg returns whether the slice has an entry prefixed by the given name
func HasArg(name string, args []string) bool {
for _, v := range args {
if strings.HasPrefix(v, name) {
return true
}
}

return false
}
10 changes: 10 additions & 0 deletions pkg/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,13 @@ func TestLabels(t *testing.T) {
"app.kubernetes.io/managed-by": "jaeger-operator",
}, Labels("joe", "leg", *v1.NewJaeger("thatone")))
}

func TestHasArgs(t *testing.T) {
args := []string{
"--a-option=a-value",
"--b-option=b-value",
}

assert.True(t, HasArg("--a-option", args))
assert.False(t, HasArg("--c-option", args))
}

0 comments on commit bd1518f

Please sign in to comment.