docs: consolidate Claude Code deployment docs and fix settings.json mount#181
Open
jwm4 wants to merge 8 commits into
Open
docs: consolidate Claude Code deployment docs and fix settings.json mount#181jwm4 wants to merge 8 commits into
jwm4 wants to merge 8 commits into
Conversation
Add "Retrieving Files from the Container" section showing how to use oc cp to download generated files (reports, documents, code) from the container to a local machine without pushing to a Git repository. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ount Consolidate three overlapping documents (deployment/README.md, mlflow-tracing.md, docs/claude-code-vllm-direct.md) into a single agents/claude-code/README.md. Deduplicate repeated build, interactive mode, and debug mode steps across all four backend options. Fix settings.json ConfigMap mount to support MLflow autolog: stage the ConfigMap at a read-only path (/etc/claude-config/settings.json) and copy it to the writable PVC in the entrypoint, so mlflow autolog can inject hooks at runtime. Add commented-out MLflow and context window env var placeholders to all four deployment manifests. Add MLflow UI access instructions (use gateway URL from ConsoleLink, login with deployer identity). Add 32K context window example to the configuration table. Revert test-specific values in deployment-vllm.yaml to placeholders. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughConsolidates OpenShift deployment docs into a central Claude Code guide, stages ConfigMap data under /etc in all deployment manifests with optional MLflow env blocks, tweaks the Containerfile, and implements entrypoint logic to copy/symlink staged settings/skills and pre-approve Anthropic API keys. ChangesClaude Code OpenShift Deployment Documentation & Manifests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
|
Large PR detected (2799 lines changed) This PR exceeds 1200 lines of code changes (excluding lock files, generated content, and images). Large PRs are harder to review thoroughly and are more likely to introduce bugs. Consider splitting this PR into smaller, focused changes. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![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: 3
🤖 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 `@agents/claude-code/deployment/entrypoint.sh`:
- Around line 193-195: The startup copy unconditionally overwrites persisted
settings.json; change the logic around staged_settings/target_settings so we
only copy when target_settings does not already exist (or is empty) to preserve
runtime/user updates across restarts: check if "${target_settings}" exists and
is non-empty and skip the cp (calling log_info to state preservation), otherwise
copy staged_settings to target_settings and log the copy; update the conditional
around staged_settings and the log_info call in entrypoint.sh to reflect the new
behavior.
In `@agents/claude-code/mlflow-tracing.md`:
- Around line 42-48: Add an explicit privacy/retention warning noting that the
"Prompt / response" field captures full prompt and output text (which may
contain secrets, PII, or proprietary code) and that MLflow experiments/backends
must be treated as sensitive; update the documentation near the Claude Code hook
/ OTel spans section (adjacent to the table rows "Prompt / response" and
"Session ID") to state required retention policies, redaction/obfuscation
recommendations, and operator access controls, and include a short example of
mitigation steps (e.g., automatic redaction, encryption at rest, shorter
retention) so operators know how to handle traced data.
In `@agents/claude-code/README.md`:
- Around line 599-600: The README currently hardcodes `/workspace/.claude` for
where the entrypoint writes settings; update the wording to reference the
configurable environment variable `CLAUDE_CONFIG_DIR` (and the path
`${CLAUDE_CONFIG_DIR}/settings.json`) instead so the doc matches the entrypoint
behavior when persistence is disabled or the config dir is customized; change
the sentence that says "copied to `/workspace/.claude/settings.json`" to "copied
to `${CLAUDE_CONFIG_DIR}/settings.json` (the entrypoint writes to
$CLAUDE_CONFIG_DIR/settings.json)" and keep the note about the staged read-only
source `/etc/claude-config/settings.json` and the varying ConfigMap names
(`claude-settings`, `claude-vertex-settings`, `claude-vllm-settings`,
`claude-ogx-vllm-settings`).
🪄 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: CHILL
Plan: Enterprise
Run ID: b8e49603-a2a2-4c8e-ac59-da74bf85ef1f
📒 Files selected for processing (10)
agents/claude-code/README.mdagents/claude-code/deployment/README.mdagents/claude-code/deployment/deployment-ogx-vllm.yamlagents/claude-code/deployment/deployment-vertex.yamlagents/claude-code/deployment/deployment-vllm.yamlagents/claude-code/deployment/deployment.yamlagents/claude-code/deployment/entrypoint.shagents/claude-code/mlflow-tracing.mdagents/claude-code/vllm/README.mddocs/claude-code-vllm-direct.md
💤 Files with no reviewable changes (1)
- docs/claude-code-vllm-direct.md
Remove inline sample skill from Anthropic and Vertex manifests to match vLLM/OGX (all four now have empty skills ConfigMap). Move example skill content and complete injection instructions into the README, including the required items mapping that was previously undocumented. Testing confirmed that skills require the items mapping to create the subdirectory structure Claude Code expects. Without it, files appear flat and are not discovered. Drop scheme prefix from URL placeholders (YOUR_VLLM_URL, YOUR_OGX_URL) to avoid implying http vs https. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Preserve runtime settings.json across pod restarts instead of unconditionally overwriting on every start. Update README to reference CLAUDE_CONFIG_DIR variable instead of hardcoded path. Add privacy note to mlflow-tracing.md about traced data containing sensitive content. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ents Replace misleading oc rollout restart with oc set image, which correctly updates the deployment to use the new build. OpenShift pins deployments to a specific image digest, so a restart alone recreates pods with the old image. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ments - Move skills ConfigMap mount to /etc/claude-skills/ with entrypoint symlink to prevent Kubernetes from creating /workspace/.claude/ as root on fresh PVCs - Add chmod g+w to config/projects dirs for OpenShift group-writable requirement - Add approve_api_key() to entrypoint to pre-approve configured API key, preventing interactive wizard authentication trap - Add CLAUDE_MODEL env var to vLLM and OGX manifests (required for --model CLI flag) - Add ENV HOME to Containerfile, removing need for export HOME in all oc exec examples - Add tar to container image (required for oc cp) - Add ANTHROPIC_BASE_URL examples (http internal, https external route) - Add auth loop troubleshooting section with recovery script Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
deployment/README.md,mlflow-tracing.md,docs/claude-code-vllm-direct.md) into a singleagents/claude-code/README.md, deduplicating repeated build, interactive mode, and debug mode steps across all four backend optionssettings.jsonConfigMap mount to support MLflow autolog: stages the ConfigMap at a read-only path (/etc/claude-config/settings.json) and copies it to the writable PVC in the entrypoint, somlflow autologcan inject hooks at runtimedeployment-vllm.yamlto placeholdersTest plan
"Say hello") and confirmed traces recorded in MLflow experimentFrom Bill Murdock with assistance from Claude Code.
🤖 Generated with Claude Code