Rework testing cluster infra: K3s-over-SSH module + stable EKS node groups#37
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors testing/infra/clusters to make test cluster provisioning more predictable and single-purpose by introducing a new K3s-over-SSH Terraform module and stabilizing EKS GPU node group addressing to prevent unintended rotations.
Changes:
- Added
testing/infra/clusters/k3s: installs/uninstalls K3s on SSH-reachable hosts (servers + optional agents), fetches and merges kubeconfig, and cleans up kubeconfig entries on destroy. - Updated
testing/infra/clusters/eks: changed GPU node group input from positional lists to a name-keyed map for stable node group keys, and removed Helm/operator deployment so the module only provisions the cluster. - Added destroy-time kubeconfig cleanup for EKS and updated provider locks accordingly.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| testing/infra/clusters/k3s/variables.tf | Defines K3s module inputs (hosts, SSH, release, flannel backend) and validation. |
| testing/infra/clusters/k3s/terraform.tf | Pins required providers for the K3s module. |
| testing/infra/clusters/k3s/output.tf | Exposes kubeconfig/context and resolved host outputs for consumers. |
| testing/infra/clusters/k3s/main.tf | Implements K3s install/join/uninstall and kubeconfig fetch/merge/cleanup logic. |
| testing/infra/clusters/k3s/.terraform.lock.hcl | Locks provider versions/hashes for reproducible K3s module init. |
| testing/infra/clusters/k3s/.gitignore | Prevents committing the generated standalone kubeconfig file. |
| testing/infra/clusters/eks/variables.tf | Changes GPU node group input type to a map keyed by group name for stability. |
| testing/infra/clusters/eks/terraform.tf | Removes Helm provider requirement; keeps only cluster provisioning providers. |
| testing/infra/clusters/eks/main.tf | Updates GPU node group map iteration; removes Helm/operator deployment; adds kubeconfig cleanup on destroy. |
| testing/infra/clusters/eks/.terraform.lock.hcl | Removes Helm provider lock entries to match provider requirements. |
Files not reviewed (2)
- testing/infra/clusters/eks/.terraform.lock.hcl: Generated file
- testing/infra/clusters/k3s/.terraform.lock.hcl: Generated file
thxCode
force-pushed
the
spec/testing-clusters-eks-and-k3s
branch
from
6178315 to
2cc4bed
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.
Files not reviewed (2)
- testing/infra/clusters/eks/.terraform.lock.hcl: Generated file
- testing/infra/clusters/k3s/.terraform.lock.hcl: Generated file
- new testing/infra/clusters/k3s module: install/uninstall K3s on SSH-reachable hosts via remote-exec, driven by server/agent host lists (user@host, per-role SSH ports) - first server bootstraps embedded etcd (--cluster-init); extra servers and agents join with a shared random_string token; an empty agent list leaves the servers schedulable - reclaim each target host before installing (uninstall any pre-existing k3s) so a node with a prior or foreign deployment is redeployed cleanly instead of failing on stale etcd/token data - fetch the kubeconfig, namespace its identifiers to a k3s-<host> context with anchored sed, and flatten-merge it into ~/.kube/config (chmod 600) via a unique mktemp; destroy strips that context/cluster/user - release takes a k3s release tag (e.g. v1.34.9+k3s1) passed straight to INSTALL_K3S_VERSION; named release because Terraform reserves version - flannel_backend selects the flannel backend (default vxlan; host-gw for faster same-L2 networking) - reject duplicate server/agent hosts (each host keys a node) at plan time - ssh_private_key defaults to ~/.ssh/id_ed25519 (the provisioner cannot auth RSA against OpenSSH 8.8+) - expose context_name, server_hosts, agent_hosts, and kubeconfig_path outputs Signed-off-by: thxCode <thxcode0824@gmail.com>
- key GPU node groups by name (map(list(string)) -> gpu-<name>) instead of positional list index, so adding or editing one entry no longer rotates the untouched node groups - remove helm_release, the helm provider, var.image, and the gpustack_operator_image local so the module only provisions the cluster - strip the cluster's context/cluster/user from ~/.kube/config on destroy via a self.triggers-only, on_failure=continue local-exec Signed-off-by: thxCode <thxcode0824@gmail.com>
thxCode
force-pushed
the
spec/testing-clusters-eks-and-k3s
branch
from
2cc4bed to
0cb53a2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- testing/infra/clusters/eks/.terraform.lock.hcl: Generated file
- testing/infra/clusters/k3s/.terraform.lock.hcl: Generated file
Comment on lines
+199
to
+201
| raw="$(mktemp)" | ||
| merged="" | ||
| trap 'rm -f "$raw" "$merged"' EXIT |
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
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.
What type of PR is this?
/kind enhancement
/kind bug
/kind cleanup
/area testing
What this PR does / why we need it:
Reworks
testing/infra/clustersinto predictable, single-purpose Terraform and adds a new K3s-over-SSH module.New K3s module (
testing/infra/clusters/k3s)remote-exec, driven byserver/agenthost lists (user@host, per-role SSH ports). The first server bootstraps embedded etcd (--cluster-init); extra servers and agents join with a shared token; an emptyagentlist leaves the servers schedulable.releasetakes a K3s GitHub release tag (e.g.v1.34.9+k3s1) passed straight toINSTALL_K3S_VERSION.flannel_backendselects the flannel backend (defaultvxlan;host-gwfor faster node-to-node networking when all nodes share one L2 segment).k3s-<host>context, and flatten-merges it into~/.kube/config(mode 600);destroystrips that context/cluster/user.EKS module
map(list(string))->gpu-<name>) instead of positional list index, so adding or editing one entry no longer rotates the untouched node groups (the original bug).helm_release, thehelmprovider,var.image, and thegpustack_operator_imagelocal, so the module only provisions the cluster.~/.kube/configondestroyvia aself.triggers-only,on_failure = continuelocal-exec.Which issue(s) this PR fixes:
N/A (direct request, no tracking issue)
Special notes for your reviewer:
testing/infra/clustersonly — no operator/runtime code changes.host-gw(confirmed via direct pod-CIDR routes over the physical NIC with noflannel.1) on a same-LAN pair; EKS non-rotation proven by a three-step apply (adding a second GPU group left the first group's node group and EC2 instance unchanged); the destroy-time~/.kube/configcleanup was exercised on a real cluster.terraform fmt/validatepass on both modules.Does this PR introduce a user-facing change?