Sorted the container arguments inside deployments

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

pkg/deployment/agent.go

@@ -2,6 +2,7 @@ package deployment
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/spf13/viper"
@@ -52,6 +53,10 @@ func (a *Agent) Get() *appsv1.DaemonSet {
 
 	commonSpec := util.Merge([]v1.JaegerCommonSpec{a.jaeger.Spec.Agent.JaegerCommonSpec, a.jaeger.Spec.JaegerCommonSpec, baseCommonSpec})
 
+	// ensure we have a consistent order of the arguments
+	// see https://github.com/jaegertracing/jaeger-operator/issues/334
+	sort.Strings(args)
+
 	return &appsv1.DaemonSet{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "apps/v1",

pkg/deployment/agent_test.go

@@ -2,6 +2,7 @@ package deployment
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/spf13/viper"
@@ -130,3 +131,26 @@ func TestAgentLabels(t *testing.T) {
 	assert.Equal(t, a.jaeger.Name, dep.Spec.Template.Labels["app.kubernetes.io/instance"])
 	assert.Equal(t, fmt.Sprintf("%s-agent", a.jaeger.Name), dep.Spec.Template.Labels["app.kubernetes.io/name"])
 }
+
+func TestAgentOrderOfArguments(t *testing.T) {
+	jaeger := v1.NewJaeger("TestAgentOrderOfArguments")
+	jaeger.Spec.Agent.Strategy = "daemonset"
+	jaeger.Spec.Agent.Options = v1.NewOptions(map[string]interface{}{
+		"b-option": "b-value",
+		"a-option": "a-value",
+		"c-option": "c-value",
+	})
+
+	a := NewAgent(jaeger)
+	dep := a.Get()
+
+	assert.Len(t, dep.Spec.Template.Spec.Containers, 1)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 5)
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[0], "--a-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[1], "--b-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[2], "--c-option"))
+
+	// the following are added automatically
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[3], "--reporter.grpc.host-port"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[4], "--reporter.type"))
+}

pkg/deployment/all-in-one.go

@@ -2,6 +2,7 @@ package deployment
 
 import (
 	"fmt"
+	"sort"
 
 	"github.com/spf13/viper"
 	appsv1 "k8s.io/api/apps/v1"
@@ -54,6 +55,10 @@ func (a *AllInOne) Get() *appsv1.Deployment {
 	configmap.Update(a.jaeger, commonSpec, &options)
 	sampling.Update(a.jaeger, commonSpec, &options)
 
+	// ensure we have a consistent order of the arguments
+	// see https://github.com/jaegertracing/jaeger-operator/issues/334
+	sort.Strings(options)
+
 	var envFromSource []corev1.EnvFromSource
 	if len(a.jaeger.Spec.Storage.SecretName) > 0 {
 		envFromSource = append(envFromSource, corev1.EnvFromSource{

pkg/deployment/all-in-one_test.go

@@ -1,6 +1,7 @@
 package deployment
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/spf13/viper"
@@ -240,3 +241,24 @@ func TestAllInOneLabels(t *testing.T) {
 	assert.Equal(t, a.jaeger.Name, dep.Spec.Template.Labels["app.kubernetes.io/instance"])
 	assert.Equal(t, a.jaeger.Name, dep.Spec.Template.Labels["app.kubernetes.io/name"])
 }
+
+func TestAllInOneOrderOfArguments(t *testing.T) {
+	jaeger := v1.NewJaeger("TestAllInOneOrderOfArguments")
+	jaeger.Spec.AllInOne.Options = v1.NewOptions(map[string]interface{}{
+		"b-option": "b-value",
+		"a-option": "a-value",
+		"c-option": "c-value",
+	})
+
+	a := NewAllInOne(jaeger)
+	dep := a.Get()
+
+	assert.Len(t, dep.Spec.Template.Spec.Containers, 1)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 4)
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[0], "--a-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[1], "--b-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[2], "--c-option"))
+
+	// the following are added automatically
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[3], "--sampling.strategies-file"))
+}

pkg/deployment/collector.go

@@ -2,6 +2,7 @@ package deployment
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/spf13/viper"
@@ -82,6 +83,10 @@ func (c *Collector) Get() *appsv1.Deployment {
 
 	sampling.Update(c.jaeger, commonSpec, &options)
 
+	// ensure we have a consistent order of the arguments
+	// see https://github.com/jaegertracing/jaeger-operator/issues/334
+	sort.Strings(options)
+
 	return &appsv1.Deployment{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "apps/v1",

pkg/deployment/collector_test.go

@@ -2,6 +2,7 @@ package deployment
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/spf13/viper"
@@ -422,3 +423,24 @@ func TestCollectorWithIngesterNoOptionsStorageType(t *testing.T) {
 	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 2)
 	assert.Equal(t, "--kafka.brokers=http://brokers", dep.Spec.Template.Spec.Containers[0].Args[0])
 }
+
+func TestCollectorOrderOfArguments(t *testing.T) {
+	jaeger := v1.NewJaeger("TestCollectorOrderOfArguments")
+	jaeger.Spec.Collector.Options = v1.NewOptions(map[string]interface{}{
+		"b-option": "b-value",
+		"a-option": "a-value",
+		"c-option": "c-value",
+	})
+
+	a := NewCollector(jaeger)
+	dep := a.Get()
+
+	assert.Len(t, dep.Spec.Template.Spec.Containers, 1)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 4)
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[0], "--a-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[1], "--b-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[2], "--c-option"))
+
+	// the following are added automatically
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[3], "--sampling.strategies-file"))
+}

pkg/deployment/ingester.go

@@ -2,6 +2,7 @@ package deployment
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/spf13/viper"
@@ -76,6 +77,10 @@ func (i *Ingester) Get() *appsv1.Deployment {
 		i.jaeger.Spec.Storage.Options.Filter(storage.OptionsPrefix(i.jaeger.Spec.Storage.Type)),
 		i.jaeger.Spec.Storage.Options.Filter("kafka"))
 
+	// ensure we have a consistent order of the arguments
+	// see https://github.com/jaegertracing/jaeger-operator/issues/334
+	sort.Strings(options)
+
 	return &appsv1.Deployment{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "apps/v1",

pkg/deployment/ingester_test.go

@@ -2,6 +2,7 @@ package deployment
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/spf13/viper"
@@ -334,9 +335,9 @@ func TestIngesterWithStorageType(t *testing.T) {
 	}
 	assert.Equal(t, envvars, dep.Spec.Template.Spec.Containers[0].Env)
 	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 3)
-	assert.Equal(t, "--kafka.topic=mytopic", dep.Spec.Template.Spec.Containers[0].Args[0])
-	assert.Equal(t, "--es.server-urls=http://somewhere", dep.Spec.Template.Spec.Containers[0].Args[1])
-	assert.Equal(t, "--kafka.brokers=http://brokers", dep.Spec.Template.Spec.Containers[0].Args[2])
+	assert.Equal(t, "--es.server-urls=http://somewhere", dep.Spec.Template.Spec.Containers[0].Args[0])
+	assert.Equal(t, "--kafka.brokers=http://brokers", dep.Spec.Template.Spec.Containers[0].Args[1])
+	assert.Equal(t, "--kafka.topic=mytopic", dep.Spec.Template.Spec.Containers[0].Args[2])
 }
 
 func TestIngesterLabels(t *testing.T) {
@@ -348,6 +349,24 @@ func TestIngesterLabels(t *testing.T) {
 	assert.Equal(t, fmt.Sprintf("%s-ingester", ingester.jaeger.Name), dep.Spec.Template.Labels["app.kubernetes.io/name"])
 }
 
+func TestIngesterOrderOfArguments(t *testing.T) {
+	jaeger := newIngesterJaeger("TestIngesterOrderOfArguments")
+	jaeger.Spec.Ingester.Options = v1.NewOptions(map[string]interface{}{
+		"b-option": "b-value",
+		"a-option": "a-value",
+		"c-option": "c-value",
+	})
+
+	a := NewIngester(jaeger)
+	dep := a.Get()
+
+	assert.Len(t, dep.Spec.Template.Spec.Containers, 1)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 3)
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[0], "--a-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[1], "--b-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[2], "--c-option"))
+}
+
 func newIngesterJaeger(name string) *v1.Jaeger {
 	return &v1.Jaeger{
 		ObjectMeta: metav1.ObjectMeta{

pkg/deployment/query.go

@@ -2,6 +2,7 @@ package deployment
 
 import (
 	"fmt"
+	"sort"
 
 	"github.com/spf13/viper"
 	appsv1 "k8s.io/api/apps/v1"
@@ -79,6 +80,10 @@ func (q *Query) Get() *appsv1.Deployment {
 		})
 	}
 
+	// ensure we have a consistent order of the arguments
+	// see https://github.com/jaegertracing/jaeger-operator/issues/334
+	sort.Strings(options)
+
 	return &appsv1.Deployment{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "apps/v1",

pkg/deployment/query_test.go

@@ -2,6 +2,7 @@ package deployment
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/spf13/viper"
@@ -302,3 +303,21 @@ func TestQueryLabels(t *testing.T) {
 	assert.Equal(t, query.jaeger.Name, dep.Spec.Template.Labels["app.kubernetes.io/instance"])
 	assert.Equal(t, fmt.Sprintf("%s-query", query.jaeger.Name), dep.Spec.Template.Labels["app.kubernetes.io/name"])
 }
+
+func TestQueryOrderOfArguments(t *testing.T) {
+	jaeger := v1.NewJaeger("TestQueryOrderOfArguments")
+	jaeger.Spec.Query.Options = v1.NewOptions(map[string]interface{}{
+		"b-option": "b-value",
+		"a-option": "a-value",
+		"c-option": "c-value",
+	})
+
+	a := NewQuery(jaeger)
+	dep := a.Get()
+
+	assert.Len(t, dep.Spec.Template.Spec.Containers, 1)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[0].Args, 3)
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[0], "--a-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[1], "--b-option"))
+	assert.True(t, strings.HasPrefix(dep.Spec.Template.Spec.Containers[0].Args[2], "--c-option"))
+}

pkg/inject/oauth-proxy.go

@@ -32,18 +32,22 @@ func OAuthProxy(jaeger *v1.Jaeger, dep *appsv1.Deployment) *appsv1.Deployment {
 }
 
 func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container {
+	// keep this sorted!
+	// see https://github.com/jaegertracing/jaeger-operator/pull/337
+	args := []string{
+		"--cookie-secret=SECRET",
+		"--https-address=:8443",
+		fmt.Sprintf("--openshift-service-account=%s", account.OAuthProxyAccountNameFor(jaeger)),
+		"--provider=openshift",
+		"--tls-cert=/etc/tls/private/tls.crt",
+		"--tls-key=/etc/tls/private/tls.key",
+		"--upstream=http://localhost:16686",
+	}
+
 	return corev1.Container{
 		Image: viper.GetString("openshift-oauth-proxy-image"),
 		Name:  "oauth-proxy",
-		Args: []string{
-			"--https-address=:8443",
-			"--provider=openshift",
-			fmt.Sprintf("--openshift-service-account=%s", account.OAuthProxyAccountNameFor(jaeger)),
-			"--upstream=http://localhost:16686",
-			"--tls-cert=/etc/tls/private/tls.crt",
-			"--tls-key=/etc/tls/private/tls.key",
-			"--cookie-secret=SECRET",
-		},
+		Args:  args,
 		VolumeMounts: []corev1.VolumeMount{{
 			MountPath: "/etc/tls/private",
 			Name:      service.GetTLSSecretNameForQueryService(jaeger),

pkg/inject/oauth-proxy_test.go

@@ -2,6 +2,7 @@ package inject
 
 import (
 	"fmt"
+	"sort"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -54,3 +55,17 @@ func TestOAuthProxyConsistentServiceAccountName(t *testing.T) {
 	}
 	assert.True(t, found)
 }
+
+func TestOAuthProxyOrderOfArguments(t *testing.T) {
+	jaeger := v1.NewJaeger("TestOAuthProxyConsistentServiceAccountName")
+	jaeger.Spec.Ingress.Security = v1.IngressSecurityOAuthProxy
+	dep := OAuthProxy(jaeger, deployment.NewQuery(jaeger).Get())
+
+	sortedArgs := make([]string, len(dep.Spec.Template.Spec.Containers[1].Args))
+	copy(sortedArgs, dep.Spec.Template.Spec.Containers[1].Args)
+	sort.Strings(sortedArgs)
+
+	assert.Len(t, dep.Spec.Template.Spec.Containers, 2)
+	assert.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 7)
+	assert.Equal(t, sortedArgs, dep.Spec.Template.Spec.Containers[1].Args)
+}

pkg/inject/sidecar.go

@@ -2,6 +2,7 @@ package inject
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -87,6 +88,11 @@ func Select(target *appsv1.Deployment, availableJaegerPods *v1.JaegerList) *v1.J
 
 func container(jaeger *v1.Jaeger) corev1.Container {
 	args := append(jaeger.Spec.Agent.Options.ToArgs(), fmt.Sprintf("--collector.host-port=%s.%s:14267", service.GetNameForCollectorService(jaeger), jaeger.Namespace))
+
+	// ensure we have a consistent order of the arguments
+	// see https://github.com/jaegertracing/jaeger-operator/issues/334
+	sort.Strings(args)
+
 	return corev1.Container{
 		Image: jaeger.Spec.Agent.Image,
 		Name:  "jaeger-agent",

pkg/inject/sidecar_test.go

@@ -1,6 +1,7 @@
 package inject
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/spf13/viper"
@@ -211,6 +212,25 @@ func TestSelectBasedOnName(t *testing.T) {
 	assert.Equal(t, "the-second-jaeger-instance-available", jaeger.Name)
 }
 
+func TestSidecarOrderOfArguments(t *testing.T) {
+	jaeger := v1.NewJaeger("TestQueryOrderOfArguments")
+	jaeger.Spec.Agent.Options = v1.NewOptions(map[string]interface{}{
+		"b-option": "b-value",
+		"a-option": "a-value",
+		"c-option": "c-value",
+	})
+
+	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.Len(t, dep.Spec.Template.Spec.Containers[1].Args, 4)
+	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"))
+}
+
 func dep(annotations map[string]string, labels map[string]string) *appsv1.Deployment {
 	return &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{