ci: read the EKS cluster name from Terraform state, not from outputs

[bobbyiliev]

Follow-up to #38721, which does not work. unblock_destroy took the cluster name from terraform output -raw eks_cluster_name, but a destroy removes the root outputs before the resources they reference, so by the time a destroy has failed the state has no outputs left while the cluster is still in it. terraform output -raw then exits 0 and writes a 540-character "No outputs found" warning to stdout rather than stderr, so no CalledProcessError fires and the if not cluster guard sees a non-empty string. The banner was passed to aws eks list-nodegroups --cluster-name, which failed, and the swallowed error left the hook silently inert: the leak it targets still happened, with no log line to say so. Thanks @def- for catching it.

_eks_cluster_name now reads the aws_eks_cluster resource out of terraform state pull, since resources survive a failed destroy, and requires the name to match [A-Za-z0-9][A-Za-z0-9_-]* so no future unexpected stdout can reach the AWS CLI. It also logs when no cluster is found, so an inert hook is distinguishable from one that ran with nothing to do.

Test plan

The reported behavior is confirmed against the CI-pinned Terraform 1.13.5: with a state carrying resources and "outputs": {}, terraform output -raw eks_cluster_name exits 0 with 540 bytes on stdout and nothing on stderr. The new parser was exercised against the real leaked state from Nightly 18272 and returns aws-test-dev-eks both as uploaded and with its outputs stripped, returns empty for a fully destroyed state, and rejects the warning banner as a name. End to end it still needs a destroy that fails with a node group attached, so the AWS nightlies remain the real check.

🤖 Generated with Claude Code

[bosconi]

Looks good. TIL about the output going away before the resources.

[def-]

QA LLM Review

1. MEDIUM -- unblock_destroy now really fires, and deletes healthy node groups, killing Karpenter mid-teardown

test/terraform/mzcompose.py:762

This PR is what makes unblock_destroy actually do something, and the hook deletes every node group on the cluster, not just the CREATE_FAILED one its comment describes. It fires on any destroy failure that leaves aws_eks_cluster in state, which includes failures in the Helm/kubectl_manifest stages. In that case the node group it deletes is base_node_group, the only managed one and the one Karpenter runs on, so the next terraform destroy attempt faces a cluster with no Karpenter controller.

Details

In the scenario the hook targets, this is harmless: the destroy got all the way to the cluster delete, so Terraform has already torn down nodepool_*, ec2nodeclass_* and karpenter, and list-nodegroups returns only the leftover failed node group. "Delete all" and "delete the failed one" coincide, and restricting to unhealthy node groups costs nothing there.

The divergence is when the destroy fails earlier. test/terraform/aws-temporary/main.tf:80 puts Karpenter on local.base_node_labels, i.e. on module.base_node_group; cert-manager, the AWS LBC and the operator run on Karpenter-provisioned nodes (local.generic_node_labels). Deleting the base node group therefore removes the Karpenter controller while its EC2NodeClass/NodePool objects and its provisioned instances are still live. On attempt 2, kubectl_manifest deletes of the EC2NodeClass block on the karpenter.k8s.aws/termination finalizer that nothing is left to clear, and the Karpenter-provisioned instances are never terminated, so the subsequent subnet/security-group destroys hit DependencyViolation. That is the same cluster/VPC leak the hook exists to prevent, now reachable from a transient early failure that previously would have been fixed by a plain retry.

Two supporting details: run_ignore_error swallows a failed delete-nodegroup, and the wait loop at line 786 polls until all node groups disappear, so any node group left behind burns the full 300s of the cleanup budget.

Suggested fix, gate on status so only the stuck node group is touched and only it is waited on:

def _node_group_status(self, cluster: str, node_group: str) -> str:
    """Node group status, empty if it cannot be read."""
    try:
        return json.loads(
            spawn.capture([
                "aws", "eks", "describe-nodegroup",
                "--cluster-name", cluster,
                "--nodegroup-name", node_group,
                "--region", "us-east-1",
                "--output", "json",
            ])
        )["nodegroup"]["status"]
    except (subprocess.CalledProcessError, json.JSONDecodeError, KeyError):
        return ""

then in unblock_destroy skip anything whose status is ACTIVE (a healthy node group is Terraform's to delete, in its own dependency order), collect the ones actually deleted, and poll only that set in the wait loop.

[bobbyiliev]

Good catch, fixed in 2ac511f. I verified the Karpenter dependency before taking it: module.karpenter has node_selector = local.base_node_labels and depends_on = [module.base_node_group], while cert-manager, the LBC and the operator all sit on Karpenter-provisioned generic_node_labels, so deleting the base node group out from under an early destroy failure really does strand the provisioned instances.

unblock_destroy now reads each node group's status and skips anything ACTIVE or DELETING, since a healthy node group is Terraform's to delete in its own dependency order. It also skips a status it cannot read, on the same don't-touch-what-you-can't-identify grounds, and the wait loop polls only the node groups this hook actually deleted rather than every one on the cluster, so a node group left in place no longer burns the 300s budget.