How to set env for Jaeger

[clyang82]

I want to add base path for jaeger query (jaegertracing/jaeger-ui#258) so that the yaml file should be:

apiVersion: io.jaegertracing/v1alpha1
kind: Jaeger
metadata:
  name: istio-tracing
spec:
  strategy: production
  env:
    - name: QUERY_BASE_PATH
      value: /jaeger
  storage:
    type: elasticsearch
    options:
      es:
        server-urls: http://elasticsearch:9200

Right now, the jaeger operator cannot support set env in yaml file.

[clyang82]

We need this enhancement to enable jaeger-operator can be used in the Istio helm chart - istio/istio#9508

[objectiser]

Rather than express as an environment variable, as discussed in #72 it would be good to have this defined as:

apiVersion: io.jaegertracing/v1alpha1
kind: Jaeger
metadata:
  name: typed-options
spec:
  strategy: production
  query:
    base-path: /jaeger

[clyang82]

@objectiser it will increase the learning curve, because it is different from the original way. I do think we should provide a general way instead of adding one-by-one. What do you think?

[objectiser]

@clyang82 I think both approaches have their pros and cons - however I prefer the typed approach as:

• it means a CR would be rejected if it had incorrect content - whereas errors in free form options or env entries would not be detected until attempting to deploy the Jaeger components.
• the operator should not be exposing (and passing through) all of the config complexities - it should provide the simplest possible configuration to the end user, and the operator handle the complexity.

[clyang82]

@objectiser agree with your point - both approaches have their pros and cons. We need to consider the goal for the operator, it should not expose all of the config complexities, it should be handled by operator and provide a easy to use way. in this way, it will be used by more and more people. But the operator cannot handle all of requirements, otherwise, the operator will be very heavy. The operator should keep flexible. the operator should not introduce new configs since there always has well-known configs. Thanks.

[clyang82]

@jpkrohling what is your point? to provide env just like options or more specific pre-defined values? Thanks.

[jpkrohling]

I discussed with @objectiser yesterday and we'll refrain from implementing #72 for now, until we have a better understanding of the pros/cons and requirements.

For this one, I think the best solution isn't via env var or typed value, but to use options as it's used for other components. The query.base-path option can be used instead of the env var QUERY_BASE_PATH, similar to what you did with the ES options:

apiVersion: io.jaegertracing/v1alpha1
kind: Jaeger
metadata:
  name: istio-tracing
spec:
  strategy: production
  query:
    options:
      query:
        base-path: /jaeger
  storage:
    type: elasticsearch
    options:
      es:
        server-urls: http://elasticsearch:9200

[jpkrohling]

I'm closing this, as I think the solution from my previous comment should be sufficient. If this does not fix the problem for you, feel free to reopen this.

[clyang82]

@jpkrohling Thanks for your solution. It can fix my problem.

[jungopro]

guys, this is great and worked for me as well, thank you!
on a related topic, is there a way to modify the corresponding ingress object that gets created to include path: <base-path> as well?

We would like to expose it via ingress route /jaeger and currently we implemented the ingress object ourselves and set ingress to false inside the jaeger object:

kind: ingress
...
spec:
  rules:
  - host: app.domain.co.uk
     http:
       paths:
       - backend:
            serviceName: jaeger-query
            servicePort: 16686
         path: /jaeger
---
kind: jaeger
...
spec:
  ingress:
    enabled: false

[jpkrohling]

@jungopro, setting the base path should have an effect in the ingress path as well:

jaeger-operator/pkg/ingress/query_test.go

Lines 47 to 64 in 01bb012

	func TestQueryIngressAllInOneBasePath(t *testing.T) {
	enabled := true
	name := "TestQueryIngressAllInOneBasePath"
	jaeger := v1.NewJaeger(types.NamespacedName{Name: name})
	jaeger.Spec.Ingress.Enabled = &enabled
	jaeger.Spec.Strategy = "allInOne"
	jaeger.Spec.AllInOne.Options = v1.NewOptions(map[string]interface{}{"query.base-path": "/jaeger"})
	ingress := NewQueryIngress(jaeger)
	dep := ingress.Get()
	assert.NotNil(t, dep)
	assert.Nil(t, dep.Spec.Backend)
	assert.Len(t, dep.Spec.Rules, 1)
	assert.Len(t, dep.Spec.Rules[0].HTTP.Paths, 1)
	assert.Equal(t, "/jaeger", dep.Spec.Rules[0].HTTP.Paths[0].Path)
	assert.NotNil(t, dep.Spec.Rules[0].HTTP.Paths[0].Backend)
	}

Are you seeing something different?

[jungopro]

thanks @jpkrohling. I'm using the prod strategy, not the allInOne, but I do see the change reflected. I didn't notice that before, that is helpful

But, nothing is easy 😄
I'm getting 404 not found from the ingress-controller.
Is it possible that the controller didn't pickup the ingress rule?
Normally I would add annotations to the ingress rule for that to work:

nginx.ingress.kubernetes.io/ingress.class: nginx
nginx.ingress.kubernetes.io/rewrite-target: /<the-base-path>

I tried editing the ingress rule post-creation and add them manually but that didn't help either... I'm a bit lost to be honest. All I know is that with a stand-alone ingress rule it does work, I'm unsure if I'm missing something or simply utterly confused 😄

Thanks

[jpkrohling]

Could you open a new issue, ideally reproducible with minikube? I can take a look once we clear some other issues.

[jungopro]

will do. thanks.

[mohit-chawla]

@jpkrohling if use the following strategy with minikube, the base path is not reflected.

apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
  name: jaeger-tracing
spec:
  strategy: allInOne
  query:
    options:
      query:
        base-path: /jaeger

output of kubectl get ingress

NAME                   HOSTS   ADDRESS          PORTS   AGE
jaeger-tracing-query   *       192.168.99.104   80      2m54s

output of kubectl describe ingress jaeger-tracing-query

Name:             jaeger-tracing-query
Namespace:        default
Address:          192.168.99.104
Default backend:  jaeger-tracing-query:16686 (172.17.0.28:16686)
Rules:
  Host  Path  Backends
  ----  ----  --------
  *     *     jaeger-tracing-query:16686 (172.17.0.28:16686)
Annotations:
Events:
  Type    Reason  Age    From                      Message
  ----    ------  ----   ----                      -------
  Normal  CREATE  2m59s  nginx-ingress-controller  Ingress default/jaeger-tracing-query
  Normal  UPDATE  2m41s  nginx-ingress-controller  Ingress default/jaeger-tracing-query

It looks like the base path is not set at all. (ideally, correct me if i am wrong but Path should be '/jaeger' instead of *)

[jpkrohling]

@mohit-chawla could you try this instead:

apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
  name: jaeger-tracing
spec:
  strategy: allInOne
  allInOne:
    options:
      query:
        base-path: /jaeger

[mohit-chawla]

@jpkrohling , this worked, thanks.