[CI] Extend withHealthyNode to blacklist k8s hosts

[leo-automation]

Summary

withHealthyNode currently blacklists only the Jenkins agent name. For Kubernetes workers, that name identifies an ephemeral pod, so a replacement pod can be scheduled onto the same unhealthy physical node.

This change:

• Tracks rejected physical Kubernetes nodes using K8S_NODE_NAME.
• Shares the physical-node blacklist across parallel matrix rows.
• Rejects replacement pods scheduled on a node already blacklisted during the build.
• Records the physical node after health-check failures and recoverable per-server failures.
• Preserves the existing behavior for bare-metal agents.
• Warns when a Kubernetes agent does not expose K8S_NODE_NAME.

The blacklist is scoped to the current Jenkins build and does not modify Kubernetes nodes, labels, taints, or scheduling configuration.

Jenkins configuration

All templates in the frameworks-devops-k8s-cluster Jenkins cloud now expose the physical node through the Kubernetes Downward API:

- name: K8S_NODE_NAME
  valueFrom:
    fieldRef:
      apiVersion: v1
      fieldPath: spec.nodeName

[justinrosner]

Changes look good to me. For testing is there a way to force node selection to a bad node so that we can see this working?

[leo-automation]

Changes look good to me. For testing is there a way to force node selection to a bad node so that we can see this working?

It already did so on the PR's CI run

[rocmlir-pr-reviewer]

This early return exits the node {} block after attempt += 1 has already run at line 1346, so a pod rejected purely because it landed on an already-blacklisted physical host burns one of the three maxAttempts without ever executing a health check. Adding the ephemeral pod name to blacklist does not prevent the next pod from being scheduled onto the same host, so the collision can repeat: because the map is now shared across parallel matrix rows, a row that has failed nothing itself can consume all three attempts on scheduling collisions and then hit error "No healthy node found..." at line 1440 — escalating one unhealthy host into a whole-job re-kick (or a red PR build). Suggest not charging scheduling rejections to the health-check budget: track them in a separate counter with its own cap (e.g. rescheduleAttempts < 2 * maxAttempts) and decrement attempt on this path, so the loop still terminates but the health-check budget is preserved for actual health checks.

[rocmlir-pr-reviewer]

This records the physical host on any health-check exception, but the health-check block is not purely node-scoped: gitHealthCheck() (line 64) probes the SCM remote over the network and errors on auth/connectivity problems, and healthChecks() is caller-supplied. Previously such a failure only excluded one ephemeral agent from one row's retry loop; now it permanently removes the underlying physical node from the pool for every parallel row for the remainder of the build. During a transient GitHub or network blip every row's health check fails, and the shared map can blacklist the entire pool, converting a recoverable blip into "No healthy node found" for the whole job — the opposite of this PR's goal. Suggest either recording the physical node only for failures that are demonstrably node-scoped (e.g. reuse isPerServerTransient("${err}") to gate this call, as is already done on the body path at line 1411), or capping BLACKLISTED_K8S_NODES.size() and skipping further physical-node blacklisting once the cap is reached.

[rocmlir-pr-reviewer]

Kubernetes-agent detection hinges on an undocumented k8s- pod-name prefix. If a pod template in the frameworks-devops-k8s-cluster cloud is renamed, this warning silently stops firing and the loss of physical-node blacklisting becomes invisible — exactly the failure mode the warning exists to surface. Suggest hoisting the prefix into a named @Field constant next to BLACKLISTED_K8S_NODES (line 17) with a comment naming the Jenkins cloud whose templates it must track, so the coupling is discoverable from one place. Minor secondary point: agentName is assigned from env.NODE_NAME inside a node {} block, where it is always set, so the ?. safe-navigation here is dead defensiveness and can be a plain .startsWith(...).

[rocmlir-pr-reviewer]

Verdict: COMMENT  ·  Findings: 3 (0 Critical, 2 Major, 1 Minor)

---

Scope

Single-file CI change to mlir/utils/jenkins/Jenkinsfile. withHealthyNode previously blacklisted only the Jenkins agent name, which on the Kubernetes cloud is an ephemeral pod name, so a replacement pod could be rescheduled onto the same unhealthy physical host. The PR adds a build-scoped ConcurrentHashMap of physical nodes (BLACKLISTED_K8S_NODES, read from K8S_NODE_NAME via the Downward API), shares it across parallel matrix rows, rejects pods landing on an already-blacklisted host, and records the host on both health-check failures and per-server transients. Bare-metal agents are unaffected (k8sNode stays null, so every new code path short-circuits). The mechanics are sound: putIfAbsent gives a race-free first-writer-wins record, the @Field map matches the existing DOCKER_ARGS_BY_NODE pattern, and the env.NODE_NAME → local agentName refactor is a faithful rename.

Findings

Two Major reliability concerns and one Minor maintainability nit, all on newly added lines:

• Jenkinsfile:1365 — a pod rejected purely for scheduling onto a blacklisted host consumes one of maxAttempts without ever running a health check, so a row can exhaust its budget on scheduling collisions alone.
• Jenkinsfile:1382 — health-check failures are not always node-scoped (gitHealthCheck() probes the remote over the network), yet they now evict the physical host for every matrix row for the rest of the build.
• Jenkinsfile:1357 — the k8s- agent-name prefix is an undocumented naming assumption; if it drifts, the diagnostic warning silently stops firing.

Verdict is COMMENT rather than REQUEST_CHANGES: nothing here is Critical, and both Major items are bounded by the existing self-healing paths (blacklist is build-scoped, and "no healthy node" re-kicks the job). They are worth resolving before merge because they cut against the PR's own robustness goal.

Notes

• Line 1 of this file says "ON CHANGING THESE, ALSO CHANGE Jenkinsfile.downstream". That file is not present in this repo (private CI), so please confirm the mirror update was made there.
• The Jenkins-side pod-template change is a prerequisite: until every template in the cloud exports K8S_NODE_NAME, the new logic degrades to today's behavior plus a warning. That degradation is graceful, which is the right call.
• Groovy/Jenkinsfile changes fall outside the C++/MLIR-centric bullets in docs/PR_REVIEW_CHECKLIST.md; the findings above are cited as retry-loop correctness and maintainability rather than to a specific bullet.

CI status

Jenkins is FAILURE and ml-ci-internal.amd.com is ERROR at this SHA. Given the PR rewrites the Jenkins node-selection path, please confirm whether these are pre-existing/infra flakes or caused by this change before merging. The review check is this auto-review pipeline and is expected to be in progress.