Summary
/reconcile gap analysis at HEAD be9be50 of openshell-gateway-helm-adoption.spec.md found the shell-out Helm integration (PR #194) faithfully implements chart embedding, values mapping, SCC binding, trusted-CA normalization, deletion flow, and SSA removal - but several spec requirements are divergent or unimplemented. These need decisions because some interact with the external chart schema (not vendored in this workspace) and one changes steady-state reconcile behavior.
Findings (ranked)
helm upgrade fires on every reconcile of a live gateway - internal/helm/helm_deploy.go:74-84 calls Upgrade when status==deployed and for any other status. The spec states upgrades are not handled and that helm upgrade is invoked only to retry a failed/pending-install release. Steady-state reconciles should be a no-op when status==deployed.- OCI registry override is dead code -
internal/helm/chart.go PullChart has no caller; cmd/hypershell-controller/main.go:277-280 always uses the embedded chart path. HELM_CHART_REGISTRY/HELM_CHART_VERSION are read/validated in config.go but the override is inert. Startup should call PullChart once (cached) when HELM_CHART_REGISTRY is set and point ShellClient.ChartPath at it.
- Retry policy incomplete - no Helm-specific exponential backoff (1m/2m/4m/8m/max 15m) and no "mark
State: Failed after 5 consecutive failures." Only a generic 2s->30s requeue limiter exists (internal/watcher/requeue.go:24-25).
- Startup validation is a soft warning, not fail-fast -
internal/gateway/reconciler.go:1483-1485 logs WARN when a cluster lacks Gateway API and EXTERNAL_CA_ISSUER_NAME is unset. Spec requires failing startup with the exact message EXTERNAL_CA_ISSUER_NAME required for Route passthrough mode on cluster <name>.
supervisorImage values key mismatch - code writes supervisor.image.repository/.tag (internal/helm/values.go:113-114) while the spec Core Values table specifies supervisorImage.repository/.tag. One is wrong relative to the actual chart values.yaml; needs verification against the vendored chart schema, then align code or spec.- Helm v3 check is advisory -
internal/helm/shell_client.go:216-219 logs WARN on a non-v3 helm; spec says init SHALL verify v3.x.
Decision needed
Confirm intended behavior for (1) and (4) (both change control flow / startup), and verify (5) against the real chart before editing code or spec. (2), (3), (6) are additive and can be scheduled once (1)/(4)/(5) are settled.
Filed by /reconcile; not fixed in-run - chart schema is not vendored in the reconcile sandbox and several items are behavior/spec decisions.
Summary
/reconcilegap analysis at HEADbe9be50ofopenshell-gateway-helm-adoption.spec.mdfound the shell-out Helm integration (PR #194) faithfully implements chart embedding, values mapping, SCC binding, trusted-CA normalization, deletion flow, and SSA removal - but several spec requirements are divergent or unimplemented. These need decisions because some interact with the external chart schema (not vendored in this workspace) and one changes steady-state reconcile behavior.Findings (ranked)
helm upgradefires on every reconcile of a live gateway -internal/helm/helm_deploy.go:74-84callsUpgradewhen status==deployedand for any other status. The spec states upgrades are not handled and thathelm upgradeis invoked only to retry afailed/pending-installrelease. Steady-state reconciles should be a no-op when status==deployed.internal/helm/chart.goPullCharthas no caller;cmd/hypershell-controller/main.go:277-280always uses the embedded chart path.HELM_CHART_REGISTRY/HELM_CHART_VERSIONare read/validated inconfig.gobut the override is inert. Startup should callPullChartonce (cached) whenHELM_CHART_REGISTRYis set and pointShellClient.ChartPathat it.State: Failedafter 5 consecutive failures." Only a generic 2s->30s requeue limiter exists (internal/watcher/requeue.go:24-25).internal/gateway/reconciler.go:1483-1485logsWARNwhen a cluster lacks Gateway API andEXTERNAL_CA_ISSUER_NAMEis unset. Spec requires failing startup with the exact messageEXTERNAL_CA_ISSUER_NAME required for Route passthrough mode on cluster <name>.supervisorImagevalues key mismatch - code writessupervisor.image.repository/.tag(internal/helm/values.go:113-114) while the spec Core Values table specifiessupervisorImage.repository/.tag. One is wrong relative to the actual chartvalues.yaml; needs verification against the vendored chart schema, then align code or spec.internal/helm/shell_client.go:216-219logsWARNon a non-v3 helm; spec says init SHALL verify v3.x.Decision needed
Confirm intended behavior for (1) and (4) (both change control flow / startup), and verify (5) against the real chart before editing code or spec. (2), (3), (6) are additive and can be scheduled once (1)/(4)/(5) are settled.
Filed by
/reconcile; not fixed in-run - chart schema is not vendored in the reconcile sandbox and several items are behavior/spec decisions.