Merged
Conversation
Kind is the recommended local development tool and matches CI. CRC remains available for OpenShift-specific testing. The CI workflow now uses kind instead of minikube. The Makefile "local-up", "local-down", "local-clean", and "local-rebuild" targets are now aliases for their kind equivalents, so existing muscle memory still works. Fixes #898 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Kind requires cluster names to match "^[a-z0-9.-]+$". In CI, "git rev-parse --abbrev-ref HEAD" returns "HEAD" (detached), producing "ambient-HEAD" which kind rejects. Pipe through "tr A-Z a-z" before sanitizing so the slug is always lowercase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR removes Minikube support and consolidates local development and CI workflows around Kind: deletes Minikube manifests and docs, updates Makefile targets, CI workflow, tests, and documentation to use Kind-only commands and references. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
📝 Coding Plan
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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/local-dev-test.sh (1)
421-436:⚠️ Potential issue | 🔴 CriticalTest references removed Makefile targets - will fail.
This test invokes
local-reload-backend,local-reload-frontend, andlocal-reload-operator, but these targets were removed from the Makefile in this PR. The test will fail with "no rule to make target" errors.Proposed fix: Update to test existing targets
test_reload_commands() { - log_section "Test 14: Reload Commands (Dry Run)" + log_section "Test 14: Rebuild Commands (Dry Run)" - local reload_cmds=("local-reload-backend" "local-reload-frontend" "local-reload-operator") + local rebuild_cmds=("kind-rebuild" "local-rebuild") - for cmd in "${reload_cmds[@]}"; do + for cmd in "${rebuild_cmds[@]}"; do if make -n "$cmd" >/dev/null 2>&1; then log_success "make $cmd syntax is valid" ((PASSED_TESTS++)) else log_error "make $cmd has syntax errors" ((FAILED_TESTS++)) fi done }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/local-dev-test.sh` around lines 421 - 436, The test function test_reload_commands currently checks for removed Makefile targets via the reload_cmds array ("local-reload-backend", "local-reload-frontend", "local-reload-operator"); update this test to reference the actual reload targets that remain in the Makefile (or remove the test) so make -n "$cmd" no longer fails with "no rule to make target". Locate test_reload_commands and modify the reload_cmds array to contain the current target names (or add a guard to skip the loop when those targets are absent) and keep the same success/error logging behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test-local-dev.yml:
- Line 9: The KIND_VERSION variable in the workflow is pinned to "v0.27.0" but
must be updated to "v0.31.0" to match the node image used in
e2e/scripts/setup-kind.sh; change the KIND_VERSION value to "v0.31.0" in the
workflow definition so kind and the kindest/node image are compatible.
In `@docs/internal/architecture/diagrams/deployment-stack.mmd`:
- Line 10: Rename the Mermaid node identifier MK to a clearer identifier KIND
everywhere it’s used (the node declaration MK[Kind] and all references to MK in
the diagram) so the identifier matches the represented runtime; update the label
if needed to remain "Kind" (e.g., change MK[Kind] -> KIND[Kind]) and replace all
other occurrences of MK with KIND to keep references consistent throughout the
diagram (including the two other places referencing MK).
In `@Makefile`:
- Line 2: The test test_reload_commands in tests/local-dev-test.sh still invokes
removed Makefile targets local-reload-backend, local-reload-frontend, and
local-reload-operator causing "no rule to make target" failures; update that
test to either remove the test_reload_commands block entirely or change its
invoked targets to existing Makefile targets (for example replace the removed
targets with kind-rebuild or other valid targets), ensuring the test uses make
-n against current target names and keep the test_reload_commands identifier and
assertions adjusted accordingly.
---
Outside diff comments:
In `@tests/local-dev-test.sh`:
- Around line 421-436: The test function test_reload_commands currently checks
for removed Makefile targets via the reload_cmds array ("local-reload-backend",
"local-reload-frontend", "local-reload-operator"); update this test to reference
the actual reload targets that remain in the Makefile (or remove the test) so
make -n "$cmd" no longer fails with "no rule to make target". Locate
test_reload_commands and modify the reload_cmds array to contain the current
target names (or add a guard to skip the loop when those targets are absent) and
keep the same success/error logging behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8024164b-9d99-4a43-bf5d-dfc4b2decedf
📒 Files selected for processing (27)
.claude/skills/dev-cluster/README.md.claude/skills/dev-cluster/SKILL.md.github/workflows/test-local-dev.ymlBOOKMARKS.mdCONTRIBUTING.mdMakefilecomponents/README.mdcomponents/backend/README.mdcomponents/frontend/README.mdcomponents/manifests/minikube/backend-deployment.yamlcomponents/manifests/minikube/backend-service.yamlcomponents/manifests/minikube/frontend-deployment.yamlcomponents/manifests/minikube/frontend-service.yamlcomponents/manifests/minikube/ingress.yamlcomponents/manifests/minikube/local-dev-rbac.yamlcomponents/manifests/minikube/operator-config.yamlcomponents/manifests/minikube/operator-deployment.yamldocs/internal/README.mddocs/internal/architecture/diagrams/component-structure.mmddocs/internal/architecture/diagrams/deployment-stack.mmddocs/internal/developer/README.mddocs/internal/developer/local-development/README.mddocs/internal/developer/local-development/crc.mddocs/internal/developer/local-development/minikube.mddocs/src/content/docs/development/index.mdtests/README.mdtests/local-dev-test.sh
💤 Files with no reviewable changes (10)
- docs/internal/developer/local-development/minikube.md
- components/manifests/minikube/operator-config.yaml
- components/manifests/minikube/ingress.yaml
- BOOKMARKS.md
- components/manifests/minikube/frontend-service.yaml
- components/manifests/minikube/operator-deployment.yaml
- components/manifests/minikube/backend-deployment.yaml
- components/manifests/minikube/frontend-deployment.yaml
- components/manifests/minikube/backend-service.yaml
- components/manifests/minikube/local-dev-rbac.yaml
The "kubectl wait --for=condition=ready" step already validates the backend pod's readiness probe (which hits /health). The explicit "wget" check fails in ubi-minimal containers that lack wget, and is redundant with the readiness gate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The comprehensive test script assumed minikube-specific behavior (DISABLE_AUTH, local-dev-user, ingress-nginx, local-reload-* targets). Update all tests to match the kind overlay setup: - Check pod readiness instead of curling via minikube IP - Accept missing ingress-nginx (kind uses NodePort mapping) - Check for NAMESPACE/PORT/BACKEND_URL env vars instead of DISABLE_AUTH - Validate test-user SA instead of local-dev-user - Test kind-rebuild instead of removed local-reload-* targets - Pass CONTAINER_ENGINE to make local-status in CI Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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)
79-85:⚠️ Potential issue | 🟠 MajorPreserve per-worktree uniqueness in
CLUSTER_SLUG.Lines 79-85 truncate the sanitized branch name before it is used for both
KIND_CLUSTER_NAMEandKIND_PORT_OFFSET. Two branches with the same first 20 sanitized characters will pick the same cluster name and ports, so one worktree can end up reusing or tearing down the other worktree’s Kind cluster.Suggested fix
-CLUSTER_SLUG ?= $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null | tr 'A-Z' 'a-z' | sed 's/[^a-z0-9]/-/g' | sed 's/--*/-/g' | sed 's/^-//' | sed 's/-$$//' | cut -c1-20) +CLUSTER_SLUG ?= $(shell branch=$$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo detached); \ + slug=$$(printf '%s' "$$branch" | tr 'A-Z' 'a-z' | sed 's/[^a-z0-9]/-/g' | sed 's/--*/-/g' | sed 's/^-//' | sed 's/-$$//'); \ + hash=$$(printf '%s' "$$branch" | cksum | awk '{print $$1}' | cut -c1-6); \ + printf '%.13s-%s' "$$slug" "$$hash")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 79 - 85, CLUSTER_SLUG currently only uses the first 20 chars of the sanitized branch name so different worktrees can collide; change the CLUSTER_SLUG generation to append a per-worktree discriminator (preferably the worktree name from git rev-parse --git-dir, falling back to git rev-parse --short HEAD) before final sanitization/truncation so KIND_CLUSTER_NAME and KIND_PORT_OFFSET (which both derive from CLUSTER_SLUG) become unique per worktree; preserve the existing sanitization and then cut to 20 chars after adding the discriminator so downstream symbols CLUSTER_SLUG, KIND_CLUSTER_NAME, and KIND_PORT_OFFSET continue to work without collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 403-409: The target local-test-quick (and similarly kind-rebuild,
kind-port-forward, dev-bootstrap) only checks that a kind cluster named by
$(KIND_CLUSTER_NAME) exists but does not ensure kubectl is using that cluster's
context; update these targets to explicitly switch or pin kubectl to the
expected Kind context (e.g., run kubectl config use-context or pass
--context=kind-$(KIND_CLUSTER_NAME) / the exact context name) before running any
kubectl commands and fail if the expected context is not available, so all
subsequent kubectl invocations operate on the correct cluster.
---
Outside diff comments:
In `@Makefile`:
- Around line 79-85: CLUSTER_SLUG currently only uses the first 20 chars of the
sanitized branch name so different worktrees can collide; change the
CLUSTER_SLUG generation to append a per-worktree discriminator (preferably the
worktree name from git rev-parse --git-dir, falling back to git rev-parse
--short HEAD) before final sanitization/truncation so KIND_CLUSTER_NAME and
KIND_PORT_OFFSET (which both derive from CLUSTER_SLUG) become unique per
worktree; preserve the existing sanitization and then cut to 20 chars after
adding the discriminator so downstream symbols CLUSTER_SLUG, KIND_CLUSTER_NAME,
and KIND_PORT_OFFSET continue to work without collisions.
Bump KIND_VERSION from v0.27.0 to v0.31.0 so the CI workflow matches the "kindest/node:v1.35.0" image used in setup-kind.sh. Node images are only officially supported with the kind version they shipped with. Rename the "MK" Mermaid node identifier to "KIND" in the deployment-stack diagram, since the node now represents Kind rather than Minikube. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 12, 2026
Gkrumbach07
approved these changes
Mar 12, 2026
Merge Queue Status
This pull request spent 15 seconds in the queue, including 2 seconds running CI. Required conditions to merge
|
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
2 participants
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
components/manifests/minikube/directory anddocs/internal/developer/local-development/minikube.md.github/workflows/test-local-dev.yml) from minikube to kindlocal-reload-*,_build-and-load,_show-access-info,check-minikube,local-sync-version); aliaslocal-up/local-down/local-clean/local-rebuildto their kind equivalentslocal-test-quickto use kind cluster checks instead ofminikube ipcheck-local-contextto only acceptkind-prefixed contextsTest plan
make kind-upcreates cluster successfullymake local-upworks as alias forkind-upmake local-test-quickpasses against a running kind clustermake local-statusshows correct kind cluster infomake local-downworks as alias forkind-downmake helpoutput looks correcttests/local-dev-test.sh --cipassesFixes #898