Skip to content

ci(e2e): wire multi-controller workflow and drop RBAC patch-and-restore (4/4) - #1209

Closed
lionelvillard wants to merge 5 commits into
llm-d:mainfrom
lionelvillard:issue-1205-pr4-multi-controller-impl
Closed

ci(e2e): wire multi-controller workflow and drop RBAC patch-and-restore (4/4)#1209
lionelvillard wants to merge 5 commits into
llm-d:mainfrom
lionelvillard:issue-1205-pr4-multi-controller-impl

Conversation

@lionelvillard

Copy link
Copy Markdown
Collaborator

Summary

  • Replaces the stub .github/workflows/ci-e2e-multi-controller.yaml (added in PR 3) with a full pull_request job mirroring e2e-tests-smoke. The job installs the primary controller from the new namespace-scoped/kubernetes overlay so both controllers run namespace-scoped from a fresh kind cluster.
  • Adds a WVA_OVERLAY env override to deploy/lib/infra_wva.sh and deploy/lib/cleanup.sh so install/uninstall can target a non-default overlay without changing the default openshift/kubernetes selection.
  • Removes the kubectl patch ... clusterrolebinding and per-deployment secondary CRB bookkeeping from test/e2e/multi_controller_test.go — the TODO it referenced is resolved by the dedicated job.

This is PR 4 of 4 for #1205. Stacked on PR 2 (#1207) and PR 3 (#1208) — the diff against main will include their commits until they merge. Please review/merge #1206#1207#1208 first.

Open question for reviewers

Even with both controllers namespace-scoped, the existing namespace-scoped/kubernetes and namespace-scoped/openshift overlays still emit the same five ClusterRoleBinding names from config/base/. If the secondary install's kubectl apply overwrites them, the primary controller will lose RBAC. The fresh-cluster CI job runs the primary install first and the secondary second, so the last apply (secondary) wins for cluster-scoped resources. If a follow-up shows that cluster-scoped binding collisions still cause flakes, the right fix is to scope those bindings out of the namespace-scoped overlay (move them under the namespace-scoped component as RoleBindings, or skip them entirely when the overlay is namespace-scoped). I left that out of this PR per the issue's explicit scope ("eliminates the RBAC patch-and-restore workarounds entirely — no shared ClusterRoleBinding names means no conflict") and welcome guidance.

Test plan

  • go build ./... succeeds
  • bin/kustomize build config/overlays/namespace-scoped/kubernetes succeeds
  • CI e2e-multi-controller job goes green on this PR
  • Existing e2e-tests-smoke still passes (no regression from WVA_OVERLAY override defaulting unchanged)

Refs #1205

Adds a vanilla Kubernetes namespace-scoped overlay that mirrors the existing
namespace-scoped/openshift overlay (without the OCP-specific Thanos and
user-workload-monitoring patches). Wires the new overlay into the
kustomize-build CI check.

Prep work for the dedicated dual-controller e2e job; subsequent PRs repoint
the secondary-controller testdata to this overlay so both controllers can run
namespace-scoped from the start, eliminating the ClusterRoleBinding name
collisions between the primary cluster-scoped and secondary namespace-scoped
deployments.

Refs llm-d#1205

Signed-off-by: Lionel Villard <villard@us.ibm.com>
- Move the "Dual namespace-scoped controllers isolation" Context out of
  smoke_test.go into a new multi_controller_test.go labeled
  Label("multi-controller") so it can be selected by ginkgo label-filter.
- Repoint test/e2e/testdata/secondary-controller/kustomization.yaml to the
  new namespace-scoped/kubernetes overlay added in the prior PR; the
  --watch-namespace patch is dropped because the namespace-scoped component
  already injects it.
- Add make targets test-e2e-multi-controller and
  test-e2e-multi-controller-with-setup mirroring the smoke targets.

The patch-and-restore RBAC workaround stays in place for now; a later PR
removes it once the dedicated CI job runs the suite in a fresh kind
cluster.

Refs llm-d#1205

Signed-off-by: Lionel Villard <villard@us.ibm.com>
Adds .github/workflows/ci-e2e-multi-controller.yaml with workflow_dispatch
trigger and a placeholder step. The job name 'e2e-multi-controller' is fixed
so branch-protection rules can reference it ahead of the full implementation
landing in a follow-up PR.

Refs llm-d#1205

Signed-off-by: Lionel Villard <villard@us.ibm.com>
- Replace the stub ci-e2e-multi-controller.yaml workflow with the real
  pull_request job. It mirrors e2e-tests-smoke (kind setup, local image
  build, kind-emulator env), but installs the primary controller from the
  new namespace-scoped/kubernetes overlay (via the WVA_OVERLAY env hook
  added below) and runs only the multi-controller label suite.
- Add a WVA_OVERLAY override in deploy/lib/infra_wva.sh and
  deploy/lib/cleanup.sh so install/uninstall can target an alternative
  overlay without changing the default openshift/kubernetes selection.
- Remove the kubectl patch-and-restore workarounds and the secondary
  ClusterRoleBinding bookkeeping from multi_controller_test.go. With both
  controllers running namespace-scoped from a fresh kind cluster, the
  cluster-scoped binding collisions described in the original TODO no
  longer occur.

Refs llm-d#1205

Signed-off-by: Lionel Villard <villard@us.ibm.com>
@github-actions

Copy link
Copy Markdown
Contributor

This PR is marked as stale after 21d of inactivity. After an additional 14d of inactivity (7d to become rotten, then 7d more), it will be closed. To prevent this PR from being closed, add a comment or remove the lifecycle/stale label.

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.

1 participant