Migration to 1.22 missed Zipkin port change

[gsagula]

Describe the bug
Starting on jaegertracing/jaeger-operator:1.22.1, jaeger-collector does not start Zipkin server automatically.

{"level":"info","ts":1619421380.7131286,"caller":"server/zipkin.go:48","msg":"Not listening for Zipkin HTTP traffic, port not configured"}

CR

apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
  name: mon-jaeger
  namespace: observability
spec:
  log-level: debug
  strategy: streaming
  sampling:
    options:
      default_strategy:
        type: const
        param: 100 
  collector:
    options:
      kafka: 
        producer:
          topic: jaeger-spans
          brokers: knative-kafka-cluster.knative-kafka:9092
  ingester:
    options:
      kafka:
        consumer:
          topic: jaeger-spans
          brokers: knative-kafka-cluster.knative-kafka:9092
      ingester:
        deadlockInterval: 0s
  storage:
    type: elasticsearch
    options:
      es:
        server-urls: https://elastic-es-http:9200
        index-prefix: jaeger-tracing
        tls:
          ca: /es/certificates/ca.crt
    secretName: jaeger-secret
  volumeMounts:
    - name: certificates
      mountPath: /es/certificates/
      readOnly: true
  volumes:
    - name: certificates
      secret:
        secretName: elastic-es-http-certs-public

Discussion
jaegertracing/jaeger#2959

To Reproduce
Steps to reproduce the behavior:

1. Install operator jaegertracing/jaeger-operator:1.22.1 with log-level: debug
2. Fire a valid CR to deploy Jaeger.
3. Grab the collector's logs

You should see the message "Not listening for Zipkin HTTP traffic, port not configured" and Jaeger does not take any Zipking trace.

Expected behavior
Collector should start Zipkin server automatically.

Version (please complete the following information):

• OS: Ubuntu 20.04
• Jaeger version: 1.22.1
• Deployment: Kubernetes - 1.18.16-gke.502

What troubleshooting steps did you try?
The same CR works with the previous version jaegertracing/jaeger-operator:1.21.3.

{"level":"info","ts":1619449101.7558992,"caller":"server/zipkin.go:49","msg":"Listening for Zipkin HTTP traffic","zipkin host-port":":9411"} 

cc @jpkrohling

[jpkrohling]

@rubenvp8510, could you please take a look at this one?

[jeremyzahner]

@jpkrohling @rubenvp8510 Debugged this quickly because i ran into the same problem.
Seems like the Operator still reconciles the deployments with the env variable COLLECTOR_ZIPKIN_HTTP_PORT.
Since this (or better, the underlying arg --collector.zipkin.http-port) has been changed (in https://github.com/jaegertracing/jaeger/releases/tag/v1.22.0) to ...host-port instead, this is probably what's causing the issue.

[jeremyzahner]

@gsagula You can fix it (for now) by using the following spec:

apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
  name: tracing
spec:
  ...
  collector:
    options:
      collector.zipkin.host-port: ':9411'
     ...

(I've only verified this using the allInOne strategy.)

[gsagula]

Thanks @jeremyzahner

I did that but with "9411" instead of ":9411". It's working now!

[jpkrohling]

Would one of you be interested in contributing a fix? Looks like we should have migrated the Zipkin parameter and value. This is where the code needs to be added:

jaeger-operator/pkg/upgrade/v1_22_0.go

Lines 11 to 40 in 5e9b39e

	func upgrade1_22_0(ctx context.Context, client client.Client, jaeger v1.Jaeger) (v1.Jaeger, error) {
	flagMapCollector := []deprecationFlagMap{{
	from: "jaeger.tags",
	to: "collector.tags",
	}}
	flagMapAgent := []deprecationFlagMap{{
	from: "jaeger.tags",
	to: "agent.tags",
	}}
	flagMapQuery := []deprecationFlagMap{
	{
	from: "downsampling.hashsalt",
	to: "",
	},
	{
	from: "downsampling.ratio",
	to: "",
	},
	}
	j := &jaeger
	j.Spec.AllInOne.Options = migrateDeprecatedOptions(j, j.Spec.AllInOne.Options, flagMapCollector)
	j.Spec.Collector.Options = migrateDeprecatedOptions(j, j.Spec.Collector.Options, flagMapCollector)
	j.Spec.Agent.Options = migrateDeprecatedOptions(j, j.Spec.Agent.Options, flagMapAgent)
	j.Spec.Query.Options = migrateDeprecatedOptions(j, j.Spec.Query.Options, flagMapQuery)
	return migrateCassandraVerifyFlagv1_22_0(jaeger), nil
	}

[gsagula]

@jpkrohling You can assign this issue to me if nobody else is taking it.

[jpkrohling]

Thanks for volunteering, @gsagula!

[gsagula]

Sorry guys, I just started looking into this one today but I see that @rubenvp8510 already did it.

[jpkrohling]

That's ok, we have plenty of other issues if you are interested :-)

[gsagula]

Awesome, I can give this one a shot if nobody else is taking it. #1422