Closed
Conversation
added 5 commits
April 15, 2026 04:53
Owner
Author
|
Superseded by #2. |
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.
What problem does this solve?
OpenAB's Helm chart is currently strong on the minimal install path, but it is too thin for many real Kubernetes deployments.
It does not currently expose several common deployment controls that operators typically expect from a reusable open-source chart, including:
imagePullSecretslivenessProbe,readinessProbe,startupProbe)lifecycleinitContainersserviceAccountThis leaves users with a few unattractive options:
helm templateOne immediate example is that the chart does not currently support
imagePullSecrets, which makes private registry deployments harder than they need to be.Closes #
Discord Discussion URL: https://discordapp.com/channels/1491295327620169908/1493841502529523732
What's in this PR
charts/openab/values.yamlcharts/openab/templates/_helpers.tplcharts/openab/templates/deployment.yamlDeployment.spec.templatecharts/openab/tests/helm-template-test.shset -eKey decisions
initContainers+ shared volumes are the supported lightweight bootstrap pathPodDisruptionBudget, RBAC, and generic extra objects are deferred to follow-up PRsAt a Glance
Prior Art & Industry Research
OpenClaw:
I reviewed the local OpenClaw repository and its Kubernetes deployment manifests. While OpenClaw does not currently ship a Helm chart in this repo, it does treat startup bootstrap as a first-class deployment concern. In particular, it uses an
initContainerto prepare configuration and workspace state before the main container starts.This suggests:
initContainersare a reasonable place for startup preparationReference:
scripts/k8s/manifests/deployment.yamlHermes Agent:
I also reviewed Hermes Agent's Docker and deployment documentation. Hermes takes a clearer stance on tool installation: stable toolchains should primarily be handled through custom images or clearly defined mutable runtime environments, not by stretching the chart into a package manager.
This suggests:
References:
Dockerfilewebsite/docs/user-guide/docker.mdwebsite/docs/getting-started/nix-setup.mdOther references:
I also reviewed Bitnami charts and Helm / Kubernetes best practices.
Bitnami's more mature charts commonly expose capabilities such as:
imagePullSecretsserviceAccountlifecycleinitContainersextraVolumesextraVolumeMountsextraEnvVarsCMextraEnvVarsSecretextraDeploypdbRelevant upstream guidance also supports this direction:
initContainersimagePullSecretsProposed Solution
This PR implements the first phase of pod / deployment extensibility for the OpenAB Helm chart.
Implemented scope:
imagePullSecretsimagePullPolicylifecycleinitContainersextraVolumesextraVolumeMountsserviceAccountNamebindingFor the "install tools" question specifically, the proposal recommends two clear paths:
Custom image
the preferred path for stable, repeatable, production-grade toolchains
initContainers+ shared volumea lightweight bootstrap path for small binaries or startup initialization
Explicitly out of scope in this PR:
PodDisruptionBudgetextraDeployThis keeps the implementation focused on
Deployment.spec.template, solves the highest-value deployment gaps first, and avoids mixing pod extensibility with broader chart resource management.Why this approach?
I do not think OpenAB should model every Kubernetes field at once, and I do not think Helm should become the primary mechanism for packaging arbitrary tools.
At the same time, the current chart is thin enough that users are pushed toward forking it for fairly normal deployment requirements. That creates unnecessary friction for a public chart.
This approach takes the middle path:
initContainersbootstrap as a lightweight complement, not the main packaging modelTradeoffs and limitations:
Alternatives Considered
The main implementation question is not whether Helm extensibility should be improved, but how much should be included in the first implementation PR.
1. Do the minimum: only
imagePullSecretsRejected.
This would solve the most immediate private-registry gap, but it would still leave the chart without probes, lifecycle hooks, and pod composition controls such as
initContainers, sidecars, and extra volumes. The result would still feel incomplete for real deployments.2. Implement pod / deployment extensibility only
Chosen.
This keeps the first implementation focused on a single surface area:
Deployment.spec.template. It addresses the highest-value deployment gaps first, including private registry support, health / lifecycle controls, and pod composition hooks, while keeping the PR cohesive and reviewable.3. Implement everything at once, including PDB, RBAC, and generic extra objects
Rejected.
Although these features are valid chart capabilities, they extend beyond pod template extensibility and would make the first implementation significantly broader. Mixing pod-level changes with additional chart-managed resources would increase review complexity and make the initial rollout harder to reason about.
Validation
imagePullSecretsimagePullPolicyinitContainerslifecycleserviceAccountNameValidation command used:
Result:
14 passed, 0 failedOpen questions
PodDisruptionBudgetbefore or after chart-managed ServiceAccount support?extraDeploy,extraObjects, or deferred further?serviceAccountNameremain binding-only for now, or should chart-managed creation be the next step?