Confine kind setup and e2e tests to the kind context#1807
Open
danielorbach wants to merge 3 commits intokagent-dev:mainfrom
Open
Confine kind setup and e2e tests to the kind context#1807danielorbach wants to merge 3 commits intokagent-dev:mainfrom
danielorbach wants to merge 3 commits intokagent-dev:mainfrom
Conversation
setup-kind.sh and setup-metallb.sh used `kubectl apply` (and rollout/wait)
without `--context`, so they targeted whatever the active kubeconfig
context happened to be. Running `make create-kind-cluster` while pointed
at a remote cluster would silently push the local-registry-hosting
ConfigMap and the entire MetalLB stack onto that cluster.
Pin both scripts to `kind-${KIND_CLUSTER_NAME}` so they only ever touch
the kind cluster they are setting up.
Several Make targets ran kubectl/istioctl against the active kubeconfig context with no `--context` flag: - `use-kind-cluster` did `kubectl create namespace kagent` and `kubectl config set-context --current --namespace kagent`. If invoked while another cluster was active, both lines would mutate that cluster's state (and the user's kubeconfig) instead of kind. - `kagent-cli-port-forward` and `kagent-ui-port-forward` would forward ports from whichever cluster happened to be current. - `kagent-addon-install` pinned its `kubectl apply` lines but left `istioctl install` unscoped, so Istio could land on the wrong cluster. Pin every kubectl/istioctl invocation to `kind-$(KIND_CLUSTER_NAME)`. `use-kind-cluster` now sets the namespace on the kind context by name rather than `--current`, leaving any other context untouched.
The e2e test code calls controller-runtime's `config.GetConfig()` and
also `exec.Command("kubectl", ...)`, both of which honor the active
kubeconfig context. Running `make -C go e2e` while pointed at a remote
cluster would create Agents, MCPServers and ModelConfigs in that
cluster's `kagent` namespace.
Materialize the kind cluster's kubeconfig to a temp file and run the
test binary with KUBECONFIG pinned to it, so the tests cannot reach any
cluster other than the one they were designed for. Abort early with a
clear message if the kind cluster is missing.
danielorbach
requested review from
EItanya,
ilackarms,
peterj and
yuval-k
as code owners
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
1 participant
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.
A stray
kubectl config current-contextwas enough to makemake create-kind-cluster,make use-kind-cluster,make kagent-addon-install, andmake -C go e2emutate whichever cluster was active. None of those targets read like "may rewrite an arbitrary cluster," yet that is what they did. Resolves #1806.scripts/kind/setup-{kind,metallb}.shnow pin everykubectlinvocation to--context "kind-${KIND_CLUSTER_NAME}". The Makefile targetsuse-kind-cluster,kagent-cli-port-forward,kagent-ui-port-forward, and theistioctl installline insidekagent-addon-installget the same treatment;use-kind-clusterno longer touches--currentand instead sets the namespace on the kind context by name, so any other context the user has is left untouched.go/Makefile'se2etarget materializes the kind cluster's kubeconfig to a temp file and runsgo testwithKUBECONFIGforced to it — bothcontroller-runtime'sconfig.GetConfig()and the bareexec.Command("kubectl", ...)calls inside the test code now have no path to a remote cluster. The target aborts early with a clear message if the kind cluster is missing.The same
--kube-context kind-$(KIND_CLUSTER_NAME)discipline already applied tohelm-install*,helm-uninstall,push-test-agent, and thekubectl applylines insidekagent-addon-install; this brings the rest of the developer tooling to the same baseline.Verification
bash -nparses both kind scripts.make -n use-kind-cluster,make -n kagent-addon-install,make -n kagent-cli-port-forward,make -n kagent-ui-port-forward, andmake -C go -n e2eall expand with--context kind-kagent(orKUBECONFIG=/tmp/kind-config-e2e) on every state-mutating line.Out of scope
The Go test code itself still calls
kubectlandcontroller-runtimewithout an explicit context; the safety here is provided by the wrappingmake e2etarget settingKUBECONFIG. Anyone runninggo test ./go/core/test/e2edirectly is still responsible for settingKUBECONFIGthemselves.