jaegertracing / helm-charts

Helm Charts for Jaeger backend
Apache License 2.0
267 stars 347 forks source link

[Feature]: Update helm-chart to use elasticsearch 8+ #532

Closed Stevenpc3 closed 4 months ago

Stevenpc3 commented 9 months ago

Requirement

As a user of the helm-chart I would like the chart.yaml dependency of Elasticsearch to be updated to version 8+ so that I can use the latest or current versions of Elasticsearch with Jaeger.

Problem

Jeager was recently updated to support Elasticsearch version 8+ https://github.com/jaegertracing/jaeger/releases/tag/v1.52.0

But the chart still points to version 7.11.1 https://github.com/jaegertracing/helm-charts/blob/main/charts/jaeger/Chart.yaml#L35

Blindly updating this likely will not work since the values.yaml settings are for the older verison and the index values have changed so things like "esIndexCleaner" and "esRollover" and "esLookback" will need to be updated to work with the new version.

Proposal

Someone tests the newer version of Elasticsearch and modifies the values.yaml and templates to properly clean and manage the new indexes.

Open questions

Is this currently planned somewhere else?

Stevenpc3 commented 9 months ago

Seeing how the old elasticsearch chart is deprecated and the suggestion is to use ECK now but has license issues to consider.

I suggest doing what we did when we needed to use elasticsearch outside of jaeger which is use the elasticsearch bitnami chart.

Ironically... we were going to use the bitnami jaeger chart but it is woefully under developed compared to other charts.

Stevenpc3 commented 8 months ago

helm charts were just updated to 1.53 https://github.com/jaegertracing/helm-charts/commit/94950b7c837ccdde786ffb3308132cc759a374ca.

This implies that I can use elasticsearch 8+ . Any work on ensuring this is true and updateing the chart to point to the new bitnami charts? Bitnami charts have changed so I assume jeager charts would require a number of updates. For example elasticsearch.esConfig is no longer avaialble in the bitnami charts

dpericaxon commented 5 months ago

@Stevenpc3 I took a crack at this and created https://github.com/jaegertracing/helm-charts/pull/554 if you want test out the version by checking my branch and generating a tar, that would be great! (Obviously not in production!)

Stevenpc3 commented 5 months ago

Awesome thanks! I'll try to get to it this week.

Did you get to test esIndexCleaner it other items? I guess they should be fine since those images are in the Jaeger repo and should have been updated to with with elastic 8.

Hopefully is this simple!

Stevenpc3 commented 4 months ago

So we finally got to test this and it appears to work and esIndexCleaner works. BUT...

This breaks spark. It will throw an error stating that it only supports version 7 of elasticsearch and requires hadoop.

dpericaxon commented 4 months ago

Hey @Stevenpc3 I think you're running into this open issue: https://github.com/jaegertracing/spark-dependencies/issues/137 Theres a suggestion there to make sure to "Use ghcr.io/jaegertracing/spark-dependencies/spark-dependencies:latest instead of the on on docker.io , which is outdated for ages." Maybe try that?

Stevenpc3 commented 4 months ago

Yeah I'll give it a go tomorrow. I'm just using the default from the chart. If that works then I guess the chart needs updating. This is pulling the image that Jaeger published. I see it was updated so should work https://github.com/jaegertracing/spark-dependencies/commit/a5e17365cecfd7650ef47a1fe26f617189095a77

If the registry defaults to docker then we likely should update the registry in the chart to point to the right place otherwise it will keep happening for everyone.

Update this https://github.com/jaegertracing/helm-charts/blob/main/charts/jaeger/values.yaml#L780 to be the correct registry. And also any others that require it so they don't default to the wrong registry.

Stevenpc3 commented 4 months ago

going to close this issue and keep the spark part in a different issue