Merge branch 'master' into Issue-1314

Signed-off-by: Ruben Vargas <[email protected]>
jaegertracing · May 26, 2021 · c7c57c4 · c7c57c4
2 parents 29af842 + 03f5722
commit c7c57c4
Show file tree

Hide file tree

Showing 7 changed files with 152 additions and 19 deletions.
diff --git a/pkg/deployment/agent_test.go b/pkg/deployment/agent_test.go
@@ -201,20 +201,20 @@ func TestAgentArgumentsOpenshiftTLS(t *testing.T) {
 	defer viper.Reset()
 
 	for _, tt := range []struct {
-		name       string
-		options    v1.Options
+		name         string
+		options      v1.Options
 		expectedArgs []string
 	}{
 		{
 			name: "Openshift CA",
 			options: v1.NewOptions(map[string]interface{}{
 				"a-option": "a-value",
 			}),
-			expectedArgs:[]string{
+			expectedArgs: []string{
 				"--a-option=a-value",
 				"--reporter.grpc.host-port=dns:///my-instance-collector-headless.test:14250",
 				"--reporter.grpc.tls.enabled=true",
-				"--reporter.grpc.tls.ca="+ca.ServiceCAPath,
+				"--reporter.grpc.tls.ca=" + ca.ServiceCAPath,
 				"--reporter.grpc.tls.server-name=my-instance-collector-headless.test.svc.cluster.local",
 			},
 		},
@@ -225,7 +225,7 @@ func TestAgentArgumentsOpenshiftTLS(t *testing.T) {
 				"reporter.grpc.tls.enabled": "true",
 				"reporter.grpc.tls.ca":      "/my/custom/ca",
 			}),
-			expectedArgs:[]string{
+			expectedArgs: []string{
 				"--a-option=a-value",
 				"--reporter.grpc.host-port=dns:///my-instance-collector-headless.test:14250",
 				"--reporter.grpc.tls.enabled=true",

diff --git a/pkg/deployment/all_in_one.go b/pkg/deployment/all_in_one.go
@@ -131,8 +131,8 @@ func (a *AllInOne) Get() *appsv1.Deployment {
 								Value: string(a.jaeger.Spec.Storage.Type),
 							},
 							{
-								Name:  "COLLECTOR_ZIPKIN_HTTP_PORT",
-								Value: "9411",
+								Name:  "COLLECTOR_ZIPKIN_HOST_PORT",
+								Value: ":9411",
 							},
 							{
 								Name:  "JAEGER_DISABLED",

diff --git a/pkg/deployment/all_in_one_test.go b/pkg/deployment/all_in_one_test.go
@@ -35,8 +35,8 @@ func TestDefaultAllInOneImage(t *testing.T) {
 			Value: "",
 		},
 		{
-			Name:  "COLLECTOR_ZIPKIN_HTTP_PORT",
-			Value: "9411",
+			Name:  "COLLECTOR_ZIPKIN_HOST_PORT",
+			Value: ":9411",
 		},
 		{
 			Name:  "JAEGER_DISABLED",

diff --git a/pkg/deployment/collector.go b/pkg/deployment/collector.go
@@ -128,8 +128,8 @@ func (c *Collector) Get() *appsv1.Deployment {
 								Value: string(storageType),
 							},
 							{
-								Name:  "COLLECTOR_ZIPKIN_HTTP_PORT",
-								Value: "9411",
+								Name:  "COLLECTOR_ZIPKIN_HOST_PORT",
+								Value: ":9411",
 							},
 						},
 						VolumeMounts: commonSpec.VolumeMounts,

diff --git a/pkg/deployment/collector_test.go b/pkg/deployment/collector_test.go
@@ -81,8 +81,8 @@ func TestDefaultCollectorImage(t *testing.T) {
 			Value: "",
 		},
 		{
-			Name:  "COLLECTOR_ZIPKIN_HTTP_PORT",
-			Value: "9411",
+			Name:  "COLLECTOR_ZIPKIN_HOST_PORT",
+			Value: ":9411",
 		},
 	}
 	assert.Equal(t, envvars, containers[0].Env)
@@ -336,8 +336,8 @@ func TestCollectorWithDirectStorageType(t *testing.T) {
 			Value: string(v1.JaegerESStorage),
 		},
 		{
-			Name:  "COLLECTOR_ZIPKIN_HTTP_PORT",
-			Value: "9411",
+			Name:  "COLLECTOR_ZIPKIN_HOST_PORT",
+			Value: ":9411",
 		},
 	}
 	assert.Equal(t, envvars, dep.Spec.Template.Spec.Containers[0].Env)
@@ -375,8 +375,8 @@ func TestCollectorWithKafkaStorageType(t *testing.T) {
 			Value: "kafka",
 		},
 		{
-			Name:  "COLLECTOR_ZIPKIN_HTTP_PORT",
-			Value: "9411",
+			Name:  "COLLECTOR_ZIPKIN_HOST_PORT",
+			Value: ":9411",
 		},
 	}
 	assert.Equal(t, envvars, dep.Spec.Template.Spec.Containers[0].Env)
@@ -410,8 +410,8 @@ func TestCollectorWithIngesterNoOptionsStorageType(t *testing.T) {
 			Value: "kafka",
 		},
 		{
-			Name:  "COLLECTOR_ZIPKIN_HTTP_PORT",
-			Value: "9411",
+			Name:  "COLLECTOR_ZIPKIN_HOST_PORT",
+			Value: ":9411",
 		},
 	}
 	assert.Equal(t, envvars, dep.Spec.Template.Spec.Containers[0].Env)

diff --git a/pkg/upgrade/v1_22_0.go b/pkg/upgrade/v1_22_0.go
@@ -36,6 +36,9 @@ func upgrade1_22_0(ctx context.Context, client client.Client, jaeger v1.Jaeger)
 	j.Spec.Agent.Options = migrateDeprecatedOptions(j, j.Spec.Agent.Options, flagMapAgent)
 	j.Spec.Query.Options = migrateDeprecatedOptions(j, j.Spec.Query.Options, flagMapQuery)
 
+	//Migrate query host/port flags
+	j.Spec.Query.Options = migrateQueryHostPortFlagsv1_22_0(j.Spec.Query.Options)
+
 	return migrateCassandraVerifyFlagv1_22_0(jaeger), nil
 }
 
@@ -59,3 +62,34 @@ func updateCassandraVerifyHostFlagv1_22_0(options v1.Options) v1.Options {
 	}
 	return v1.NewOptions(in)
 }
+
+func migrateQueryHostPortFlagsv1_22_0(options v1.Options) v1.Options {
+	in := options.GenericMap()
+
+	port, hasPortFlag := in["query.port"]
+	hostPort, hasHostPortFlag := in["query.host-port"]
+
+	_, hasGrpcFlag := in["query.grpc-server.host-port"]
+	_, hasHTTPFlag := in["query.http-server.host-port"]
+
+	newValue := ""
+	if hasPortFlag {
+		newValue = ":" + port.(string)
+	} else if hasHostPortFlag {
+		newValue = hostPort.(string)
+	}
+
+	if newValue != "" {
+		if !hasGrpcFlag {
+			in["query.grpc-server.host-port"] = newValue
+		}
+		if !hasHTTPFlag {
+			in["query.http-server.host-port"] = newValue
+		}
+	}
+
+	delete(in, "query.port")
+	delete(in, "query.host-port")
+
+	return v1.NewOptions(in)
+}
diff --git a/pkg/upgrade/v1_22_0_test.go b/pkg/upgrade/v1_22_0_test.go
@@ -158,3 +158,102 @@ func TestCassandraVerifyHostFlags(t *testing.T) {
 		})
 	}
 }
+
+func TestMigrateQueryHostPortFlagsv1_22_0(t *testing.T) {
+
+	tests := []struct {
+		testName    string
+		opts        v1.Options
+		expectedOps map[string]string
+	}{
+		{
+			testName: "no old flags",
+			opts: v1.NewOptions(map[string]interface{}{
+				"query.grpc-server.host-port": ":8080",
+				"query.http-server.host-port": ":8081",
+			}),
+			expectedOps: map[string]string{
+				"query.grpc-server.host-port": ":8080",
+				"query.http-server.host-port": ":8081",
+			},
+		},
+
+		{
+			testName: "both old flags",
+			opts: v1.NewOptions(map[string]interface{}{
+				"query.port":      "8080",
+				"query.host-port": "localhost:8081",
+			}),
+			expectedOps: map[string]string{
+				"query.grpc-server.host-port": ":8080",
+				"query.http-server.host-port": ":8080",
+			},
+		},
+
+		{
+			testName: "with query.host-port",
+			opts: v1.NewOptions(map[string]interface{}{
+				"query.host-port": "localhost:8081",
+			}),
+			expectedOps: map[string]string{
+				"query.grpc-server.host-port": "localhost:8081",
+				"query.http-server.host-port": "localhost:8081",
+			},
+		},
+		{
+			testName: "with grpc-server.host-port set",
+			opts: v1.NewOptions(map[string]interface{}{
+				"query.host-port":             "localhost:8081",
+				"query.grpc-server.host-port": "other:7777",
+			}),
+			expectedOps: map[string]string{
+				"query.grpc-server.host-port": "other:7777",
+				"query.http-server.host-port": "localhost:8081",
+			},
+		},
+		{
+			testName: "with grpc-server.host-port set and query.port",
+			opts: v1.NewOptions(map[string]interface{}{
+				"query.port":                  "8081",
+				"query.grpc-server.host-port": "other:7777",
+			}),
+			expectedOps: map[string]string{
+				"query.grpc-server.host-port": "other:7777",
+				"query.http-server.host-port": ":8081",
+			},
+		},
+		{
+			testName: "with grpc/http-server.host-port set",
+			opts: v1.NewOptions(map[string]interface{}{
+				"query.host-port":             "localhost:8081",
+				"query.grpc-server.host-port": "other:7777",
+				"query.http-server.host-port": "other:9999",
+			}),
+			expectedOps: map[string]string{
+				"query.grpc-server.host-port": "other:7777",
+				"query.http-server.host-port": "other:9999",
+			},
+		},
+	}
+	latestVersion := "1.22.0"
+	for _, tt := range tests {
+		nsn := types.NamespacedName{Name: "my-instance"}
+		existing := v1.NewJaeger(nsn)
+		existing.Status.Version = "1.21.0"
+		existing.Spec.Query.Options = tt.opts
+
+		objs := []runtime.Object{existing}
+		s := scheme.Scheme
+		s.AddKnownTypes(v1.SchemeGroupVersion, &v1.Jaeger{})
+		s.AddKnownTypes(v1.SchemeGroupVersion, &v1.JaegerList{})
+		cl := fake.NewFakeClient(objs...)
+		assert.NoError(t, ManagedInstances(context.Background(), cl, cl, latestVersion))
+
+		persisted := &v1.Jaeger{}
+		assert.NoError(t, cl.Get(context.Background(), nsn, persisted))
+		assert.Equal(t, latestVersion, persisted.Status.Version)
+		assert.Equal(t, tt.expectedOps, persisted.Spec.Query.Options.Map())
+
+	}
+
+}