Skip to content

fix(helm): align defaults with raster-core and split server/telemetry values - #40

Open
CL-SHLOMIKONCHA wants to merge 10 commits into
masterfrom
fix/helm-default-values
Open

CL-SHLOMIKONCHA wants to merge 10 commits into
masterfrom
fix/helm-default-values

Conversation

@CL-SHLOMIKONCHA

@CL-SHLOMIKONCHA CL-SHLOMIKONCHA commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
Question Answer
Bug fix
New feature
Breaking change
Deprecations
Documentation
Tests added
Chore

Related issues: raster-core umbrella chart TODOs (helm-charts-newcharts/raster-core/values.yaml, site-valuesraster/core/*), MapColonies/ts-server-boilerplate#476 (MAPCO-11622)

Further information:

Defaults (resolve the raster-core / site-values # TODO overrides for cleaner)

Key Before After
configManagement.offlineMode false true
jobnik.worker.concurrency missing 1
configManagement.version '' latest
configManagement.serverUrl '' http://localhost:8080/api
server.targetPort missing 8080
worker.capabilities.pairs Ingestion_Update, Ingestion_Swap_Update Ingestion_Update, Delete_Layer (both tiles-deletion)
storage.cleanupStorageProviders {} []
telemetry.tracing.enabled false true
telemetry.logger.opentelemetryOptions.enabled false true
ca.* chart scope merged with global.ca global.ca.* only (same defaults)

Server / telemetry split (mirrors ts-server-boilerplate#476)

  • The flat env block is replaced by a server scope (port, targetPort) and a telemetry scope (logger, tracing). The cleaner specific keys (queue, jobnik, worker, httpRetry, disableHttpClientLogs, strategies) stay under env unchanged.
  • Logger keys renamed to level / prettyPrint; logger.opentelemetryOptions holds enabled, url and optional resourceAttributes.
  • The configmap exports OPENTELEMETRY_LOGGING_ENABLED, OTLP_LOGGER_ENDPOINT_URL and OTEL_RESOURCE_ATTRIBUTES (key=value,key=value), the names the commonWorkerBoilerplateV2 schema and the OpenTelemetry SDK read. The previous TELEMETRY_LOGGER_OTEL_* names were not consumed by the schema. config/custom-environment-variables.json maps the same three names. Enabled flags are always exported, urls only when enabled.
  • Global overrides move under global.telemetry with the same hierarchy. Only the tracing and OTLP logger urls are overridable from global and take precedence over chart values; enabled flags stay chart-level. global.tracing, global.openTelemetryOptions and global.metrics are removed.
  • The metricsUrl, common.tracing.merged and common.openTelemetryOptions.merged helpers are removed.
  • The ca values move under global.ca (secretName, path, key) and the deployment reads them directly, like the other raster services; the chart-level ca scope and the common.ca.merged helper are removed.

Breaking for consumers: umbrella charts and site values setting env.*, global.tracing.* or global.openTelemetryOptions.* must move to server.*, telemetry.* and global.telemetry.*; env.queue, env.jobnik, env.worker, env.httpRetry, env.disableHttpClientLogs and env.strategies remain valid paths. In particular cleaner.env.openTelemetryOptions.url (site-values raster/core/base.yaml.gotmpl) becomes cleaner.telemetry.logger.opentelemetryOptions.url, and cleaner.env.tracing / cleaner.env.openTelemetryOptions / cleaner.env.targetPort / cleaner.env.jobnik / cleaner.env.worker in raster-core/values.yaml can be dropped since the defaults now match.

Notes for reviewers

  • The pairs default follows the raster-core override exactly, which drops Ingestion_Swap_Update. config/default.json still lists four pairs (including Ingestion_Swap_Update and Delete_Layer/artifacts-deletion). Please confirm which set the chart default should carry.
  • With resourceAttributes mapped as a plain string, the config object holds the raw key=value text; the attributes reach exported log records through the OpenTelemetry SDK, which reads OTEL_RESOURCE_ATTRIBUTES itself.

🤖 Generated with Claude Code

…emetry from global

- configManagement.offlineMode: true, version: latest
- env.targetPort: 8080 (deployment/probes already reference it)
- env.jobnik.worker.concurrency: 1 (configmap referenced a missing key)
- worker.capabilities.pairs: Ingestion_Update + Delete_Layer tiles-deletion
- env.tracing and env.openTelemetryOptions are now merged with
  global.tracing / global.openTelemetryOptions like the other raster services
- drop the unused cleaner.tracingUrl helper
CL-SHLOMIKONCHA and others added 2 commits September 10, 2026 13:45
Align the chart with ts-server-boilerplate:

- replace the flat env block with a server scope (port, targetPort) and a
  telemetry scope (logger, tracing); move the cleaner specific keys
  (queue, jobnik, worker, httpRetry, disableHttpClientLogs, strategies)
  to top-level scopes with the same names
- rename logger keys to level and prettyPrint to match the config schema
- export OPENTELEMETRY_LOGGING_ENABLED, OTLP_LOGGER_ENDPOINT_URL and
  OTEL_RESOURCE_ATTRIBUTES (key=value,key=value), the names the
  commonWorkerBoilerplateV2 schema and the OpenTelemetry SDK read, and
  map them in config/custom-environment-variables.json
- always export the enabled flags, urls only when enabled
- move global overrides under global.telemetry; only the tracing and
  otlp logger urls are overridable from global and take precedence over
  chart values, enabled flags stay chart-level
- enable tracing and opentelemetry logging by default
- drop the metricsUrl helper and the merged telemetry helpers

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…eProviders

Set configManagement.serverUrl to http://localhost:8080/api as in the
raster-core umbrella chart and make storage.cleanupStorageProviders an
empty list so rendering without providers no longer fails on has "S3".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@CL-SHLOMIKONCHA CL-SHLOMIKONCHA changed the title fix(helm): align chart defaults with raster-core values and merge telemetry from global fix(helm): align defaults with raster-core and split server/telemetry values Sep 10, 2026
CL-SHLOMIKONCHA and others added 6 commits September 10, 2026 14:45
Only the server and telemetry values leave env. The cleaner specific
keys (queue, jobnik, worker, httpRetry, disableHttpClientLogs,
strategies) stay under env as on master.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…the template

The configmap already falls back to 1, so the value is not required.
Use a nil-safe lookup so the key renders without the block and still
honors env.jobnik.worker.concurrency when set.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ship the value again and keep the original with block in the configmap.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…(MAPCO-11633)

Move the ca defaults (secretName, path, key) under global.ca and read them
directly in the deployment instead of merging a chart-level ca scope.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@vitaligi vitaligi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job!

see similar prettyPrint comment

Comment thread helm/values.yaml
Comment on lines +135 to 136
- job: "Delete_Layer"
task: "tiles-deletion"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add artifacts-deletion as well? if worker.capabilities.pairs is overidden by global all/most of time, should we just put it in comment?

Comment thread helm/values.yaml
Comment on lines +10 to +13
ca:
secretName: ''
path: '/usr/local/share/ca-certificates'
key: 'ca.crt'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not aligned with the boilerplate

{{- $cloudProviderImagePullSecretName := include "cleaner.cloudProviderImagePullSecretName" . -}}
{{- $imageTag := include "cleaner.tag" . -}}
{{- $ca := fromYaml (include "common.ca.merged" .) -}}
{{- $server := .Values.server -}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was $ca removed?

Aligns with ts-server-boilerplate#477: loggerConfig already carries prettyPrint,
so spreading it is enough.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants