-
Notifications
You must be signed in to change notification settings - Fork 357
[jaeger] Major refactoring of templates to simplify #702
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
There were still CI issues, but this push should fix it CI Fix - Restore Provisioned Elasticsearch Logic & Stability
|
- Version: Changed from 5.0.0 to 4.2.0 per reviewer request - Added experimental/unstable warning to README and NOTES.txt - Removed cassandra-schema.yaml (Jaeger v2 handles schema internally) - Removed schema: section from values.yaml - Refactored storage-configmap.yaml to use blob-style config - Updated README with native Jaeger/OTEL config examples - Clarified Cassandra support status in documentation Signed-off-by: Jonah Kowall <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a major refactoring of the Jaeger Helm chart, transitioning from version 4.1.5 to what is described as version 5.0.0 in the PR description. The refactoring simplifies the chart architecture by consolidating components and removing deprecated features to provide a more streamlined deployment experience.
Key Changes:
- Unified deployment model using only the "All-in-One" architecture, removing split deployment options
- Removal of HotROD example application and legacy Elasticsearch DSL configuration
- Template consolidation for Elasticsearch maintenance jobs, Spark dependencies, and Cassandra schema into fewer files
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/jaeger/Chart.yaml | Chart version bump to 4.2.0 (though PR description indicates 5.0.0) |
| charts/jaeger/values.yaml | Removed hotrod, schema, and legacy elasticsearch configuration sections; updated elasticsearch volumeClaimTemplate |
| charts/jaeger/README.md | Completely rewritten documentation for v4.2.0 focusing on All-in-One architecture |
| charts/jaeger/templates/NOTES.txt | Updated with experimental warning and simplified installation notes |
| charts/jaeger/templates/service.yaml | New unified service consolidating agent, collector, and query services |
| charts/jaeger/templates/es-maintenance.yaml | New consolidated template combining index cleaner, rollover, and lookback CronJobs with their ServiceAccounts |
| charts/jaeger/templates/spark-dependencies.yaml | Consolidated Spark CronJob with ServiceAccount; added conditional guards for volumeMounts |
| charts/jaeger/templates/_helpers.tpl | Removed legacy helper functions (elasticsearch.client.url, hotrod helpers, schema helpers); refactored elasticsearch.env helper |
| charts/jaeger/templates/storage-configmap.yaml | Updated to use helper functions for configuration sections and added cassandra storage type condition |
| charts/jaeger/templates/hotrod-*.yaml | Removed all HotROD related templates (deployment, service, ingress, service account) |
| charts/jaeger/templates/es-*-sa.yaml | Removed separate ServiceAccount files for ES maintenance jobs (now in es-maintenance.yaml) |
| charts/jaeger/templates/es-*-cronjob.yaml | Removed separate CronJob files for ES maintenance (now in es-maintenance.yaml) |
| charts/jaeger/templates/allinone-*-svc.yaml | Removed separate agent, collector, and query services (replaced by unified service.yaml) |
| charts/jaeger/templates/cassandra-schema-*.yaml | Removed Cassandra schema job and ServiceAccount (Jaeger v2 handles schema internally) |
| charts/jaeger/templates/elasticsearch-secret.yaml | Removed Elasticsearch secret template |
| Makefile | Updated test-es target with additional elasticsearch readiness probe configuration |
| .vscode/settings.json | Added VS Code configuration file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove .vscode/ folder and add to .gitignore - Add 'Overriding the Jaeger Version' documentation section - Fix Cassandra wording: clarify chart doesn't provision Cassandra - Add link to Spark Dependencies README for env vars - Simplify storage-configmap.yaml condition per review feedback Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
- Version: Changed from 5.0.0 to 4.2.0 per reviewer request - Added experimental/unstable warning to README and NOTES.txt - Removed cassandra-schema.yaml (Jaeger v2 handles schema internally) - Removed schema: section from values.yaml - Refactored storage-configmap.yaml to use blob-style config - Updated README with native Jaeger/OTEL config examples - Clarified Cassandra support status in documentation Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
- Remove duplicate arguments for: - `jaeger-es-index-cleaner` (`numberOfDays`) - `jaeger-es-rollover` (`rollover`) - `jaeger-es-lookback` (`lookback`) - Update `elasticsearch.env` helper to generate environment variables when `storage.type` is `elasticsearch`, enabling support for external Elasticsearch clusters. Signed-off-by: Jonah Kowall <[email protected]>
- Remove .vscode/ folder and add to .gitignore - Add 'Overriding the Jaeger Version' documentation section - Fix Cassandra wording: clarify chart doesn't provision Cassandra - Add link to Spark Dependencies README for env vars - Simplify storage-configmap.yaml condition per review feedback Signed-off-by: Jonah Kowall <[email protected]>
Per review feedback, removed parameterization helpers and now render the entire .Values.config section as YAML directly instead of cherry-picking subsections. This is simpler and more maintainable. Removed unused helpers: - jaeger.extensionsConfig - jaeger.receiversConfig - jaeger.processorsConfig - jaeger.exportersConfig - jaeger.serviceExtensions - jaeger.serviceReceivers - jaeger.serviceProcessors - jaeger.serviceExporters Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
Added clear section headers and comments to distinguish: - storage.* - connection settings for external storage backends - cassandra/elasticsearch (top-level) - subchart values for provisioning Signed-off-by: Jonah Kowall <[email protected]>
Explain that ES config is now in config.extensions.jaeger_storage and that storage.elasticsearch is only used by ES maintenance jobs. Signed-off-by: Jonah Kowall <[email protected]>
BREAKING CHANGE: The 'allInOne' values section is now 'jaeger'. Migration: Replace 'allInOne:' with 'jaeger:' in your values files. - Renamed allInOne section to jaeger in values.yaml - Renamed template files from allinone-* to jaeger-* - Updated all template references - Updated README.md examples Signed-off-by: Jonah Kowall <[email protected]>
[jaeger] Major refactoring for Jaeger v2
Summary
This PR modernizes the Jaeger Helm chart for Jaeger v2, which is built on the OpenTelemetry Collector framework. It removes legacy v1 components and consolidates to a single deployment model with simplified, more maintainable templates.
Breaking Changes⚠️
Values Schema Changes
allInOne:→jaeger:- The main deployment section has been renamedschema:section removed - Cassandra schema job no longer needed (Jaeger v2 handles schema internally)hotrod:section removed - Demo app removed from chartconfig.extensions.jaeger_storageusing native OTel Collector syntaxMigration Guide
For storage configuration, see the updated README.md examples.
Changes
Templates Removed (Legacy v1 Components)
allinone-agent-svc.yaml,allinone-collector-svc.yaml,allinone-query-svc.yaml- Separate servicescassandra-schema-job.yaml&cassandra-schema-sa.yaml- Schema initializationelasticsearch-secret.yaml- ES secret managementes-index-cleaner-cronjob.yaml,es-rollover-cronjob.yaml,es-lookback-cronjob.yaml& their SA files - Now in es-maintenance.yamlhotrod-*.yaml- Demo applicationspark-sa.yaml- Merged into spark-dependencies.yamlTemplates Renamed
jaeger-sa.yamljaeger-ing.yamljaeger-configmap.yamlspark-cronjob.yaml→spark-dependencies.yamlTemplates Added/Consolidated
es-maintenance.yaml- Consolidated ES maintenance jobs with their ServiceAccountsHelper Functions Cleaned Up (_helpers.tpl)
Removed ~100 lines of unused helpers including schema, hotrod, and legacy ES helpers.
values.yaml Improvements
storage.*(connection settings) from top-levelcassandra:/elasticsearch:(subchart provisioning)Documentation
Bug Fixes
Values.configdirectlyStats
Testing
helm lint charts/jaegerpassesmake testpasses (deploys to local k8s)Checklist