fix(ontoserver): expose podManagementPolicy and OTel exporters, restore empty serverPort default - #13
Merged
Conversation
podManagementPolicy was hardcoded to Parallel. Under Parallel the controller does not wait for a replacement pod to be Ready before replacing the next one, so a rolling update on a multi-replica install can terminate every pod within seconds and leave the Service with no ready endpoint until the first replacement passes its readiness probe. A PodDisruptionBudget does not help: it only gates the Eviction API, not StatefulSet-driven pod replacement. The default stays Parallel, so existing installs render identically and keep the fast parallel scale-up. Operators who prefer availability during rolling updates over startup speed can now set OrderedReady.
OTEL_METRICS_EXPORTER and OTEL_LOGS_EXPORTER were hardcoded to "none" in the Instrumentation resource, with no way to override them from values. That leaves metricsExporter: otlp - the usual way to get JVM heap and GC metrics off the Java agent - unreachable for chart users; unlike most other agent settings it cannot be worked around from outside the chart. Both default to "none", so an existing install renders unchanged.
0.4.0 changed the serverPort default from "" to "8080" so the local port-forward instructions would produce working URLs. The side effect is that every install that never sets serverPort - including the cloud examples served on 80/443 - publishes host:8080 in its CapabilityStatement, ontoserver.fhir.base and canonical URLs. Nothing errors, so it is easy to miss. Revert the default to "" and instead set serverPort: "8080" explicitly in the two local examples whose documented access path is a 8080:80 port-forward. The k3d examples already set it. Installs on 0.4.0/0.4.1 that relied on the inherited default must now set serverPort: "8080" themselves.
Adds a "StatefulSet rolling updates and podManagementPolicy" section covering the no-ready-endpoint window under Parallel, why a PDB does not prevent it, the OrderedReady trade-off, and the kubectl delete --cascade=orphan recreate needed to adopt the policy on a live StatefulSet since the field is immutable. Also records the serverPort default change and its effect on 0.4.0/0.4.1 installs, and adds the new parameters to the values tables.
Runs the helm-readme-generator table pass so the parameter tables match values.yaml exactly, rather than the hand-written rows added in the previous commit. Only descriptions and column padding change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three issues found while migrating an install to the 0.4.0 chart and doing a throwaway scaled install on EKS alongside it.
1.
podManagementPolicywas hardcoded toParallelstatefulset.yamlsetpodManagementPolicy: Parallelas a literal. UnderParallelthe controller does not wait for a replacement pod to be Ready before replacing the next one, so a rolling update on a multi-replica install can terminate every pod within seconds and leave the Service with no ready endpoint until the first replacement passes its readiness probe. Observed in the field: pods going ~27s apart, ~74s with no ready endpoint, some 503s. A PodDisruptionBudget does not prevent this — it only gates the Eviction API (node drains), not StatefulSet-controller-driven replacement.Now
ontoserver.deployment.podManagementPolicy, defaulting toParallelso existing installs render identically and keep the fast parallel scale-up. Schema-constrained toParallel | OrderedReady.The field is immutable on a live StatefulSet, so the README documents the
kubectl delete statefulset <release>-statefulset --cascade=orphanrecreate needed to adopt it, and the trade-off thatOrderedReadyserializes startup and scale-up as well — which matters when pods need a long Lucene index preload underhealthCheckOption: -s.2.
OTEL_METRICS_EXPORTER/OTEL_LOGS_EXPORTERwere hardcoded tononeBoth were literals in the
Instrumentationresource with no values override, which putsmetricsExporter: otlp— the usual way to get JVM heap and GC metrics off the Java agent — out of reach. Unlike most other agent settings it cannot be worked around from outside the chart.Now
ontoserver.opentelemetry.instrumentation.metricsExporterand.logsExporter, both defaulting to"none", so an existing install renders unchanged.3.⚠️ behaviour change
serverPortdefault went from""to"8080"b37ce47changed the default to"8080"so the local port-forward instructions would produce working URLs. The side effect: every install that never setsserverPort— including the aks/eks examples served on 80/443 — publisheshost:8080in its CapabilityStatement,ontoserver.fhir.baseand canonical URLs. Nothing errors, so it is easy to miss.Reverted to
"", withserverPort: "8080"moved to where the intent belongs: explicit inexamples/local/single-ro.yamlandsingle-rw.yaml, the two examples whose documented access path is a8080:80port-forward. The k3d examples already set it.Reviewers note: installs on 0.4.0/0.4.1 that relied on the inherited default must now set
serverPort: "8080"themselves. Called out in the README.Verification
helm lintclean, no warningshelm unittest: 271 tests pass, 5 new — podManagementPolicy override, exporter defaults, exporter overrides, and base URLs with and withoutserverPorthelm templatechecked for the default render,OrderedReady, schema rejection of an invalid policy, and both edited examplesChart.yamlversion:deliberately untouched — Release Please owns it, and thefeatcommits should drive a minor bump. The hand-maintainedartifacthub.io/changeslist still describes the last released version and is worth updating when the release PR is cut.🤖 Generated with Claude Code