refactor(openclaw): migrate to bjw-s-labs app-template chart#318
Merged
Conversation
Upstream serhanekicii/openclaw-helm was archived 2026-05-25. Vendor the chart's config into app-template 4.6.2: init-config/init-skills init containers, scripts/config configMaps, PVC data volume, tmp/npm-cache emptyDirs, and TCP probes on 18789.
Remove the uv/pnpm runtime-dependency example comment block from init-skills.sh, collapse the three identical container securityContext blocks into a single YAML anchor, drop the redundant startup probe on the main container, and remove the unused bash_aliases configmap entry and its mount. helm template renders identically (verified).
Contributor
📝 WalkthroughWalkthroughMigrates OpenClaw to the bjw-s-labs ChangesOpenClaw App-Template Migration
Sequence Diagram(s)sequenceDiagram
participant PodStartup as Pod Startup
participant InitConfig as init-config.sh
participant InitSkills as init-skills.sh
participant Persist as Persistent Storage
participant Main as Main Container
PodStartup->>InitConfig: run with `configMode` & Helm-provided /config/openclaw.json
InitConfig->>Persist: read existing /home/node/.openclaw/openclaw.json
InitConfig->>InitConfig: strip JSON5 comments, deep-merge Helm config
InitConfig->>Persist: write merged or fallback openclaw.json
PodStartup->>InitSkills: run with SKILLS_TO_INSTALL
InitSkills->>InitSkills: loop install with retry/backoff + jitter
InitSkills->>Persist: write installed skills to .openclaw
PodStartup->>Main: start main after inits complete
Main->>Persist: load openclaw.json and installed skills
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/automation/openclaw/values.yaml`:
- Around line 107-115: The Chromium container repeats the same securityContext
as the existing YAML anchor &containerSecurityContext; replace the duplicated
block in the Chromium container's securityContext with a reference to the anchor
(use the anchor name containerSecurityContext) so the container reads
securityContext: *containerSecurityContext instead of duplicating fields; locate
the Chromium container spec (the securityContext block shown) and swap it for
the anchor reference to collapse duplication and keep consistency.
- Around line 252-257: The jitter calculation using non-POSIX $RANDOM in the
init-skills script is always 0 under sh; replace the line that sets jitter (used
by the log, sleep and delay logic in init-skills.sh) with a POSIX-safe approach:
read a single byte from /dev/urandom (e.g., via dd/od or dd + od -An -tu1),
compute modulo 5 to get 0-4, and assign that to jitter; ensure the method works
under /bin/sh, redirects errors to /dev/null, and falls back to 0 if the random
read fails so the subsequent log "WARNING... retrying in $((delay +
jitter))s..." and sleep call behave correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7888c9b-1fcd-4b9b-b6da-91b9967027a4
📒 Files selected for processing (2)
apps/automation/openclaw/application.yamlapps/automation/openclaw/values.yaml
The retry backoff jitter used $RANDOM, which is undefined under the init container's POSIX sh (busybox/dash), so it always evaluated to 0 and the anti-lockstep jitter never applied. Read one byte from /dev/urandom instead, with a fallback to 0.
The chromium container repeated the same securityContext as the &containerSecurityContext anchor used by the other containers; reference the anchor instead to remove the duplication.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/automation/openclaw/values.yaml (2)
363-376: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueQuick note on storage strategy, boss!
The coding guidelines mention using "default host paths
/var/local/<app>for config/database storage." This config uses a PVC with emptystorageClass(cluster default) instead.PVCs are more portable and Kubernetes-native, which is totally valid! But if you want to follow the established pattern for easier local backups and inspection on this homelab setup, you could switch to a hostPath:
🗂️ Alternative using host path (if preferred)
data: enabled: true type: hostPath hostPath: /var/local/openclaw advancedMounts: main: init-config: - path: /home/node/.openclaw init-skills: - path: /home/node/.openclaw main: - path: /home/node/.openclawIf PVC is the intentional choice for this app (maybe for easier migration or cluster portability), consider documenting the rationale.
As per coding guidelines: "Use default host paths
/var/local/<app>for config/database storage"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/automation/openclaw/values.yaml` around lines 363 - 376, The storage config currently sets data.type to persistentVolumeClaim with an empty storageClass which diverges from the project's guideline to use host paths; update the manifest so data.type is hostPath and set data.hostPath to /var/local/openclaw (and keep advancedMounts entries as-is) OR if you intentionally prefer PVCs, add a short comment/documentation near the data block explaining why persistentVolumeClaim (empty storageClass) is chosen; look for the data, type, storageClass, hostPath and advancedMounts keys to make the change.
329-337:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winOpenClaw is missing Traefik ingress for
openclaw.hobroker.meService config is totally boss (HTTP port 18789), but this
apps/automation/openclaw/values.yamlhas noingressconfiguration at all—so there are no Traefik routes/invitations foropenclaw.hobroker.meper the<app>.hobroker.meguideline. Add aingressblock mappingopenclaw.hobroker.me→service.main/http, or add a comment stating it’s intentionally internal-only.service: main: controller: main ipFamilyPolicy: SingleStack ipFamilies: - IPv4 ports: http: port: 18789🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/automation/openclaw/values.yaml` around lines 329 - 337, Add an ingress block for OpenClaw mapping the hostname openclaw.hobroker.me to the existing service.main HTTP port (service.main → ports.http port 18789), or explicitly add a comment stating the service is internal-only; specifically update apps/automation/openclaw/values.yaml to include an ingress entry that targets service: main and port: http (host: openclaw.hobroker.me) so Traefik will route external traffic, or add the internal-only comment next to service.main to make the intent explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/automation/openclaw/values.yaml`:
- Around line 363-376: The storage config currently sets data.type to
persistentVolumeClaim with an empty storageClass which diverges from the
project's guideline to use host paths; update the manifest so data.type is
hostPath and set data.hostPath to /var/local/openclaw (and keep advancedMounts
entries as-is) OR if you intentionally prefer PVCs, add a short
comment/documentation near the data block explaining why persistentVolumeClaim
(empty storageClass) is chosen; look for the data, type, storageClass, hostPath
and advancedMounts keys to make the change.
- Around line 329-337: Add an ingress block for OpenClaw mapping the hostname
openclaw.hobroker.me to the existing service.main HTTP port (service.main →
ports.http port 18789), or explicitly add a comment stating the service is
internal-only; specifically update apps/automation/openclaw/values.yaml to
include an ingress entry that targets service: main and port: http (host:
openclaw.hobroker.me) so Traefik will route external traffic, or add the
internal-only comment next to service.main to make the intent explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5c2bbc6b-8086-40d3-ac69-ea05f2b8cc97
📒 Files selected for processing (1)
apps/automation/openclaw/values.yaml
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
1 participant
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.
Summary
Migrates the openclaw deployment off the archived
serhanekicii/openclaw-helmchart (archived 2026-05-25) onto the maintainedbjw-s-labs/app-templatechart (v4.6.2), and reimplements the behaviour the old chart provided internally:openclaw.jsononto the PVC (merge mode preserves runtime-written config across restarts) and install the configured skills, all running non-root with a read-only root filesystem and dropped capabilities.securityContextblocks into a single YAML anchor, drops the redundant TCP startup probe on the main container, and removes an unusedbash_aliasesconfigmap entry plus a stale runtime-dependency comment block.Verified with
helm template openclaw bjw-s-labs/app-template --version 4.6.2 -f apps/automation/openclaw/values.yaml: renders cleanly (ConfigMap/Deployment/PVC/Service, liveness + readiness probes,readOnlyRootFilesystemon all containers, nobash_aliases).Type
Summary by CodeRabbit