feat: add kind-local overlay and kind-local-up target #597
Conversation
- Add kind-local kustomize overlay for local development without Quay - Add make kind-local-up target for fully local dev workflow - Add vteam_public_api to load-images.sh and minikube _build-and-load - Fix OCI archive path bug in load-images.sh (colon in tag created nested path) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude Code ReviewSummaryThis PR adds a kind-local overlay for local development with locally built images instead of pulling from Quay.io. The implementation is well-structured and follows established patterns, but there are critical issues that must be addressed before merging. Issues by Severity🚫 Blocker Issues1. Inconsistent image references in operator patch Location: The operator patch hardcodes Impact: kind-local-up will fail when operator tries to spawn state-sync pods. Fix Required: Either add state-sync build step to Makefile build-all target OR remove state-sync from the local image list (keep using Quay for this component). 2. Missing vteam_state_sync build in _build-and-load target Location: Makefile:741-766 The minikube _build-and-load target adds public-api but does NOT build/load state-sync, creating inconsistency between kind-local-up (loads state-sync via load-images.sh) and Minikube workflow (doesn't build/load state-sync). Impact: Divergent behavior between kind-local and minikube paths. Fix Required: Add state-sync to minikube build/load or remove from kind-local. 🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines. |
Critical Issues (continued)3. Shell variable escaping in filename generation Location: e2e/scripts/load-images.sh:72 The fix changes the pattern which is correct, but the commit message says it fixes a bug. However, the old code already worked. The new approach is cleaner but not a bug fix. Recommendation: Update commit message to say "Improve filename generation" rather than "Fix bug". 4. Hardcoded registry in backend patch Location: components/manifests/overlays/kind-local/backend-env-patch.yaml:14 The backend patch hardcodes CONTENT_SERVICE_IMAGE to docker.io/library/vteam_backend:latest. This creates maintenance burden and is inconsistent with how runner/state-sync images are handled. Recommendation: Use kustomize image transformation instead of hardcoding. Major Issues5. Missing validation that images exist before deployment Location: Makefile:606-648 The kind-local-up target calls build-all and load-images.sh, but no validation that all required images exist before kubectl apply. Issue: If build fails silently, deployment will fail with confusing error messages. Recommendation: Add image existence checks between steps 3 and 4. 6. Inconsistent image tag strategy The PR uses :latest tags throughout. For local dev this is acceptable but can cause confusion if images are not rebuilt after code changes. Current behavior: If user runs kind-local-up, modifies code, then re-runs kind-local-up, old images persist with :latest tag. Recommendation: Consider adding a cleanup step or warn users to run make build-all explicitly when code changes. |
Minor Issues7. Documentation gap The PR adds a new overlay and Makefile target but does not update:
Recommendation: Add a follow-up PR or update docs in this PR. 8. Inconsistent color output in Makefile The kind-local-up target uses color codes (COLOR_BOLD, COLOR_BLUE, etc.) but these variables are defined elsewhere. If not defined, output will show literal variable names. Check: Verify color variables are defined at top of Makefile. 9. Error handling in Makefile target Location: Makefile:614-620 The API server wait loop has good timeout logic, but kubectl cluster-info output is suppressed. If timeout occurs, no diagnostic information is provided. Recommendation: Capture and log error message on timeout. 10. Missing cleanup for kind-local-up The PR adds kind-local-up but reuses kind-down for cleanup. Users may expect kind-local-down or kind-local-clean to exist. Recommendation: Either alias kind-local-down to kind-down or update help text to clarify. |
Positive Highlights✅ Excellent separation of concerns - The kind-local overlay cleanly extends the kind overlay without duplicating code ✅ Proper kustomize layering - Uses resources: - ../kind to inherit base configuration ✅ Comprehensive image coverage - Includes all 5 images (backend, frontend, operator, runner, public-api) plus state-sync ✅ Consistent image naming - Uses docker.io/library/ prefix to match how ctr imports images into kind ✅ Good user experience - Clear step-by-step output with progress indicators ✅ Follows Makefile patterns - Uses --no-print-directory, proper phony targets, consistent formatting ✅ Shell script improvements - The ARCHIVE variable cleanup is cleaner than inline substitution ✅ Testing integration - Target includes test-e2e instructions at the end RecommendationsBefore Merge (Priority Order)
Follow-up (Can be separate PR)
|
Architecture Adherence✅ Follows CLAUDE.md patterns:
✅ No security concerns:
✅ Build system consistency:
Test Plan ValidationThe PR includes a test plan, but it is incomplete:
Recommendation: Add explicit test steps for:
Final RecommendationDO NOT MERGE until blocker issues 1 and 2 are resolved. The state-sync inconsistency will cause immediate failures in the main use case (make kind-local-up). Once state-sync strategy is clarified and implemented, the PR is otherwise well-structured and ready to merge with minor improvements. 🤖 Generated with Claude Code (Sonnet 4.5) |
|
needs to be refactored |
1 participant
Summary
kind-localkustomize overlay for local development without pulling images from Quaymake kind-local-uptarget for fully local dev workflow (build → load → deploy)vteam_public_apitoload-images.shand minikube_build-and-loadload-images.sh(:in image tag created nested directory path)Test plan
make kind-local-up— creates kind cluster, builds all images locally, loads them, deploys, all pods runningkubectl kustomize components/manifests/overlays/kind-local/— renders correctly with local image names andIfNotPresentmake test-e2e— e2e tests pass against locally built images🤖 Generated with Claude Code