Skip to content

chore: consolidate manifest overlays from 10 to 5 - #418

Draft
markturansky wants to merge 1 commit into
mainfrom
chore/consolidate-manifest-overlays
Draft

chore: consolidate manifest overlays from 10 to 5#418
markturansky wants to merge 1 commit into
mainfrom
chore/consolidate-manifest-overlays

Conversation

@markturansky

Copy link
Copy Markdown
Collaborator

Summary

  • Merge kind-local into kind — Makefile handles LOCAL_IMAGES=true via kubectl set image instead of a separate overlay
  • Rename openshift-localcrc (clearer naming for CRC/OpenShift Local)
  • Rename hcmaishcmai (align with project name)
  • Delete 5 unused/redundant overlays: kind-local, hcmais-dev, openshift-dev, local-dev, mpp-openshift

Before: 10 overlays (kind, kind-local, e2e, openshift-local, local-dev, openshift-dev, production, hcmais, hcmais-dev, mpp-openshift)
After: 5 overlays (kind, e2e, crc, production, hcmai)

All kustomize build verified passing for remaining overlays. Updated references across Makefile, CI workflows, docs, and skills.

Test plan

  • kustomize build passes for all 5 remaining overlays (kind, e2e, crc, production, hcmai)
  • CI kustomize-check workflow validates overlays
  • make kind-up works with default (Quay) images
  • make kind-up LOCAL_IMAGES=true works with locally built images
  • make crc-up works on CRC

🤖 Generated with Claude Code

Streamline the Kustomize overlay structure by removing redundant and
unused overlays:

- Merge kind-local into kind (Makefile handles LOCAL_IMAGES via set image)
- Rename openshift-local → crc (clearer naming)
- Rename hcmais → hcmai (align with project name)
- Delete kind-local (merged into kind)
- Delete hcmais-dev (nearly identical to hcmais, unused)
- Delete openshift-dev (only disabled JWT; OpenShift should use OIDC)
- Delete local-dev (legacy vteam- prefix model, unused)
- Delete mpp-openshift (discontinued)

Remaining overlays: kind, e2e, crc, production, hcmai

All kustomize builds verified passing.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://openshift-online.github.io/agent-control-plane/pr-preview/pr-418/

Built to branch gh-pages at 2026-07-22 03:50 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@jsell-rh jsell-rh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Amber Review

Good direction — reducing 10 overlays to 5 is the right call, and the kustomize build verification you've done gives confidence on the manifest side. Two issues need addressing before this is merge-ready.


🔴 Critical: Hardcoded image name list will silently miss new images

In Makefile, the LOCAL_IMAGES=true path now uses a hard-coded for img in ... loop:

for img in acp_api_server acp_ambient_ui acp_control_plane acp_claude_runner acp_runner_openshell acp_mcp acp_credential_github acp_credential_jira acp_credential_k8s acp_credential_google; do \
    kubectl set image deployment -n $(NAMESPACE) --all --containers="*" \
        "quay.io/ambient_code/$$img:latest=localhost/$$img:latest" 2>/dev/null || true; \
done; \

The old kind-local overlay was declarative and exhaustive by construction; this loop has to be kept in sync manually every time a new image is added. The 2>/dev/null || true suppression also means a missed image silently fails. Two options:

  1. Drive the list from a Makefile variable already used elsewhere (e.g. IMAGES), or
  2. Query the cluster: kubectl get deployments -n $(NAMESPACE) -o jsonpath='{.items[*].spec.template.spec.containers[*].image}' and filter for quay.io/ambient_code/.

Either approach avoids the silent-miss problem. I'd suggest option 1 if the variable already exists.


🔴 Critical: CLAUDE.md convention — never silently swallow partial failures

The || true on kubectl set image means a failed patch produces no error. Per CLAUDE.md: "Never silently swallow partial failures: Every error path must propagate or be collected, not discarded." At a minimum, log which images failed:

kubectl set image deployment -n $(NAMESPACE) --all --containers="*" \
    "quay.io/ambient_code/$$img:latest=localhost/$$img:latest" || \
    echo "Warning: failed to patch image $$img (may not exist — continuing)"; \

⚠️ Major: Unchecked test plan items

Three items remain unchecked:

  • CI kustomize-check workflow validates overlays
  • make kind-up works with default (Quay) images
  • make kind-up LOCAL_IMAGES=true works with locally built images

The last two are the most important — they validate the exact Makefile path being changed. Don't mark ready until these are confirmed. The CI check should also catch any remaining kustomize issues.


Minor (no action needed):

  • hcmais-dev/ambient-api-server-env-patch.yaml had a hardcoded CRC Keycloak URL in OIDC_ISSUER_URL that pointed at apps-crc.testing — good that it's deleted.
  • The README.md is updated correctly to match the new 5-overlay structure.
  • Rename from hcmaishcmai is consistent with the one-line kustomization.yaml comment fix.

Please address the two blocker items and confirm the kind-up paths work before removing Draft status.

— Amber

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants