Merged
Conversation
Contributor
WalkthroughThe Makefile is refactored to standardize on Kind as the exclusive local Kubernetes development platform. Minikube-specific targets, helpers, and checks are removed entirely, new Kind reload targets are introduced, and deprecated aliases are added for backward compatibility while directing users to Kind-based workflows. Changes
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)
Comment |
Contributor
Review Queue Status
Action needed: Fix failing CI checks
|
ktdreyer
force-pushed
the
remove-minikube-from-makefile
branch
from
80fcdb7 to
99dc330
Compare
Contributor
Author
|
This is a breaking change. The I've edited the test. |
![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: 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)
772-783:⚠️ Potential issue | 🟠 MajorDon’t relax this guard to any
kind-*context.This file already derives a worktree-specific
KIND_CLUSTER_NAME, but this check now accepts any kind context. That meanslocal-downcan delete the namespace in the wrong cluster, andkind-rebuildcan load images into$(KIND_CLUSTER_NAME)whilekubectl applyandkubectl rollouthit whichever other kind context is active.Suggested fix
`@ctx`=$$(kubectl config current-context 2>/dev/null || echo ""); \ - if echo "$$ctx" | grep -qE '^kind-'; then \ + expected="kind-$(KIND_CLUSTER_NAME)"; \ + if [ "$$ctx" = "$$expected" ]; then \ : ; \ else \ - echo "$(COLOR_RED)✗$(COLOR_RESET) Current kubectl context '$$ctx' does not look like a local cluster."; \ - echo " Expected a context starting with 'kind-'."; \ - echo " Switch context first, e.g.: kubectl config use-context kind-ambient-local"; \ + echo "$(COLOR_RED)✗$(COLOR_RESET) Current kubectl context '$$ctx' does not match this worktree's cluster."; \ + echo " Expected: $$expected"; \ + echo " Switch context first: kubectl config use-context $$expected"; \ echo ""; \ echo " To bypass this check: make <target> SKIP_CONTEXT_CHECK=true"; \ exit 1; \ fiIf
kind-loginis meant to switch from arbitrary contexts, that target should stop depending on this helper and pass--context kind-$(KIND_CLUSTER_NAME)explicitly instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 772 - 783, The current check-local-context target allows any context matching '^kind-' which is too permissive; change the check to verify the current context equals the exact expected context for this worktree (derive EXPECTED_CTX as "kind-$(KIND_CLUSTER_NAME)" and compare current-context to that string rather than a prefix match) so commands only run against the intended cluster; if kind-login is supposed to switch contexts, update that target to stop depending on check-local-context and instead invoke kubectl with --context=kind-$(KIND_CLUSTER_NAME) explicitly.
🤖 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 419-426: The local-test-quick target uses a bare `kind get
clusters` check which can miss Podman-backed clusters and may detect clusters
from other worktrees; update the check in the local-test-quick recipe (target
name: local-test-quick) to mirror the provider-aware logic used in the
local-status target: honor the KIND_EXPERIMENTAL_PROVIDER environment variable
and query for the specific cluster name $(KIND_CLUSTER_NAME) when probing (i.e.,
use the same `KIND_EXPERIMENTAL_PROVIDER=... kind get clusters | grep -q
$(KIND_CLUSTER_NAME)` style approach as in local-status) so the probe reliably
detects the intended cluster regardless of provider.
---
Outside diff comments:
In `@Makefile`:
- Around line 772-783: The current check-local-context target allows any context
matching '^kind-' which is too permissive; change the check to verify the
current context equals the exact expected context for this worktree (derive
EXPECTED_CTX as "kind-$(KIND_CLUSTER_NAME)" and compare current-context to that
string rather than a prefix match) so commands only run against the intended
cluster; if kind-login is supposed to switch contexts, update that target to
stop depending on check-local-context and instead invoke kubectl with
--context=kind-$(KIND_CLUSTER_NAME) explicitly.
🪄 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: ad6d32bc-2c3a-47d6-a75e-05f281f5f343
📒 Files selected for processing (2)
Makefiletests/local-dev-test.sh
Comment on lines
+419
to
+426
| local-test-quick: check-kubectl ## Quick smoke test of local environment | ||
| @echo "$(COLOR_BOLD)🧪 Quick Smoke Test$(COLOR_RESET)" | ||
| @echo "" | ||
| @echo "$(COLOR_BLUE)▶$(COLOR_RESET) Detecting cluster type..." | ||
| @if kind get clusters 2>/dev/null | grep -q .; then \ | ||
| echo "$(COLOR_GREEN)✓$(COLOR_RESET) Kind cluster running"; \ | ||
| CLUSTER_TYPE=kind; \ | ||
| elif command -v minikube >/dev/null 2>&1 && minikube status >/dev/null 2>&1; then \ | ||
| echo "$(COLOR_GREEN)✓$(COLOR_RESET) Minikube running"; \ | ||
| CLUSTER_TYPE=minikube; \ | ||
| else \ | ||
| echo "$(COLOR_RED)✗$(COLOR_RESET) No local cluster found (kind or minikube)"; exit 1; \ | ||
| echo "$(COLOR_RED)✗$(COLOR_RESET) No kind cluster found. Run 'make kind-up' first."; exit 1; \ |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the Makefile to understand the context
cd "$(git rev-parse --show-toplevel)" 2>/dev/null || cd .
# Check file existence
if [ -f Makefile ]; then
echo "=== Checking Makefile structure ==="
wc -l Makefile
else
echo "Makefile not found, searching..."
find . -name "Makefile" -type f | head -5
fiRepository: ambient-code/platform
Length of output: 113
🏁 Script executed:
# Let's look at the specific sections mentioned in the review
# First, find the lines around 419-426 (local-test-quick)
sed -n '415,430p' MakefileRepository: ambient-code/platform
Length of output: 1006
🏁 Script executed:
# Now look at the local-status target (mentioned as having the correct pattern)
# Search for local-status definition
grep -n "^local-status:" MakefileRepository: ambient-code/platform
Length of output: 131
🏁 Script executed:
# Search for KIND_CLUSTER_NAME definition
grep -n "KIND_CLUSTER_NAME" Makefile | head -20Repository: ambient-code/platform
Length of output: 2073
🏁 Script executed:
# Search for how local-status implements the kind get clusters check
grep -n "local-status" Makefile -A 10 | grep -E "kind get clusters|KIND_EXPERIMENTAL_PROVIDER"Repository: ambient-code/platform
Length of output: 224
Reuse the provider-aware kind get clusters check from local-status.
Kind's Quick Start documents KIND_EXPERIMENTAL_PROVIDER as the runtime selector. The current bare kind get clusters can fail to detect a Podman-backed cluster, and it checks for any cluster rather than $(KIND_CLUSTER_NAME), allowing a different worktree's cluster to satisfy the probe. The local-status target (line 317) already implements the correct pattern; apply the same approach here.
Suggested fix
- `@if` kind get clusters 2>/dev/null | grep -q .; then \
+ `@if` $(if $(filter podman,$(CONTAINER_ENGINE)),KIND_EXPERIMENTAL_PROVIDER=podman) kind get clusters 2>/dev/null | grep -q '^$(KIND_CLUSTER_NAME)$$'; then \
echo "$(COLOR_GREEN)✓$(COLOR_RESET) Kind cluster running"; \
else \
echo "$(COLOR_RED)✗$(COLOR_RESET) No kind cluster found. Run 'make kind-up' first."; exit 1; \
fi📝 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.
Suggested change
| local-test-quick: check-kubectl ## Quick smoke test of local environment | |
| @echo "$(COLOR_BOLD)🧪 Quick Smoke Test$(COLOR_RESET)" | |
| @echo "" | |
| @echo "$(COLOR_BLUE)▶$(COLOR_RESET) Detecting cluster type..." | |
| @if kind get clusters 2>/dev/null | grep -q .; then \ | |
| echo "$(COLOR_GREEN)✓$(COLOR_RESET) Kind cluster running"; \ | |
| CLUSTER_TYPE=kind; \ | |
| elif command -v minikube >/dev/null 2>&1 && minikube status >/dev/null 2>&1; then \ | |
| echo "$(COLOR_GREEN)✓$(COLOR_RESET) Minikube running"; \ | |
| CLUSTER_TYPE=minikube; \ | |
| else \ | |
| echo "$(COLOR_RED)✗$(COLOR_RESET) No local cluster found (kind or minikube)"; exit 1; \ | |
| echo "$(COLOR_RED)✗$(COLOR_RESET) No kind cluster found. Run 'make kind-up' first."; exit 1; \ | |
| local-test-quick: check-kubectl ## Quick smoke test of local environment | |
| `@echo` "$(COLOR_BOLD)🧪 Quick Smoke Test$(COLOR_RESET)" | |
| `@echo` "" | |
| `@echo` "$(COLOR_BLUE)▶$(COLOR_RESET) Detecting cluster type..." | |
| `@if` $(if $(filter podman,$(CONTAINER_ENGINE)),KIND_EXPERIMENTAL_PROVIDER=podman) kind get clusters 2>/dev/null | grep -q '^$(KIND_CLUSTER_NAME)$$'; then \ | |
| echo "$(COLOR_GREEN)✓$(COLOR_RESET) Kind cluster running"; \ | |
| else \ | |
| echo "$(COLOR_RED)✗$(COLOR_RESET) No kind cluster found. Run 'make kind-up' first."; exit 1; \ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 419 - 426, The local-test-quick target uses a bare
`kind get clusters` check which can miss Podman-backed clusters and may detect
clusters from other worktrees; update the check in the local-test-quick recipe
(target name: local-test-quick) to mirror the provider-aware logic used in the
local-status target: honor the KIND_EXPERIMENTAL_PROVIDER environment variable
and query for the specific cluster name $(KIND_CLUSTER_NAME) when probing (i.e.,
use the same `KIND_EXPERIMENTAL_PROVIDER=... kind get clusters | grep -q
$(KIND_CLUSTER_NAME)` style approach as in local-status) so the probe reliably
detects the intended cluster regardless of provider.
ktdreyer
commented
Mar 26, 2026
ktdreyer
force-pushed
the
remove-minikube-from-makefile
branch
2 times, most recently
from
a6c72ca to
a67ece7
Compare
Issue #898 dropped minikube support, but the Makefile still had minikube targets and references. These stale targets cause confusion when working on Makefile changes because they appear to be active code that needs maintenance. Remove minikube-only targets and update remaining targets (local-down, local-status, local-test-quick, makefile-health, check-local-context, _show-access-info) to reference kind instead. Add per-component kind reload targets (kind-reload-backend, kind-reload-frontend, kind-reload-operator) so developers can iterate on a single service without rebuilding the entire stack. Preserve backward compatibility with deprecation aliases that print a warning and delegate to the kind equivalent. A follow-up issue (#1080) tracks updating docs that still reference the old names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ktdreyer
force-pushed
the
remove-minikube-from-makefile
branch
from
a67ece7 to
0ced67a
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Makefile (1)
419-427:⚠️ Potential issue | 🟠 MajorInconsistent cluster detection pattern in
local-test-quick.The
kind get clusterscheck here doesn't use the provider-aware pattern (KIND_EXPERIMENTAL_PROVIDER=podman) and checks for any cluster rather than the specific$(KIND_CLUSTER_NAME). This is inconsistent with the correct implementation inlocal-status(line 317).Suggested fix to align with local-status
- `@if` kind get clusters 2>/dev/null | grep -q .; then \ + `@if` $(if $(filter podman,$(CONTAINER_ENGINE)),KIND_EXPERIMENTAL_PROVIDER=podman) kind get clusters 2>/dev/null | grep -q '^$(KIND_CLUSTER_NAME)$$'; then \ echo "$(COLOR_GREEN)✓$(COLOR_RESET) Kind cluster running"; \ else \ echo "$(COLOR_RED)✗$(COLOR_RESET) No kind cluster found. Run 'make kind-up' first."; exit 1; \ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 419 - 427, The local-test-quick Make target uses a generic `kind get clusters` check and doesn't respect the provider or the configured cluster name; update the `local-test-quick` target to mirror the `local-status` pattern by invoking `KIND_EXPERIMENTAL_PROVIDER=$(KIND_EXPERIMENTAL_PROVIDER) kind get clusters` and grepping specifically for `$(KIND_CLUSTER_NAME)` instead of any non-empty output, so detection is provider-aware and matches the intended cluster; ensure the success/failure messages remain and the failure path exits with non-zero status as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Makefile`:
- Around line 419-427: The local-test-quick Make target uses a generic `kind get
clusters` check and doesn't respect the provider or the configured cluster name;
update the `local-test-quick` target to mirror the `local-status` pattern by
invoking `KIND_EXPERIMENTAL_PROVIDER=$(KIND_EXPERIMENTAL_PROVIDER) kind get
clusters` and grepping specifically for `$(KIND_CLUSTER_NAME)` instead of any
non-empty output, so detection is provider-aware and matches the intended
cluster; ensure the success/failure messages remain and the failure path exits
with non-zero status as before.
bobbravo2
approved these changes
Mar 27, 2026
Member
bobbravo2
left a comment
bobbravo2
left a comment
There was a problem hiding this comment.
Tested this PR locally when working a bug. Kind is solid, thanks 🙏
Merge Queue Status
This pull request spent 10 seconds in the queue, including 1 second 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
local-url,check-minikube,_build-and-load) and minikube references from remaining targets (local-down,local-status,local-test-quick,makefile-health,check-local-context,_show-access-info)kind-reload-backend,kind-reload-frontend,kind-reload-operatorlocal-up,local-clean,local-rebuild,local-reload-backend/frontend/operator) that print a warning and delegate to the kind equivalentFixes #992
Test plan
make helpdisplays correctly with updated examplesmake helpstill listslocal-up(with deprecation note)make kind-upstill works as the local dev entrypointmake kind-rebuildstill works for reloading componentsmake kind-reload-backend/kind-reload-frontend/kind-reload-operatordry-run succeedsmake local-upprints deprecation warning and delegates tokind-upmake local-statuscorrectly detects kind clustersmake local-test-quickcorrectly detects kind clustersmake makefile-healthruns without errors