docs(kwok): add prerequisites and fix copy-paste pitfalls#709
Conversation
- Add Prerequisites section listing required tools (kind, ctlptl, helm, yq, goreleaser) and Docker Desktop requirement - Clarify kwok/kwokctl binaries are not needed; KWOK controller is applied via kubectl in make kwok-cluster - Add GITLAB_TOKEN warning before make build - Add repo root note so users don't run make from kwok/ subdirectory - Split Quick Start into distinct operations to avoid unintended copy-paste execution of multiple targets - Remove inline comments from code blocks (zsh interprets them unexpectedly when pasting blocks) - Replace redundant local CLI section with link to docs/user/cli-reference.md Validated on Apple Silicon Mac: make kwok-e2e RECIPE=h100-eks-ubuntu-training-kubeflow passes with 35 pods scheduled across 2 system nodes and 4 fake H100 GPU nodes. Signed-off-by: Arun Gupta <[email protected]>
📝 WalkthroughWalkthroughThe Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@kwok/README.md`:
- Around line 29-48: Add blank lines before and after each fenced code block in
the "Quick Start" section of README.md so the Markdown renders correctly;
specifically ensure there's an empty line between the preceding paragraph or
heading (e.g., the "Build", "Test a single recipe:", and "Test all recipes:"
lines) and the starting ```bash fence, and an empty line after the closing ```
fence before the next paragraph or heading.
- Around line 23-25: Remove the blank line between the two consecutive
blockquotes in README.md to satisfy MD028: place the `**Note:** The `kwok` and
`kwokctl` binaries...` blockquote immediately above the `**Note:** If
`GITLAB_TOKEN`...` blockquote with no empty line between them (or alternatively
merge both notes into a single blockquote containing two paragraphs) so the
linter no longer flags a blank line between blockquotes.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 52d212be-1e09-4178-92bb-cb2d04c46ef9
📒 Files selected for processing (1)
kwok/README.md
| > **Note:** The `kwok` and `kwokctl` binaries are not required — the KWOK controller is installed into the cluster automatically by `make kwok-cluster` via `kubectl apply`. | ||
|
|
||
| > **Note:** If `GITLAB_TOKEN` is set in your environment, `make build` will fail. Always run `unset GITLAB_TOKEN` before building. |
There was a problem hiding this comment.
Address blank line between consecutive blockquotes.
Markdown linting (MD028) flags the blank line between these two blockquote notes. While the current structure is readable, standard Markdown style prefers no blank lines between consecutive blockquotes.
📝 Proposed fix to remove blank line between blockquotes
> **Note:** The `kwok` and `kwokctl` binaries are not required — the KWOK controller is installed into the cluster automatically by `make kwok-cluster` via `kubectl apply`.
-
> **Note:** If `GITLAB_TOKEN` is set in your environment, `make build` will fail. Always run `unset GITLAB_TOKEN` before building.Alternatively, you could combine them into a single blockquote with multiple paragraphs:
-> **Note:** The `kwok` and `kwokctl` binaries are not required — the KWOK controller is installed into the cluster automatically by `make kwok-cluster` via `kubectl apply`.
+> **Note:**
+> - The `kwok` and `kwokctl` binaries are not required — the KWOK controller is installed into the cluster automatically by `make kwok-cluster` via `kubectl apply`.
+> - If `GITLAB_TOKEN` is set in your environment, `make build` will fail. Always run `unset GITLAB_TOKEN` before building.
-> **Note:** If `GITLAB_TOKEN` is set in your environment, `make build` will fail. Always run `unset GITLAB_TOKEN` before building.
-
Before running KWOK tests, use `aicr recipe`, `aicr query`, and `aicr bundle` to generate and inspect configuration without a cluster. See the [CLI reference](../docs/user/cli-reference.md) for details.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| > **Note:** The `kwok` and `kwokctl` binaries are not required — the KWOK controller is installed into the cluster automatically by `make kwok-cluster` via `kubectl apply`. | |
| > **Note:** If `GITLAB_TOKEN` is set in your environment, `make build` will fail. Always run `unset GITLAB_TOKEN` before building. | |
| > **Note:** The `kwok` and `kwokctl` binaries are not required — the KWOK controller is installed into the cluster automatically by `make kwok-cluster` via `kubectl apply`. | |
| > **Note:** If `GITLAB_TOKEN` is set in your environment, `make build` will fail. Always run `unset GITLAB_TOKEN` before building. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 24-24: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kwok/README.md` around lines 23 - 25, Remove the blank line between the two
consecutive blockquotes in README.md to satisfy MD028: place the `**Note:** The
`kwok` and `kwokctl` binaries...` blockquote immediately above the `**Note:** If
`GITLAB_TOKEN`...` blockquote with no empty line between them (or alternatively
merge both notes into a single blockquote containing two paragraphs) so the
linter no longer flags a blank line between blockquotes.
| ## Quick Start | ||
|
|
||
| All `make` commands must be run from the **repo root**. Run `cd /path/to/aicr` first, then: | ||
|
|
||
| **Build** (one-time, or after code changes): | ||
| ```bash | ||
| unset GITLAB_TOKEN | ||
| make build | ||
| ``` | ||
|
|
||
| **Test a single recipe:** | ||
| ```bash | ||
| make build # Build aicr binary | ||
| make kwok-test-all # Test all recipes (serial) | ||
| make kwok-test-all-parallel # Test all recipes (parallel, faster) | ||
| make kwok-e2e RECIPE=h100-eks-ubuntu-training-kubeflow # Test single recipe | ||
| make kwok-e2e RECIPE=h100-eks-ubuntu-training-kubeflow | ||
| ``` | ||
|
|
||
| **Test all recipes:** | ||
| ```bash | ||
| make kwok-test-all | ||
| make kwok-test-all-parallel | ||
| ``` |
There was a problem hiding this comment.
Add blank lines before fenced code blocks per Markdown best practices.
The Quick Start section is well-structured and clearly separates build from test operations. However, fenced code blocks should be surrounded by blank lines for proper Markdown formatting.
📝 Proposed fix to add blank lines before code blocks
**Build** (one-time, or after code changes):
+
```bash
unset GITLAB_TOKEN
make buildTest a single recipe:
+
make kwok-e2e RECIPE=h100-eks-ubuntu-training-kubeflowTest all recipes:
+
make kwok-test-all
make kwok-test-all-parallel</details>
The operational flow is clear and the separation of build/test steps helps prevent copy-paste errors.
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
## Quick Start
All `make` commands must be run from the **repo root**. Run `cd /path/to/aicr` first, then:
**Build** (one-time, or after code changes):
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 34-34: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 40-40: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 45-45: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kwok/README.md` around lines 29 - 48, Add blank lines before and after each
fenced code block in the "Quick Start" section of README.md so the Markdown
renders correctly; specifically ensure there's an empty line between the
preceding paragraph or heading (e.g., the "Build", "Test a single recipe:", and
"Test all recipes:" lines) and the starting ```bash fence, and an empty line
after the closing ``` fence before the next paragraph or heading.
mchmarny
left a comment
mchmarny
left a comment
There was a problem hiding this comment.
Useful onboarding cleanup. The relative link to ../docs/user/cli-reference.md resolves, the make kwok-cluster/kwok-e2e/kwok-test-all targets in the Makefile match the README claims, and the unset GITLAB_TOKEN guidance lines up with the project's documented goreleaser quirk. Splitting the Quick Start block (so a paste doesn't inadvertently run all three flavors) is a nice touch.
Brew-only install instructions are Mac-specific; that matches the validation context the author called out, and broader OS coverage belongs in the top-level docs. LGTM.
2 participants
Summary
Improve
kwok/README.mdto be self-contained and usable from a fresh setup — adding prerequisites, fixing copy-paste pitfalls, and linking out to the CLI reference instead of duplicating it.Motivation / Context
The existing README assumed tools were already installed and commands were run from the right directory. First-time users hit avoidable errors before getting to the actual KWOK workflow.
Fixes: N/A
Related: N/A
Type of Change
Component(s) Affected
docs/,examples/)Changes
kind,ctlptl,helm,yq,goreleaser, Docker Desktopkwok/kwokctlbinaries are not required (KWOK controller is applied viakubectl)unset GITLAB_TOKENwarning beforemake buildmakefromkwok/docs/user/cli-reference.mdTesting
Validated end-to-end on Apple Silicon Mac:
make build— binary built intodist/make kwok-e2e RECIPE=h100-eks-ubuntu-training-kubeflow— 35 pods scheduled across 2 system nodes and 4 fake H100 GPU nodes, all passingRisk Assessment
Rollout notes: N/A — docs only, no functional changes.
Checklist
make testwith-race)make lint)git commit -S) — GPG signing info