[incubator/jaeger] Error with default chart settings

[sneko]

Version of Helm and Kubernetes:

Helm:
Client: &version.Version{SemVer:"v2.8.2", GitCommit:"a80231648a1473929271764b920a8e346f6de844", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.8.2", GitCommit:"a80231648a1473929271764b920a8e346f6de844", GitTreeState:"clean"}

Kubernetes:
Client Version: version.Info{Major:"1", Minor:"10", GitVersion:"v1.10.3", GitCommit:"2bba0127d85d5a46ab4b778548be28623b32d0b0", GitTreeState:"clean", BuildDate:"2018-05-21T09:17:39Z", GoVersion:"go1.9.3
", Compiler:"gc", Platform:"windows/amd64"}
Server Version: version.Info{Major:"1", Minor:"9", GitVersion:"v1.9.6", GitCommit:"9f8ebd171479bec0ada837d7ee641dec2f8c6dd1", GitTreeState:"clean", BuildDate:"2018-03-21T15:13:31Z", GoVersion:"go1.9.3",
Compiler:"gc", Platform:"linux/amd64"}

Which chart:

[incubator/jaeger]

What happened:

When trying to install the chart while provisioning a Cassandra datastore it fails with the following error:

Error: release jaeger failed: StatefulSet.apps "jaeger-cassandra" is invalid: [spec.selector: Required value, spec.template.metadata.labels: Invalid value: map[string]string{"app":"cassandra", "release":"jaeger"}: `selector` does not match template `labels`]

I was thinking it came from my custom settings so I also tried with the default values.yaml and I get the same error.

What you expected to happen:

That it works.

How to reproduce it (as minimally and precisely as possible):

helm install --name jaeger --namespace XXXXXXX -f values.yaml incubator/jaeger

with the default "values.yaml" present in the chart.

Thank you 😃

[jpkrohling]

cc @pavelnikolov , @dvonthenen , would you know what's going on and/or who could assist with this one?

[pavelnikolov]

I’m looking into it. I have the all-in-one PR almost done, but it seems that this needs to be handled separately. I’ll try to send a PR later today.

[jpkrohling]

@pavelnikolov would it make sense to send a PR to use the operator instead?

[cpanato]

@pavelnikolov let me know if you need any help

[davidvonthenen]

sorry @jpkrohling... I have been on vacation since Aug 24, but I am back now.

[pavelnikolov]

@jpkrohling Ideally, I want to use a separate chart for the operator jaeger-operator, WDYT?
ATM the chart supports 4 scenarios:

1. Provisioned Elasticsearch
2. Existing Elasticsearch
3. Provisioned Cassandra (default)
4. Existing Cassandara

I still haven't pushed the all-in-one, which would use in-memory storage. Do you think the operator would support all these cases?

[jpkrohling]

My initial idea was to have the Jaeger Helm Chart to allow users to specify a dependency on other storage charts, to get ES/Cassandra/Kafka running before Jaeger is installed. Then, it would just install the CRD and the operator (in case they aren't there yet) and build a CR to install individual Jaeger instances, which use the operator behind the scenes. So, instead of the chart building all the individual Deployment, Service, and other Kubernetes objects, it would just build a single kind: Jaeger object and let the operator handle how to best achieve the desired state.

This is a sample YAML that the chart could be building instead:

apiVersion: io.jaegertracing/v1alpha1
kind: Jaeger
metadata:
  name: simple-prod
spec:
  strategy: production
  storage:
    type: elasticsearch
    options:
      es:
        server-urls: http://elasticsearch:9200
        username: elastic
        password: changeme

Or, for a simple all-in-one:

apiVersion: io.jaegertracing/v1alpha1
kind: Jaeger
metadata:
  name: simplest

The jaeger-operator deals only with Jaeger and expects the backing storage to exist, in case the storage is not set to memory. My impression is that Helm can handle this kind of dependency management, so that it would run/install the Cassandra/ES chart as specified by the user.

The operator currently supports:

• all-in-one with all backing storage mechanisms (Kafka, ES, Cassandra, Memory)
• production with all backing storage mechanisms, except Memory

[pavelnikolov]

@jpkrohling ideally, I would like to keep the existing chart separate from the operator. It's totally fine to have more than one chart for a given application/system. There are plenty of operators and all of them have the -operator suffix (e.g. etcd-operator, kanister-operator, vault-operator. I would suggest following the same pattern to be consistent. It's ok to allow people to use both a chart that provisions everything end to end as well as operator which requires existing datastore. WDYT?

[jpkrohling]

I would suggest following the same pattern to be consistent

I wasn't aware of the extra chart for operators. In that case, I'm all for it!

[okgolove]

Same error during installing incubator/cassandra:
Error: release cassandra failed: StatefulSet.apps "cassandra" is invalid: [spec.selector: Required value, spec.template.metadata.labels: Invalid value: map[string]string{"app":"cassandra", "release":"cassandra"}: `selector` does not match template `labels`]

[lambertjosh]

@pavelnikolov are you still intending to work on this? Unfortunately it's blocking other PR's due to the CI failures.

[pavelnikolov]

Yes! I’ll try to push a fix today or tomorrow. I’m sorry for the delay.

[AntonAleksandrov13]

@pavelnikolov is there any update on this issue?

[maver1ck]

I think this error was fixed by #8320

[AntonAleksandrov13]

@maver1ck great, thanks!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

[stale]

This issue is being automatically closed due to inactivity.