rework examples to use kustomize#124
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors examples into a parametric generation flow: adds ChangesExamples Generation & Deployment Refactor
sequenceDiagram
participant Makefile as Makefile
participant Generator as examples/generate-files.sh
participant Kustomize as kustomize
participant Kubectl as kubectl (cluster)
Note over Makefile,Generator: user runs Makefile orchestration target
Makefile->>Generator: invoke with EXAMPLE_* env vars
Generator->>Kustomize: write overlays/templates then run `kustomize build`
Kustomize-->>Generator: emitted YAML file
Generator->>Kubectl: run `kubectl ${EXAMPLE_CMD} -f <output>.yaml`
Kubectl-->>Makefile: cluster applies/deletes generated resources
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
691fd28 to
d58226c
Compare
|
I still need to update docs, but I think functionality is in place. |
| @@ -0,0 +1,3 @@ | |||
| key: nvidia.com/gpu | |||
There was a problem hiding this comment.
@maryamtahhan I didn't see a Toleration for ROCm in #107 . I can add a similar patch if you there is one.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
390-397:⚠️ Potential issue | 🟠 MajorUpdate the legacy aggregate undeploy target.
undeploy-examplesstill points at the old fixed manifest directories, while the new workflow generates parameterized manifests. Either remove this target or make it delegate to the newundeploy-examples-*targets.🐛 Possible fix
.PHONY: undeploy-examples undeploy-examples: ## Undeploy the examples from the K8s cluster specified in ~/.kube/config. - `@echo` "Remove Namespace based GKMCache" - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/namespace/RWO/ - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/namespace/ROX/ - `@echo` "Remove Cluster based ClusterGKMCache" - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/cluster/RWO/ - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/cluster/ROX/ + $(MAKE) undeploy-examples-kind + $(MAKE) undeploy-examples-nfd-cuda + $(MAKE) undeploy-examples-nfd-rocm🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 390 - 397, The Makefile target undeploy-examples is outdated because it deletes fixed manifests under examples/namespace/* and examples/cluster/*; update it to delegate to the new parameterized targets undeploy-examples-namespace and undeploy-examples-cluster (or remove the legacy target). Edit the undeploy-examples recipe so it either calls the new targets (e.g., make undeploy-examples-namespace && make undeploy-examples-cluster) or delete the target entirely, ensuring any callers are updated to the new undeploy-examples-* targets.
🧹 Nitpick comments (2)
examples/base/access/rwo/ds-2.yaml (1)
19-27: Consider security hardening for example DaemonSets.Similar to the pod examples, the container runs without explicit security constraints. While acceptable for local examples, consider adding security context fields if these examples are used as templates:
securityContext.allowPrivilegeEscalation: falsesecurityContext.runAsNonRoot: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/base/access/rwo/ds-2.yaml` around lines 19 - 27, The DaemonSet's container "test" currently has no securityContext; add a container-level securityContext to the pod template (for the container named "test") with allowPrivilegeEscalation: false and runAsNonRoot: true (and consider adding a pod-level securityContext if you want to enforce UID, e.g., runAsUser) to harden the example; update the pod template spec where the containers list and volumeMounts are defined so the new securityContext sits alongside the existing fields for the "test" container.examples/base/access/rox/pod-2.yaml (1)
8-16: Consider security hardening for example pods.The pod runs without explicit security constraints. While acceptable for local examples, consider adding security context fields if these examples are used as templates:
securityContext.allowPrivilegeEscalation: falsesecurityContext.runAsNonRoot: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/base/access/rox/pod-2.yaml` around lines 8 - 16, The pod spec for the container named "test" lacks security constraints; update the Pod/Container spec by adding a securityContext (either at pod level or container level for the container with name "test") that sets allowPrivilegeEscalation: false and runAsNonRoot: true (reference the "containers" list and the "name: test" container and ensure the change coexists with existing volumeMounts/kernel-volume and volumes entries).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/base/access/rox/pod-1.yaml`:
- Around line 8-19: The example pod's container named "test" is missing a
securityContext which can cause failures in restricted clusters; add a hardened
container securityContext under the "test" container (and add a pod-level
securityContext if needed) that enforces runAsNonRoot and a non-root UID (e.g.,
runAsUser: 1000), sets allowPrivilegeEscalation: false, drops all capabilities,
sets readOnlyRootFilesystem: true, ensures privileged: false, and adds a
seccompProfile (runtime/default) to demonstrate best-practice hardening for
containers and to avoid admission denials in restrictive environments.
In `@examples/base/access/rwo/ds-1.yaml`:
- Around line 19-31: The DaemonSet's container "test" lacks container-level
security hardening; update the Pod template so the container (containers: -
name: test) has a securityContext that sets allowPrivilegeEscalation: false,
readOnlyRootFilesystem: true, drops all capabilities (capabilities.drop:
["ALL"]), and enforces non-root execution (runAsNonRoot: true and a non-zero
runAsUser), and also add a podSecurityContext at the template.spec level to
reinforce runAsNonRoot if appropriate; keep the existing volume (name:
kernel-volume / persistentVolumeClaim.claimName: gkm-test-obj) unchanged.
In `@examples/generate-files.sh`:
- Around line 467-471: The script fails to ensure OUTPUT_DIR exists before
redirecting to OUTPUT_FILENAME; add a step to create the directory (e.g., run
mkdir -p "${OUTPUT_DIR}") before the first ${KUSTOMIZE} build/redirection so the
redirect to ${OUTPUT_FILENAME} cannot fail; keep this creation early in the
block that uses OUTPUT_FILENAME and preserve the existing error handling on
${KUSTOMIZE} failures.
- Around line 246-254: The patch is creating affinity as a list (adding an empty
array and appending) which is invalid for PodSpec.affinity; update the
POD_TEMPLATE_ADD_GKMCACHE modification so it adds
/spec/podTemplate/spec/affinity with a single object value using
AFFINITY_INSTANCE (i.e. remove the "value: []" and the "/-" append pattern and
set "value: ${AFFINITY_INSTANCE}" directly), and apply the same change to the
other analogous block (lines ~290-298) so both places create affinity as an
object, not a list.
In `@Makefile`:
- Around line 344-353: The gen-apply-example Makefile target can fail because
generate-files.sh expects ../bin/kustomize but the target doesn't ensure
kustomize is installed; update the gen-apply-example target to ensure kustomize
is present before calling generate-files.sh (for example add a prerequisite or
invocation of an existing kustomize installation target or a short check/install
step that creates ../bin/kustomize), so that running generate-files.sh
$(EXAMPLE_ACCESS) $(EXAMPLE_SCOPE) $(EXAMPLE_GPU) $(EXAMPLE_VER) $(EXAMPLE_ENV)
always has ../bin/kustomize available while preserving the existing use of
KUBECTL and the EXAMPLE_* variables.
---
Outside diff comments:
In `@Makefile`:
- Around line 390-397: The Makefile target undeploy-examples is outdated because
it deletes fixed manifests under examples/namespace/* and examples/cluster/*;
update it to delegate to the new parameterized targets
undeploy-examples-namespace and undeploy-examples-cluster (or remove the legacy
target). Edit the undeploy-examples recipe so it either calls the new targets
(e.g., make undeploy-examples-namespace && make undeploy-examples-cluster) or
delete the target entirely, ensuring any callers are updated to the new
undeploy-examples-* targets.
---
Nitpick comments:
In `@examples/base/access/rox/pod-2.yaml`:
- Around line 8-16: The pod spec for the container named "test" lacks security
constraints; update the Pod/Container spec by adding a securityContext (either
at pod level or container level for the container with name "test") that sets
allowPrivilegeEscalation: false and runAsNonRoot: true (reference the
"containers" list and the "name: test" container and ensure the change coexists
with existing volumeMounts/kernel-volume and volumes entries).
In `@examples/base/access/rwo/ds-2.yaml`:
- Around line 19-27: The DaemonSet's container "test" currently has no
securityContext; add a container-level securityContext to the pod template (for
the container named "test") with allowPrivilegeEscalation: false and
runAsNonRoot: true (and consider adding a pod-level securityContext if you want
to enforce UID, e.g., runAsUser) to harden the example; update the pod template
spec where the containers list and volumeMounts are defined so the new
securityContext sits alongside the existing fields for the "test" container.
🪄 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: Pro Plus
Run ID: 1f281712-d7e5-4021-8eb9-37b59e116d5d
📒 Files selected for processing (51)
Makefileexamples/.gitignoreexamples/base/access/rox/kustomization.yamlexamples/base/access/rox/pod-1.yamlexamples/base/access/rox/pod-2.yamlexamples/base/access/rox/pod-3.yamlexamples/base/access/rwo/ds-1.yamlexamples/base/access/rwo/ds-2.yamlexamples/base/access/rwo/ds-3.yamlexamples/base/access/rwo/kustomization.yamlexamples/base/common/kustomization.yamlexamples/base/common/namespace-1.envexamples/base/scope/cluster/clustergkmcache.yamlexamples/base/scope/cluster/kustomization.yamlexamples/base/scope/cluster/namespace-2.envexamples/base/scope/namespace/gkmcache.yamlexamples/base/scope/namespace/kustomization.yamlexamples/cleanup-files.shexamples/cluster/ROX/10-namespace.yamlexamples/cluster/ROX/11-clustergkmcache.yamlexamples/cluster/ROX/12-pod.yamlexamples/cluster/ROX/13-pod.yamlexamples/cluster/ROX/14-pod.yamlexamples/cluster/RWO/10-namespace.yamlexamples/cluster/RWO/11-clustergkmcache.yamlexamples/cluster/RWO/12-ds.yamlexamples/cluster/RWO/13-ds.yamlexamples/cluster/RWO/14-ds.yamlexamples/cluster/RWO/21-clustergkmcache-cosign-v3.yamlexamples/cluster/RWO/22-ds.yamlexamples/generate-files.shexamples/namespace/ROX/11-gkmcache.yamlexamples/namespace/ROX/12-pod.yamlexamples/namespace/ROX/13-pod.yamlexamples/namespace/ROX/14-pod.yamlexamples/namespace/RWO/11-gkmcache.yamlexamples/namespace/RWO/12-ds.yamlexamples/namespace/RWO/13-ds.yamlexamples/namespace/RWO/14-ds.yamlexamples/namespace/RWO/21-gkmcache-cosign-v3.yamlexamples/namespace/RWO/22-ds.yamlexamples/patch/affinity-nfd-cuda.txtexamples/patch/affinity-nfd-rocm.txtexamples/patch/node-selector-kind-false.txtexamples/patch/node-selector-kind-true.txtexamples/patch/toleration-kind.txtexamples/patch/toleration-nfd-cuda.txtexamples/variants/access/rox/kustomization.envexamples/variants/access/rwo/kustomization.envexamples/variants/scope/cluster/kustomization.envexamples/variants/scope/namespace/kustomization.env
💤 Files with no reviewable changes (22)
- examples/namespace/RWO/12-ds.yaml
- examples/cluster/ROX/14-pod.yaml
- examples/cluster/ROX/11-clustergkmcache.yaml
- examples/namespace/ROX/11-gkmcache.yaml
- examples/cluster/ROX/10-namespace.yaml
- examples/namespace/ROX/14-pod.yaml
- examples/cluster/RWO/21-clustergkmcache-cosign-v3.yaml
- examples/cluster/RWO/11-clustergkmcache.yaml
- examples/namespace/RWO/11-gkmcache.yaml
- examples/cluster/RWO/14-ds.yaml
- examples/cluster/RWO/13-ds.yaml
- examples/cluster/ROX/12-pod.yaml
- examples/cluster/RWO/12-ds.yaml
- examples/cluster/ROX/13-pod.yaml
- examples/namespace/RWO/14-ds.yaml
- examples/namespace/RWO/22-ds.yaml
- examples/namespace/RWO/21-gkmcache-cosign-v3.yaml
- examples/namespace/ROX/13-pod.yaml
- examples/cluster/RWO/10-namespace.yaml
- examples/cluster/RWO/22-ds.yaml
- examples/namespace/ROX/12-pod.yaml
- examples/namespace/RWO/13-ds.yaml
d58226c to
5fa7755
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
390-397:⚠️ Potential issue | 🟠 MajorStale
undeploy-examplestarget references removed directories.The counterpart
deploy-examplestarget (and the staticexamples/namespace/{RWO,ROX}/andexamples/cluster/{RWO,ROX}/directories it relied on) was removed in this PR, but thisundeploy-examplestarget still shells out to them. Invoking it will fail witherror: the path "examples/namespace/RWO/" does not exist—--ignore-not-foundonly suppresses missing Kubernetes resources, not missing-fpaths.Either delete this target outright or have it fan out to the new parameterized undeploys.
🐛 Proposed fix
-.PHONY: undeploy-examples -undeploy-examples: ## Undeploy the examples from the K8s cluster specified in ~/.kube/config. - `@echo` "Remove Namespace based GKMCache" - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/namespace/RWO/ - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/namespace/ROX/ - `@echo` "Remove Cluster based ClusterGKMCache" - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/cluster/RWO/ - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/cluster/ROX/ +.PHONY: undeploy-examples +undeploy-examples: undeploy-examples-kind undeploy-examples-nfd-cuda undeploy-examples-nfd-rocm ## Undeploy all example variants from the K8s cluster specified in ~/.kube/config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 390 - 397, The Makefile's undeploy-examples target still references removed directories (examples/namespace/{RWO,ROX}/ and examples/cluster/{RWO,ROX}/) and will fail because kubectl -f paths must exist; update the undeploy-examples rule to either be removed or rewritten to call the new parameterized undeploy targets (instead of directly using the deleted paths), e.g., drop the target entirely if unused, or replace the four $(KUBECTL) delete -f ... lines with invocations of the new per-example undeploy targets and preserve the $(ignore-not-found) usage so missing K8s resources are tolerated; ensure you update references to the undeploy-examples target and any docs/Makefile help text accordingly.
🧹 Nitpick comments (1)
examples/generate-files.sh (1)
453-458: Remove dead commented code.This block is superseded by the
ACCESS_ROX_ACCESS_MODEsed substitution on line 449 and can be deleted.✂️ Proposed cleanup
- -# If using ReadOnlyMany (rox), then add "ReadOnlyMany" to the GKMCache or ClusterGKMCache -# AccessMode field. -#if [[ "$ACCESS" == "rox" ]]; then -# echo "${ACCESS_ROX_ACCESS_MODE}" >> kustomization.yaml -#fi popd > /dev/null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/generate-files.sh` around lines 453 - 458, Delete the dead commented conditional block that adds "ReadOnlyMany" to kustomization.yaml (the commented lines starting with `#if` [[ "$ACCESS" == "rox" ]]; then ... `#fi`) since it is superseded by the ACCESS_ROX_ACCESS_MODE sed substitution; remove only those commented lines and leave the surrounding logic (including popd > /dev/null) intact so the ACCESS_ROX_ACCESS_MODE sed replacement remains the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/generate-files.sh`:
- Line 389: The comment above the multiline handling block incorrectly
references NODE_SELECTOR_2 while the code operates on NODE_SELECTOR_3; update
the comment to mention NODE_SELECTOR_3 (or adjust to correctly reflect which
variable the block handles) so the comment and the code for NODE_SELECTOR_3 are
consistent.
---
Outside diff comments:
In `@Makefile`:
- Around line 390-397: The Makefile's undeploy-examples target still references
removed directories (examples/namespace/{RWO,ROX}/ and
examples/cluster/{RWO,ROX}/) and will fail because kubectl -f paths must exist;
update the undeploy-examples rule to either be removed or rewritten to call the
new parameterized undeploy targets (instead of directly using the deleted
paths), e.g., drop the target entirely if unused, or replace the four $(KUBECTL)
delete -f ... lines with invocations of the new per-example undeploy targets and
preserve the $(ignore-not-found) usage so missing K8s resources are tolerated;
ensure you update references to the undeploy-examples target and any
docs/Makefile help text accordingly.
---
Nitpick comments:
In `@examples/generate-files.sh`:
- Around line 453-458: Delete the dead commented conditional block that adds
"ReadOnlyMany" to kustomization.yaml (the commented lines starting with `#if` [[
"$ACCESS" == "rox" ]]; then ... `#fi`) since it is superseded by the
ACCESS_ROX_ACCESS_MODE sed substitution; remove only those commented lines and
leave the surrounding logic (including popd > /dev/null) intact so the
ACCESS_ROX_ACCESS_MODE sed replacement remains the single source of truth.
🪄 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: 6fbf5957-b4b9-4a5d-a916-6e87ed7777b1
📒 Files selected for processing (54)
Makefileconfig/agent/kustomization.yamlconfig/configMap/kustomization.yamlconfig/operator/kustomization.yamlexamples/.gitignoreexamples/base/access/rox/kustomization.yamlexamples/base/access/rox/pod-1.yamlexamples/base/access/rox/pod-2.yamlexamples/base/access/rox/pod-3.yamlexamples/base/access/rwo/ds-1.yamlexamples/base/access/rwo/ds-2.yamlexamples/base/access/rwo/ds-3.yamlexamples/base/access/rwo/kustomization.yamlexamples/base/common/kustomization.yamlexamples/base/common/namespace-1.envexamples/base/scope/cluster/clustergkmcache.yamlexamples/base/scope/cluster/kustomization.yamlexamples/base/scope/cluster/namespace-2.envexamples/base/scope/namespace/gkmcache.yamlexamples/base/scope/namespace/kustomization.yamlexamples/cleanup-files.shexamples/cluster/ROX/10-namespace.yamlexamples/cluster/ROX/11-clustergkmcache.yamlexamples/cluster/ROX/12-pod.yamlexamples/cluster/ROX/13-pod.yamlexamples/cluster/ROX/14-pod.yamlexamples/cluster/RWO/10-namespace.yamlexamples/cluster/RWO/11-clustergkmcache.yamlexamples/cluster/RWO/12-ds.yamlexamples/cluster/RWO/13-ds.yamlexamples/cluster/RWO/14-ds.yamlexamples/cluster/RWO/21-clustergkmcache-cosign-v3.yamlexamples/cluster/RWO/22-ds.yamlexamples/generate-files.shexamples/namespace/ROX/11-gkmcache.yamlexamples/namespace/ROX/12-pod.yamlexamples/namespace/ROX/13-pod.yamlexamples/namespace/ROX/14-pod.yamlexamples/namespace/RWO/11-gkmcache.yamlexamples/namespace/RWO/12-ds.yamlexamples/namespace/RWO/13-ds.yamlexamples/namespace/RWO/14-ds.yamlexamples/namespace/RWO/21-gkmcache-cosign-v3.yamlexamples/namespace/RWO/22-ds.yamlexamples/patch/affinity-nfd-cuda.txtexamples/patch/affinity-nfd-rocm.txtexamples/patch/node-selector-kind-false.txtexamples/patch/node-selector-kind-true.txtexamples/patch/toleration-kind.txtexamples/patch/toleration-nfd-cuda.txtexamples/variants/access/rox/kustomization.envexamples/variants/access/rwo/kustomization.envexamples/variants/scope/cluster/kustomization.envexamples/variants/scope/namespace/kustomization.env
💤 Files with no reviewable changes (22)
- examples/cluster/ROX/14-pod.yaml
- examples/namespace/ROX/14-pod.yaml
- examples/namespace/ROX/11-gkmcache.yaml
- examples/cluster/ROX/10-namespace.yaml
- examples/cluster/RWO/14-ds.yaml
- examples/cluster/RWO/21-clustergkmcache-cosign-v3.yaml
- examples/cluster/RWO/11-clustergkmcache.yaml
- examples/namespace/RWO/22-ds.yaml
- examples/namespace/RWO/21-gkmcache-cosign-v3.yaml
- examples/namespace/RWO/13-ds.yaml
- examples/namespace/ROX/13-pod.yaml
- examples/cluster/ROX/12-pod.yaml
- examples/cluster/ROX/13-pod.yaml
- examples/namespace/RWO/12-ds.yaml
- examples/cluster/RWO/12-ds.yaml
- examples/namespace/RWO/14-ds.yaml
- examples/namespace/ROX/12-pod.yaml
- examples/cluster/RWO/22-ds.yaml
- examples/cluster/RWO/10-namespace.yaml
- examples/cluster/RWO/13-ds.yaml
- examples/namespace/RWO/11-gkmcache.yaml
- examples/cluster/ROX/11-clustergkmcache.yaml
✅ Files skipped from review due to trivial changes (26)
- examples/patch/node-selector-kind-true.txt
- examples/patch/toleration-kind.txt
- examples/patch/toleration-nfd-cuda.txt
- config/operator/kustomization.yaml
- examples/patch/node-selector-kind-false.txt
- examples/base/common/kustomization.yaml
- config/agent/kustomization.yaml
- examples/base/scope/namespace/kustomization.yaml
- config/configMap/kustomization.yaml
- examples/base/access/rox/kustomization.yaml
- examples/base/access/rwo/kustomization.yaml
- examples/base/scope/cluster/namespace-2.env
- examples/base/scope/namespace/gkmcache.yaml
- examples/.gitignore
- examples/base/scope/cluster/clustergkmcache.yaml
- examples/patch/affinity-nfd-cuda.txt
- examples/cleanup-files.sh
- examples/base/access/rox/pod-1.yaml
- examples/patch/affinity-nfd-rocm.txt
- examples/base/access/rox/pod-2.yaml
- examples/base/common/namespace-1.env
- examples/base/access/rwo/ds-1.yaml
- examples/base/scope/cluster/kustomization.yaml
- examples/base/access/rwo/ds-3.yaml
- examples/base/access/rox/pod-3.yaml
- examples/variants/access/rwo/kustomization.env
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/variants/scope/cluster/kustomization.env
- examples/base/access/rwo/ds-2.yaml
- examples/variants/scope/namespace/kustomization.env
a1e5823 to
f3758ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
390-397:⚠️ Potential issue | 🟡 MinorDead
undeploy-examplestarget referencing removed directories.This target deletes from
examples/namespace/RWO/,examples/namespace/ROX/,examples/cluster/RWO/,examples/cluster/ROX/— directories removed by this PR. With--ignore-not-found=truethe target succeeds silently but does nothing useful, and is now misleading alongside the newundeploy-examples-kind/-nfd-*targets. Consider removing it (or re-pointing it to the new generator workflow).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 390 - 397, The Makefile's undeploy-examples target references deleted example directories (examples/namespace/RWO, examples/namespace/ROX, examples/cluster/RWO, examples/cluster/ROX) and should be removed or repointed; delete the undeploy-examples target (and its use of $(KUBECTL) and $(ignore-not-found)) or update it to call the new generator workflow/targets (e.g., the new undeploy-examples-kind / undeploy-examples-nfd-* targets) so the target no longer silently references non-existent paths.
🧹 Nitpick comments (1)
Makefile (1)
344-353: Generated YAML is left behind after apply/delete.
gen-apply-examplecallsgenerate-files.shwhich writes toexamples/output/*.yamleach invocation. Forundeploy-examples-*, the generator is re-run just to rebuild the same YAML in order to delete it; afterwards the file stays in the tree. Consider either invokingexamples/cleanup-files.shat the end of the undeploy targets, or capturing the output filename andrming it post-apply/delete in the recipe, to keep the workspace clean. Not a correctness issue — purely hygiene.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 344 - 353, The gen-apply-example recipe leaves generated YAML in examples/output; change the recipe to capture the EXAMPLE_YAML returned by generate-files.sh (the existing EXAMPLE_YAML variable) and remove that file after running $(KUBECTL) (i.e., rm $$EXAMPLE_YAML), or alternatively invoke examples/cleanup-files.sh at the end of the recipe; apply the same cleanup approach to the undeploy-examples-* targets which re-run generate-files.sh so generated YAML is removed after delete.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/Examples.md`:
- Around line 273-278: The docs examples list wrong environment values for the
deploy-examples-nfd-rocm invocations: update the two lines describing the script
runs so they reflect EXAMPLE_ENV=nfd (not kind) to match the Makefile target
deploy-examples-nfd-rocm; specifically, when describing the two calls to
examples/generate-files.sh that currently show `kind`, replace those `kind`
entries with `nfd` so the parameter list matches EXAMPLE_ENV=nfd used by the
deploy-examples-nfd-rocm target.
- Around line 200-204: The sentence currently reads "Node of the files that are
generated by the `examples/generate-files.sh` script" and contains a typo;
update that text to "None of the files that are generated by the
`examples/generate-files.sh` script" so the meaning is correct—locate the exact
sentence in Examples.md and replace "Node" with "None".
In `@docs/GettingStartedGuide.md`:
- Around line 92-94: Replace the broken intra-document anchor "[Examples
Directory](`#examples-directory`)" in GettingStartedGuide.md with a correct
cross-document link to the actual Examples page (e.g., change to "[Examples
Directory](./Examples.md)" or the repository URL to docs/Examples.md) so the
MD051 pre-commit check passes; locate the link text "[Examples Directory]" in
GettingStartedGuide.md and update it accordingly.
In `@examples/generate-files.sh`:
- Line 9: The help text currently prints "outputs/" but the script uses
OUTPUT_DIR="output" and other references use "output/"; update the help/usage
echo to mention "output/" so it matches the OUTPUT_DIR constant and
Makefile/docs (change the echo on the usage line that currently says "outputs/"
to "output/").
- Around line 11-21: The help text in generate-files.sh incorrectly describes
the positional parameters SCOPE and ACCESS (it lists SCOPE as "rox|rwo" and
ACCESS as "namespace|cluster") while the script assigns ACCESS=$1 and SCOPE=$2
and validates ACCESS for "rox|rwo" and SCOPE for "cluster|namespace"; update the
help block strings in generate-files.sh so <SCOPE> is documented as "cluster or
namespace" and <ACCESS> as "rox or rwo" (matching the variables ACCESS and SCOPE
and their validations), and apply the same correction to the mirrored text in
docs/Examples.md lines mentioned to keep documentation consistent with the
Makefile/usage.
- Around line 134-137: The error message for the ENVIRONMENT branch is
incorrect—update the echo in the elif block that checks "$ENVIRONMENT" to list
both valid values ("kind" and "nfd") (i.e., change the message currently saying
Parameter 5 must be "kind" to say it must be "kind" or "nfd") so the error
accurately reflects accepted ENVIRONMENT values.
---
Outside diff comments:
In `@Makefile`:
- Around line 390-397: The Makefile's undeploy-examples target references
deleted example directories (examples/namespace/RWO, examples/namespace/ROX,
examples/cluster/RWO, examples/cluster/ROX) and should be removed or repointed;
delete the undeploy-examples target (and its use of $(KUBECTL) and
$(ignore-not-found)) or update it to call the new generator workflow/targets
(e.g., the new undeploy-examples-kind / undeploy-examples-nfd-* targets) so the
target no longer silently references non-existent paths.
---
Nitpick comments:
In `@Makefile`:
- Around line 344-353: The gen-apply-example recipe leaves generated YAML in
examples/output; change the recipe to capture the EXAMPLE_YAML returned by
generate-files.sh (the existing EXAMPLE_YAML variable) and remove that file
after running $(KUBECTL) (i.e., rm $$EXAMPLE_YAML), or alternatively invoke
examples/cleanup-files.sh at the end of the recipe; apply the same cleanup
approach to the undeploy-examples-* targets which re-run generate-files.sh so
generated YAML is removed after delete.
🪄 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: 87f35e06-7815-4813-a5a8-8aff216fb792
📒 Files selected for processing (57)
Makefileconfig/agent/kustomization.yamlconfig/configMap/kustomization.yamlconfig/operator/kustomization.yamldocs/DeploymentOptions.mddocs/Examples.mddocs/GettingStartedGuide.mdexamples/.gitignoreexamples/base/access/rox/kustomization.yamlexamples/base/access/rox/pod-1.yamlexamples/base/access/rox/pod-2.yamlexamples/base/access/rox/pod-3.yamlexamples/base/access/rwo/ds-1.yamlexamples/base/access/rwo/ds-2.yamlexamples/base/access/rwo/ds-3.yamlexamples/base/access/rwo/kustomization.yamlexamples/base/common/kustomization.yamlexamples/base/common/namespace-1.envexamples/base/scope/cluster/clustergkmcache.yamlexamples/base/scope/cluster/kustomization.yamlexamples/base/scope/cluster/namespace-2.envexamples/base/scope/namespace/gkmcache.yamlexamples/base/scope/namespace/kustomization.yamlexamples/cleanup-files.shexamples/cluster/ROX/10-namespace.yamlexamples/cluster/ROX/11-clustergkmcache.yamlexamples/cluster/ROX/12-pod.yamlexamples/cluster/ROX/13-pod.yamlexamples/cluster/ROX/14-pod.yamlexamples/cluster/RWO/10-namespace.yamlexamples/cluster/RWO/11-clustergkmcache.yamlexamples/cluster/RWO/12-ds.yamlexamples/cluster/RWO/13-ds.yamlexamples/cluster/RWO/14-ds.yamlexamples/cluster/RWO/21-clustergkmcache-cosign-v3.yamlexamples/cluster/RWO/22-ds.yamlexamples/generate-files.shexamples/namespace/ROX/11-gkmcache.yamlexamples/namespace/ROX/12-pod.yamlexamples/namespace/ROX/13-pod.yamlexamples/namespace/ROX/14-pod.yamlexamples/namespace/RWO/11-gkmcache.yamlexamples/namespace/RWO/12-ds.yamlexamples/namespace/RWO/13-ds.yamlexamples/namespace/RWO/14-ds.yamlexamples/namespace/RWO/21-gkmcache-cosign-v3.yamlexamples/namespace/RWO/22-ds.yamlexamples/patch/affinity-nfd-cuda.txtexamples/patch/affinity-nfd-rocm.txtexamples/patch/node-selector-kind-false.txtexamples/patch/node-selector-kind-true.txtexamples/patch/toleration-kind.txtexamples/patch/toleration-nfd-cuda.txtexamples/variants/access/rox/kustomization.envexamples/variants/access/rwo/kustomization.envexamples/variants/scope/cluster/kustomization.envexamples/variants/scope/namespace/kustomization.env
💤 Files with no reviewable changes (22)
- examples/cluster/RWO/10-namespace.yaml
- examples/cluster/RWO/21-clustergkmcache-cosign-v3.yaml
- examples/namespace/RWO/12-ds.yaml
- examples/namespace/ROX/11-gkmcache.yaml
- examples/cluster/ROX/14-pod.yaml
- examples/namespace/ROX/12-pod.yaml
- examples/cluster/RWO/13-ds.yaml
- examples/namespace/RWO/11-gkmcache.yaml
- examples/cluster/ROX/10-namespace.yaml
- examples/namespace/RWO/22-ds.yaml
- examples/cluster/RWO/12-ds.yaml
- examples/namespace/ROX/14-pod.yaml
- examples/cluster/RWO/22-ds.yaml
- examples/cluster/RWO/14-ds.yaml
- examples/cluster/ROX/13-pod.yaml
- examples/namespace/ROX/13-pod.yaml
- examples/cluster/ROX/12-pod.yaml
- examples/namespace/RWO/21-gkmcache-cosign-v3.yaml
- examples/namespace/RWO/14-ds.yaml
- examples/namespace/RWO/13-ds.yaml
- examples/cluster/RWO/11-clustergkmcache.yaml
- examples/cluster/ROX/11-clustergkmcache.yaml
✅ Files skipped from review due to trivial changes (24)
- examples/base/scope/namespace/kustomization.yaml
- examples/base/common/kustomization.yaml
- examples/patch/node-selector-kind-true.txt
- examples/patch/node-selector-kind-false.txt
- examples/patch/toleration-kind.txt
- examples/base/access/rwo/kustomization.yaml
- examples/base/scope/cluster/kustomization.yaml
- examples/base/access/rox/kustomization.yaml
- examples/patch/affinity-nfd-cuda.txt
- examples/base/scope/namespace/gkmcache.yaml
- config/configMap/kustomization.yaml
- config/operator/kustomization.yaml
- examples/patch/affinity-nfd-rocm.txt
- examples/base/scope/cluster/namespace-2.env
- examples/patch/toleration-nfd-cuda.txt
- examples/base/scope/cluster/clustergkmcache.yaml
- examples/base/access/rox/pod-1.yaml
- examples/base/common/namespace-1.env
- examples/base/access/rwo/ds-1.yaml
- examples/base/access/rwo/ds-2.yaml
- examples/base/access/rox/pod-3.yaml
- examples/variants/scope/cluster/kustomization.env
- examples/base/access/rwo/ds-3.yaml
- examples/variants/access/rox/kustomization.env
🚧 Files skipped from review as they are similar to previous changes (5)
- examples/.gitignore
- config/agent/kustomization.yaml
- examples/base/access/rox/pod-2.yaml
- examples/variants/scope/namespace/kustomization.env
- examples/variants/access/rwo/kustomization.env
91932e8 to
c7a9ece
Compare
c7a9ece to
2b2830d
Compare
Done |
667d2c7 to
c7110b3
Compare
c5ed9bc to
e1003d6
Compare
cmagina
left a comment
There was a problem hiding this comment.
Found an issue with the generate-files script and a minor type in the new readme.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
390-397:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
undeploy-examplesis broken after static manifests are removed.This target references
examples/namespace/RWO/,examples/namespace/ROX/,examples/cluster/RWO/, andexamples/cluster/ROX/— the exact static manifest directories that this PR removes.kubectl delete --ignore-not-found -f <dir>does not suppress filesystem "path does not exist" errors; it only ignores API-server 404s. Runningmake undeploy-examplesafter this PR merges will fail immediately.Either remove this target (since the new
undeploy-examples-kind/nfd-*targets cover the use cases) or replace its body to use the generated YAMLs, e.g.:.PHONY: undeploy-examples -undeploy-examples: ## Undeploy the examples from the K8s cluster specified in ~/.kube/config. - `@echo` "Remove Namespace based GKMCache" - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/namespace/RWO/ - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/namespace/ROX/ - `@echo` "Remove Cluster based ClusterGKMCache" - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/cluster/RWO/ - $(KUBECTL) delete --ignore-not-found=$(ignore-not-found) -f examples/cluster/ROX/ +undeploy-examples: undeploy-examples-kind undeploy-examples-nfd-cuda undeploy-examples-nfd-rocm ## Undeploy all generated examples from the K8s cluster.(Adjust the chained targets to match whichever combination was actually deployed.)
🤖 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 `@Makefile` around lines 390 - 397, The undeploy-examples Makefile target still references removed static manifest directories (targets/paths in the body: examples/namespace/RWO/, examples/namespace/ROX/, examples/cluster/RWO/, examples/cluster/ROX/) so running make undeploy-examples will fail; either delete the undeploy-examples target entirely or rewrite its recipe to call the new undeploy targets (e.g., undeploy-examples-kind and any nfd-* undeploy targets) or to delete the generated YAMLs instead of the removed static directories, and update any uses of $(KUBECTL) and --ignore-not-found to operate against files that now exist (or rely on the new chained targets) so the target no longer attempts to delete non-existent paths.
♻️ Duplicate comments (1)
docs/Examples.md (1)
202-202:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
.gitignoredescription is semantically inverted.The sentence "keeps the generated files as being flagged as changed" says the opposite of what
.gitignoredoes—it prevents those files from appearing as changed. This could confuse someone debugging unexpectedgit statusoutput.📝 Proposed fix
-The `examples/.gitignore` keeps the generated files as being flagged as changed. +The `examples/.gitignore` prevents the generated files from being flagged as changed.🤖 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 `@docs/Examples.md` at line 202, The sentence in docs/Examples.md saying "keeps the generated files as being flagged as changed" is semantically inverted; update that sentence to correctly state that the examples/.gitignore prevents or ignores generated files from being flagged as changed (e.g., replace "keeps ... flagged as changed" with "prevents/generated files from being shown as changed" or similar), ensuring the meaning is that .gitignore excludes those generated files from git status output.
🧹 Nitpick comments (2)
examples/cleanup-files.sh (1)
5-9: ⚡ Quick winUse idiomatic
if ! pushd ...instead of checking$?separately.The pattern on lines 5–8 diverges from the idiomatic form used for
popdon line 24 and is flagged by ShellCheck (SC2181 / SC2164).♻️ Proposed fix
- pushd examples &>/dev/null - if [[ $? -ne 0 ]]; then - echo "ERROR: Must run from \"./GKM\" or \"./GKM/examples\"" - exit 1 - fi + if ! pushd examples &>/dev/null; then + echo "ERROR: Must run from \"./GKM\" or \"./GKM/examples\"" + exit 1 + fi🤖 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 `@examples/cleanup-files.sh` around lines 5 - 9, Replace the separate exit-status check after calling pushd with the idiomatic combined conditional: call pushd examples &>/dev/null directly inside an if ! ...; then block so the script uses "if ! pushd examples &>/dev/null; then" instead of checking "$?" afterward; keep the existing error message and exit/cleanup behavior and leave the matching popd usage unchanged.examples/generate-files.sh (1)
443-500: ⚡ Quick winAdd
|| exitto allpushd/popdcalls.If any of the
base/orvariants/directories is absent,pushdsilently fails and the subsequentsedruns in the wrong directory — potentially overwriting unrelatedkustomization.yamlfiles without any error output.🛡️ Proposed fix
-pushd ${BASE_DIR_COMMON} > /dev/null +pushd ${BASE_DIR_COMMON} > /dev/null || exit 1 ... -popd > /dev/null +popd > /dev/null || exit 1 -pushd ${BASE_DIR_SCOPE} > /dev/null +pushd ${BASE_DIR_SCOPE} > /dev/null || exit 1 ... -popd > /dev/null +popd > /dev/null || exit 1 -pushd ${VARIANTS_DIR_ACCESS} > /dev/null +pushd ${VARIANTS_DIR_ACCESS} > /dev/null || exit 1 ... -popd > /dev/null +popd > /dev/null || exit 1 -pushd ${VARIANTS_DIR_SCOPE} > /dev/null +pushd ${VARIANTS_DIR_SCOPE} > /dev/null || exit 1 ... -popd > /dev/null +popd > /dev/null || exit 1🤖 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 `@examples/generate-files.sh` around lines 443 - 500, The script uses pushd/popd (e.g., the pushd/popd calls around the ${BASE_DIR_COMMON}, ${BASE_DIR_SCOPE}, ${VARIANTS_DIR_ACCESS}, ${VARIANTS_DIR_SCOPE} blocks) without checking failure, so a missing directory will cause sed to run in the wrong folder; update every pushd and every popd invocation to immediately abort on failure by adding a fail-fast guard (e.g., after each pushd ... > /dev/null and each popd > /dev/null append a conditional like "|| exit" so the script exits if directory change fails).
🤖 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 `@docs/Examples.md`:
- Line 30: The documentation line describing Kubernetes access modes is
incorrect: update the phrase that reads "**rox** vs **rwo**: The access mode of
`ReadOnlyMany` or `ReadWriteOnly`" so it uses the correct access mode name
`ReadWriteOnce` instead of `ReadWriteOnly`, and ensure the pairing remains
"**rox** vs **rwo**" with `ReadOnlyMany` and `ReadWriteOnce` respectively
(locate the string containing "**rox** vs **rwo**" in Examples.md and replace
the invalid name).
In `@examples/generate-files.sh`:
- Line 262: The script reads user-supplied path variables unquoted (e.g.,
AFFINITY_INSTANCE=$(cat ${CUSTOM_AFFINITY})) which breaks on paths with spaces
and is unsafe; update every use of cat with variable expansions to use quoted
variables (e.g., AFFINITY_INSTANCE=$(cat "${CUSTOM_AFFINITY}") ), and similarly
quote ${CUSTOM_TOLERATION} and all ${CUSTOM_NODE_SELECTOR_*} expansions where
they are passed to cat (and any other command substitutions) to preserve spaces
and prevent word-splitting.
---
Outside diff comments:
In `@Makefile`:
- Around line 390-397: The undeploy-examples Makefile target still references
removed static manifest directories (targets/paths in the body:
examples/namespace/RWO/, examples/namespace/ROX/, examples/cluster/RWO/,
examples/cluster/ROX/) so running make undeploy-examples will fail; either
delete the undeploy-examples target entirely or rewrite its recipe to call the
new undeploy targets (e.g., undeploy-examples-kind and any nfd-* undeploy
targets) or to delete the generated YAMLs instead of the removed static
directories, and update any uses of $(KUBECTL) and --ignore-not-found to operate
against files that now exist (or rely on the new chained targets) so the target
no longer attempts to delete non-existent paths.
---
Duplicate comments:
In `@docs/Examples.md`:
- Line 202: The sentence in docs/Examples.md saying "keeps the generated files
as being flagged as changed" is semantically inverted; update that sentence to
correctly state that the examples/.gitignore prevents or ignores generated files
from being flagged as changed (e.g., replace "keeps ... flagged as changed" with
"prevents/generated files from being shown as changed" or similar), ensuring the
meaning is that .gitignore excludes those generated files from git status
output.
---
Nitpick comments:
In `@examples/cleanup-files.sh`:
- Around line 5-9: Replace the separate exit-status check after calling pushd
with the idiomatic combined conditional: call pushd examples &>/dev/null
directly inside an if ! ...; then block so the script uses "if ! pushd examples
&>/dev/null; then" instead of checking "$?" afterward; keep the existing error
message and exit/cleanup behavior and leave the matching popd usage unchanged.
In `@examples/generate-files.sh`:
- Around line 443-500: The script uses pushd/popd (e.g., the pushd/popd calls
around the ${BASE_DIR_COMMON}, ${BASE_DIR_SCOPE}, ${VARIANTS_DIR_ACCESS},
${VARIANTS_DIR_SCOPE} blocks) without checking failure, so a missing directory
will cause sed to run in the wrong folder; update every pushd and every popd
invocation to immediately abort on failure by adding a fail-fast guard (e.g.,
after each pushd ... > /dev/null and each popd > /dev/null append a conditional
like "|| exit" so the script exits if directory change fails).
🪄 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: 5aea793a-9db4-4888-aad2-6eb93c3029c9
📒 Files selected for processing (55)
MakefileREADME.mddocs/DeploymentOptions.mddocs/Examples.mddocs/GettingStartedGuide.mdexamples/.gitignoreexamples/base/access/rox/kustomization.yamlexamples/base/access/rox/pod-1.yamlexamples/base/access/rox/pod-2.yamlexamples/base/access/rox/pod-3.yamlexamples/base/access/rwo/ds-1.yamlexamples/base/access/rwo/ds-2.yamlexamples/base/access/rwo/ds-3.yamlexamples/base/access/rwo/kustomization.yamlexamples/base/common/kustomization.yamlexamples/base/common/namespace-1.envexamples/base/scope/cluster/clustergkmcache.yamlexamples/base/scope/cluster/kustomization.yamlexamples/base/scope/cluster/namespace-2.envexamples/base/scope/namespace/gkmcache.yamlexamples/base/scope/namespace/kustomization.yamlexamples/cleanup-files.shexamples/cluster/ROX/10-namespace.yamlexamples/cluster/ROX/11-clustergkmcache.yamlexamples/cluster/ROX/12-pod.yamlexamples/cluster/ROX/13-pod.yamlexamples/cluster/ROX/14-pod.yamlexamples/cluster/RWO/10-namespace.yamlexamples/cluster/RWO/11-clustergkmcache.yamlexamples/cluster/RWO/12-ds.yamlexamples/cluster/RWO/13-ds.yamlexamples/cluster/RWO/14-ds.yamlexamples/cluster/RWO/21-clustergkmcache-cosign-v3.yamlexamples/cluster/RWO/22-ds.yamlexamples/generate-files.shexamples/namespace/ROX/11-gkmcache.yamlexamples/namespace/ROX/12-pod.yamlexamples/namespace/ROX/13-pod.yamlexamples/namespace/ROX/14-pod.yamlexamples/namespace/RWO/11-gkmcache.yamlexamples/namespace/RWO/12-ds.yamlexamples/namespace/RWO/13-ds.yamlexamples/namespace/RWO/14-ds.yamlexamples/namespace/RWO/21-gkmcache-cosign-v3.yamlexamples/namespace/RWO/22-ds.yamlexamples/patch/affinity-nfd-cuda.txtexamples/patch/affinity-nfd-rocm.txtexamples/patch/node-selector-kind-false.txtexamples/patch/node-selector-kind-true.txtexamples/patch/toleration-kind.txtexamples/patch/toleration-nfd-cuda.txtexamples/variants/access/rox/kustomization.envexamples/variants/access/rwo/kustomization.envexamples/variants/scope/cluster/kustomization.envexamples/variants/scope/namespace/kustomization.env
💤 Files with no reviewable changes (22)
- examples/cluster/RWO/10-namespace.yaml
- examples/cluster/RWO/21-clustergkmcache-cosign-v3.yaml
- examples/namespace/ROX/12-pod.yaml
- examples/cluster/RWO/11-clustergkmcache.yaml
- examples/namespace/ROX/14-pod.yaml
- examples/cluster/ROX/10-namespace.yaml
- examples/namespace/RWO/22-ds.yaml
- examples/namespace/ROX/11-gkmcache.yaml
- examples/cluster/ROX/11-clustergkmcache.yaml
- examples/cluster/ROX/13-pod.yaml
- examples/cluster/RWO/13-ds.yaml
- examples/namespace/ROX/13-pod.yaml
- examples/namespace/RWO/21-gkmcache-cosign-v3.yaml
- examples/namespace/RWO/13-ds.yaml
- examples/cluster/RWO/14-ds.yaml
- examples/cluster/ROX/12-pod.yaml
- examples/namespace/RWO/12-ds.yaml
- examples/namespace/RWO/11-gkmcache.yaml
- examples/namespace/RWO/14-ds.yaml
- examples/cluster/ROX/14-pod.yaml
- examples/cluster/RWO/22-ds.yaml
- examples/cluster/RWO/12-ds.yaml
✅ Files skipped from review due to trivial changes (19)
- examples/patch/node-selector-kind-true.txt
- examples/patch/node-selector-kind-false.txt
- examples/patch/toleration-kind.txt
- examples/base/scope/cluster/namespace-2.env
- examples/base/scope/namespace/kustomization.yaml
- examples/base/scope/cluster/kustomization.yaml
- examples/patch/toleration-nfd-cuda.txt
- examples/base/common/kustomization.yaml
- examples/.gitignore
- examples/base/access/rox/kustomization.yaml
- examples/base/common/namespace-1.env
- examples/base/access/rwo/kustomization.yaml
- examples/base/access/rox/pod-2.yaml
- examples/base/access/rox/pod-3.yaml
- examples/base/access/rox/pod-1.yaml
- examples/base/scope/namespace/gkmcache.yaml
- examples/base/scope/cluster/clustergkmcache.yaml
- examples/base/access/rwo/ds-1.yaml
- examples/base/access/rwo/ds-3.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/patch/affinity-nfd-cuda.txt
- examples/patch/affinity-nfd-rocm.txt
- docs/GettingStartedGuide.md
- examples/variants/scope/namespace/kustomization.env
f152936 to
ba0898c
Compare
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 `@examples/generate-files.sh`:
- Around line 166-191: The script currently writes kustomize overlays directly
into fixed directories (e.g., OVERLAY_DIR_SCOPE, OVERLAY_DIR_ACCESS) causing
races between concurrent runs; change generation to a per-run isolated workspace
by creating a temporary directory (mktemp -d) and writing all generated YAML
under that temp directory (use new temp-based paths for OVERLAY_DIR_SCOPE and
OVERLAY_DIR_ACCESS and any other overlay/base/variant dirs referenced around
166-191 and 442-518), ensure the rest of the script uses those temp paths for
kustomize build, add a trap to clean up the temp directory on exit or keep
option to persist for debugging, and update any variable assignments or
references that set BASE_DIR_*, VARIANTS_DIR_*, or OVERLAY_DIR_* so they point
into the temp workspace for the lifetime of the run.
- Around line 260-279: The script currently uses cat to load user files (e.g.,
CUSTOM_AFFINITY, CUSTOM_TOLERATION, CUSTOM_NODE_SELECTOR_*) but does not fail if
those files are missing/unreadable; update the loading logic to fail fast by
verifying the file is present and readable before cat (e.g., check [[ -r
"${CUSTOM_AFFINITY}" ]] / [[ -r "${CUSTOM_TOLERATION}" ]] and exit with a clear
error if not), or enable strict mode (set -euo pipefail) at the top of the
script and still validate readability for user-specified files; ensure the error
message names the variable (CUSTOM_AFFINITY/CUSTOM_TOLERATION/etc.) so a typo is
obvious and do not silently continue when the file check fails.
🪄 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: 181303f9-6ec4-4ab5-b9d2-fca45751b32d
📒 Files selected for processing (55)
MakefileREADME.mddocs/DeploymentOptions.mddocs/Examples.mddocs/GettingStartedGuide.mdexamples/.gitignoreexamples/base/access/rox/kustomization.yamlexamples/base/access/rox/pod-1.yamlexamples/base/access/rox/pod-2.yamlexamples/base/access/rox/pod-3.yamlexamples/base/access/rwo/ds-1.yamlexamples/base/access/rwo/ds-2.yamlexamples/base/access/rwo/ds-3.yamlexamples/base/access/rwo/kustomization.yamlexamples/base/common/kustomization.yamlexamples/base/common/namespace-1.envexamples/base/scope/cluster/clustergkmcache.yamlexamples/base/scope/cluster/kustomization.yamlexamples/base/scope/cluster/namespace-2.envexamples/base/scope/namespace/gkmcache.yamlexamples/base/scope/namespace/kustomization.yamlexamples/cleanup-files.shexamples/cluster/ROX/10-namespace.yamlexamples/cluster/ROX/11-clustergkmcache.yamlexamples/cluster/ROX/12-pod.yamlexamples/cluster/ROX/13-pod.yamlexamples/cluster/ROX/14-pod.yamlexamples/cluster/RWO/10-namespace.yamlexamples/cluster/RWO/11-clustergkmcache.yamlexamples/cluster/RWO/12-ds.yamlexamples/cluster/RWO/13-ds.yamlexamples/cluster/RWO/14-ds.yamlexamples/cluster/RWO/21-clustergkmcache-cosign-v3.yamlexamples/cluster/RWO/22-ds.yamlexamples/generate-files.shexamples/namespace/ROX/11-gkmcache.yamlexamples/namespace/ROX/12-pod.yamlexamples/namespace/ROX/13-pod.yamlexamples/namespace/ROX/14-pod.yamlexamples/namespace/RWO/11-gkmcache.yamlexamples/namespace/RWO/12-ds.yamlexamples/namespace/RWO/13-ds.yamlexamples/namespace/RWO/14-ds.yamlexamples/namespace/RWO/21-gkmcache-cosign-v3.yamlexamples/namespace/RWO/22-ds.yamlexamples/patch/affinity-nfd-cuda.txtexamples/patch/affinity-nfd-rocm.txtexamples/patch/node-selector-kind-false.txtexamples/patch/node-selector-kind-true.txtexamples/patch/toleration-kind.txtexamples/patch/toleration-nfd-cuda.txtexamples/variants/access/rox/kustomization.envexamples/variants/access/rwo/kustomization.envexamples/variants/scope/cluster/kustomization.envexamples/variants/scope/namespace/kustomization.env
💤 Files with no reviewable changes (22)
- examples/cluster/ROX/14-pod.yaml
- examples/cluster/ROX/10-namespace.yaml
- examples/cluster/RWO/10-namespace.yaml
- examples/namespace/ROX/11-gkmcache.yaml
- examples/cluster/RWO/21-clustergkmcache-cosign-v3.yaml
- examples/cluster/RWO/12-ds.yaml
- examples/cluster/ROX/12-pod.yaml
- examples/namespace/RWO/21-gkmcache-cosign-v3.yaml
- examples/cluster/RWO/11-clustergkmcache.yaml
- examples/namespace/RWO/14-ds.yaml
- examples/namespace/ROX/14-pod.yaml
- examples/cluster/RWO/22-ds.yaml
- examples/cluster/ROX/11-clustergkmcache.yaml
- examples/namespace/RWO/22-ds.yaml
- examples/cluster/ROX/13-pod.yaml
- examples/namespace/ROX/13-pod.yaml
- examples/cluster/RWO/13-ds.yaml
- examples/namespace/RWO/11-gkmcache.yaml
- examples/namespace/ROX/12-pod.yaml
- examples/namespace/RWO/12-ds.yaml
- examples/cluster/RWO/14-ds.yaml
- examples/namespace/RWO/13-ds.yaml
✅ Files skipped from review due to trivial changes (23)
- examples/base/scope/namespace/kustomization.yaml
- examples/base/scope/cluster/namespace-2.env
- examples/patch/node-selector-kind-false.txt
- examples/base/scope/cluster/kustomization.yaml
- README.md
- examples/patch/toleration-kind.txt
- examples/.gitignore
- examples/patch/affinity-nfd-cuda.txt
- examples/patch/affinity-nfd-rocm.txt
- examples/base/common/namespace-1.env
- examples/base/common/kustomization.yaml
- examples/base/access/rox/pod-2.yaml
- examples/patch/toleration-nfd-cuda.txt
- examples/base/access/rwo/kustomization.yaml
- examples/base/scope/namespace/gkmcache.yaml
- examples/base/access/rox/kustomization.yaml
- examples/base/access/rox/pod-3.yaml
- examples/base/scope/cluster/clustergkmcache.yaml
- examples/base/access/rox/pod-1.yaml
- examples/base/access/rwo/ds-1.yaml
- examples/base/access/rwo/ds-2.yaml
- examples/patch/node-selector-kind-true.txt
- examples/variants/scope/namespace/kustomization.env
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/variants/access/rwo/kustomization.env
- docs/GettingStartedGuide.md
- examples/variants/access/rox/kustomization.env
- docs/DeploymentOptions.md
ba0898c to
b6d7aa1
Compare
Rework the examples directory to use kustomize instead of duplicating the examples for each possible combination. Also remove the hardcoded KIND cluster settings. Signed-off-by: Billy McFall <22157057+Billy99@users.noreply.github.com>
Rework the examples directory to use kustomize instead of duplicating the examples for each possible combination. Also remove the hardcoded KIND cluster settings.
Summary by CodeRabbit
New Features
Documentation
Chores