test(breakfix): implement cordon node validation - #572
Conversation
Signed-off-by: Hasan Khan <hasank@nvidia.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe provider now runs a Kubernetes cordon test. It selects a schedulable node, verifies existing and new workload behavior, restores the node, reports JSON results, and adds workflow and failure-path tests. ChangesKubernetes cordon validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant cordon_node
participant kubectl
participant Kubernetes
TestRunner->>cordon_node: invoke cordon workflow
cordon_node->>kubectl: select node and create existing probe
kubectl->>Kubernetes: create and inspect probe pod
cordon_node->>kubectl: cordon node and create second probe
kubectl->>Kubernetes: mark node unschedulable
Kubernetes-->>cordon_node: existing probe remains Ready
Kubernetes-->>cordon_node: second probe remains unschedulable
cordon_node->>kubectl: delete probes and uncordon node
kubectl->>Kubernetes: restore node schedulability
cordon_node-->>TestRunner: emit structured JSON result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py`:
- Around line 238-321: Update main to restore the required DEMO_MODE gate before
any kubectl or live validation work: when ISVCTL_DEMO_MODE=1, return the
provider-neutral dummy success result immediately, and when demo mode is
disabled, return the required not-implemented status instead of executing the
cordon operation. Keep the existing live validation logic out of the my-isv
template path or otherwise prevent it from being reached.
- Around line 46-52: Update the kubectl execution helper around subprocess.run
to pass a finite subprocess timeout and add a nonzero --request-timeout argument
to every kubectl invocation. Catch subprocess.TimeoutExpired and translate it to
CordonTestError, including cleanup handling in _cleanup, so timed-out commands
cannot leave the node cordoned.
- Line 255: Update the node cordon flow around _select_node and the subsequent
kubectl operations to atomically claim ownership: conditionally update the node
using its metadata.resourceVersion, requiring spec.unschedulable to be false
before setting it true, and record cleanup ownership only after that update
succeeds. Make uncordon conditional on the ownership established by that update
so a later actor’s cordon remains intact, and add coverage for concurrent cordon
attempts.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 57f4ce3b-41d5-448a-b1fc-121b42426213
📒 Files selected for processing (3)
isvctl/configs/providers/my-isv/config/k8s.yamlisvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.pyisvctl/tests/test_my_isv_cordon_node.py
| completed = subprocess.run( | ||
| [*kubectl, *args], | ||
| input=input_text, | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file='isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py'
printf '%s\n' '--- outline ---'
ast-grep outline "$file" --view expanded || true
printf '%s\n' '--- target file ---'
cat -n "$file"
printf '%s\n' '--- related subprocess and kubectl usage ---'
rg -n -C 3 'subprocess\.run|request-timeout|def _run|finally|uncordon|kubectl' isvctl/configs/providers/my-isv
printf '%s\n' '--- tests and docs mentioning cordon_node ---'
rg -n -C 3 'cordon_node|CordonTestError|request-timeout|TimeoutExpired' . --glob '!node_modules' --glob '!build' --glob '!dist'Repository: NVIDIA/ai-cloud-validation
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- focused cordon tests ---'
cat -n isvctl/tests/test_my_isv_cordon_node.py
printf '%s\n' '--- executor timeout context ---'
sed -n '370,445p' isvctl/src/isvctl/orchestrator/step_executor.py
printf '%s\n' '--- configuration for the cordon step ---'
rg -n -C 8 'cordon|timeout|breakfix' isvctl/configs/providers/my-isv/config/k8s.yaml isvctl/configs/suites/k8s.yaml isvctl/configs/suites/bare_metal.yaml
printf '%s\n' '--- direct references to the test timeout ---'
rg -n -C 3 'step.*timeout|timeout.*step|subprocess\.run\(' isvctl/src/isvctl isvctl/tests --glob '*.py' | head -240Repository: NVIDIA/ai-cloud-validation
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import subprocess
import sys
import time
script = (
"import time\n"
"time.sleep(0.25)\n"
)
started = time.monotonic()
completed = subprocess.run(
[sys.executable, "-c", script],
capture_output=True,
text=True,
check=False,
)
elapsed_without_timeout = time.monotonic() - started
started = time.monotonic()
try:
subprocess.run(
[sys.executable, "-c", script],
capture_output=True,
text=True,
check=False,
timeout=0.05,
)
except subprocess.TimeoutExpired as exc:
elapsed_with_timeout = time.monotonic() - started
print({
"without_timeout_returncode": completed.returncode,
"without_timeout_elapsed_at_least_0_2": elapsed_without_timeout >= 0.2,
"with_timeout_raises_timeout_expired": True,
"with_timeout_elapsed_below_0_2": elapsed_with_timeout < 0.2,
"timeout_value": exc.timeout,
})
else:
print({
"without_timeout_returncode": completed.returncode,
"without_timeout_elapsed_at_least_0_2": elapsed_without_timeout >= 0.2,
"with_timeout_raises_timeout_expired": False,
})
PYRepository: NVIDIA/ai-cloud-validation
Length of output: 352
🌐 Web query:
kubectl generated command cordon --request-timeout default 0 request-timeout documentation
💡 Result:
The --request-timeout flag is a valid, inherited option available for the kubectl cordon command [1][2]. According to official Kubernetes documentation, the --request-timeout flag specifies the length of time to wait before giving up on a single server request [1][3]. The default value for this flag is "0," which signifies that there is no timeout for requests [1][4]. If a non-zero value is specified, it must include a corresponding time unit (e.g., 1s, 2m, or 3h) [1][5]. Because this is a global flag inherited by kubectl commands, it allows users to override the default client-side timeout behavior when performing operations like marking a node as unschedulable [3][6].
Citations:
- 1: https://kubernetes.io/docs/reference/kubectl/generated/kubectl_cordon/
- 2: https://kubernetes.io/docs/reference/kubectl/kubectl/
- 3: https://kubernetes.io/docs/reference/kubectl/generated/kubectl_options/
- 4: https://man.archlinux.org/man/kubectl-cordon.1.en
- 5: https://manpages.opensuse.org/Tumbleweed/kubernetes1.36-client-common/kubectl-cordon.1.en.html
- 6: https://kubernetes.io/docs/reference/kubectl/generated/
Bound every kubectl process.
Pass a finite timeout to subprocess.run and a nonzero --request-timeout to kubectl. Translate subprocess.TimeoutExpired to CordonTestError, including in _cleanup, so a hung command cannot leave the node cordoned.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py` around lines
46 - 52, Update the kubectl execution helper around subprocess.run to pass a
finite subprocess timeout and add a nonzero --request-timeout argument to every
kubectl invocation. Catch subprocess.TimeoutExpired and translate it to
CordonTestError, including cleanup handling in _cleanup, so timed-out commands
cannot leave the node cordoned.
| def main() -> int: | ||
| """Run the reversible cordon test and emit its provider-neutral JSON result.""" | ||
| args = _parser().parse_args() | ||
| operation: dict[str, Any] = { | ||
| "cordoned": False, | ||
| "new_workloads_blocked": False, | ||
| "existing_workloads_running": False, | ||
| } | ||
| result: dict[str, Any] = {"success": False, "platform": "my-isv", "test_name": "cordon_node"} | ||
| kubectl: list[str] = [] | ||
| created_pods: list[str] = [] | ||
| cordoned_node: str | None = None | ||
|
|
||
| try: | ||
| if args.timeout_seconds <= 0 or args.poll_interval_seconds <= 0: | ||
| raise CordonTestError("Timeout and poll interval must be greater than zero") | ||
| kubectl = _kubectl_command() | ||
| node_name, hostname, tolerations = _select_node(kubectl, args.node) | ||
| operation["node_id"] = node_name | ||
| suffix = uuid.uuid4().hex[:8] | ||
| existing_pod = f"isvtest-bfx-existing-{suffix}" | ||
| blocked_pod = f"isvtest-bfx-blocked-{suffix}" | ||
|
|
||
| _run( | ||
| kubectl, | ||
| "create", | ||
| "-f", | ||
| "-", | ||
| input_text=_pod_manifest(existing_pod, args.namespace, hostname, args.image, tolerations), | ||
| ) | ||
| created_pods.append(existing_pod) | ||
| _run( | ||
| kubectl, | ||
| "wait", | ||
| "--for=condition=Ready", | ||
| f"pod/{existing_pod}", | ||
| "-n", | ||
| args.namespace, | ||
| f"--timeout={args.timeout_seconds:g}s", | ||
| ) | ||
|
|
||
| _run(kubectl, "cordon", node_name) | ||
| cordoned_node = node_name | ||
| node = _json_output(_run(kubectl, "get", "node", node_name, "-o", "json"), f"node {node_name}") | ||
| operation["cordoned"] = node.get("spec", {}).get("unschedulable") is True | ||
| if not operation["cordoned"]: | ||
| raise CordonTestError(f"Node {node_name!r} was not marked unschedulable") | ||
|
|
||
| operation["existing_workloads_running"] = _pod_is_ready_on_node( | ||
| _get_pod(kubectl, args.namespace, existing_pod), node_name | ||
| ) | ||
| if not operation["existing_workloads_running"]: | ||
| raise CordonTestError("Existing probe pod did not remain Ready on the cordoned node") | ||
|
|
||
| _run( | ||
| kubectl, | ||
| "create", | ||
| "-f", | ||
| "-", | ||
| input_text=_pod_manifest(blocked_pod, args.namespace, hostname, args.image, tolerations), | ||
| ) | ||
| created_pods.append(blocked_pod) | ||
| operation["new_workloads_blocked"] = _wait_for_unschedulable( | ||
| kubectl, | ||
| args.namespace, | ||
| blocked_pod, | ||
| args.timeout_seconds, | ||
| args.poll_interval_seconds, | ||
| ) | ||
| if not operation["new_workloads_blocked"]: | ||
| raise CordonTestError("New probe pod was not confirmed unschedulable on the cordoned node") | ||
| result["success"] = True | ||
| except CordonTestError as exc: | ||
| result["error"] = str(exc) | ||
| finally: | ||
| cleanup_errors = _cleanup(kubectl, args.namespace, created_pods, cordoned_node) if kubectl else [] | ||
| if cleanup_errors: | ||
| result["success"] = False | ||
| result["cleanup_errors"] = cleanup_errors | ||
| result.setdefault("error", "Cordon test cleanup failed") | ||
|
|
||
| result["operation"] = operation | ||
| print(json.dumps(result, indent=2)) | ||
| return 0 if result["success"] else 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Restore the required my-isv execution gate.
When ISVCTL_DEMO_MODE=1, this script calls kubectl instead of returning dummy success. When demo mode is disabled, it performs a real operation instead of reporting not-implemented status. This makes local demo execution require a cluster.
Restore the required gate, or move this live validation to a non-template provider script.
As per coding guidelines, my-isv scripts must retain a DEMO_MODE gate, return dummy success in demo mode, and report not-implemented status for real runs.
🧰 Tools
🪛 ast-grep (0.45.0)
[info] 319-319: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py` around lines
238 - 321, Update main to restore the required DEMO_MODE gate before any kubectl
or live validation work: when ISVCTL_DEMO_MODE=1, return the provider-neutral
dummy success result immediately, and when demo mode is disabled, return the
required not-implemented status instead of executing the cordon operation. Keep
the existing live validation logic out of the my-isv template path or otherwise
prevent it from being reached.
Source: Coding guidelines
| if args.timeout_seconds <= 0 or args.poll_interval_seconds <= 0: | ||
| raise CordonTestError("Timeout and poll interval must be greater than zero") | ||
| kubectl = _kubectl_command() | ||
| node_name, hostname, tolerations = _select_node(kubectl, args.node) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file=$(git ls-files 'isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py')
printf '%s\n' "$file"
wc -l "$file"
ast-grep outline "$file" || true
printf '\n--- target sections ---\n'
sed -n '200,320p' "$file"
printf '\n--- related symbols and tests ---\n'
rg -n --glob '!dist' --glob '!build' 'cordoned_node|_select_node|cordon_node|cordon' isvctl/configs/providers/my-isv isvctl | head -240Repository: NVIDIA/ai-cloud-validation
Length of output: 16261
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- cordon_node.py: imports through selection and cleanup ---'
sed -n '1,225p' isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py
printf '%s\n' '--- test_my_isv_cordon_node.py ---'
sed -n '1,245p' isvctl/tests/test_my_isv_cordon_node.pyRepository: NVIDIA/ai-cloud-validation
Length of output: 17357
🌐 Web query:
Kubernetes kubectl cordon already cordoned node exit code successful behavior source
💡 Result:
When you run kubectl cordon on a node that is already cordoned, the command exits successfully with a return code of 0 [1]. The behavior is intentional and idempotent [1]. In the kubectl source code, the CordonHelper utility checks whether the node.Spec.Unschedulable field is already set to the desired state (in this case, true) [2]. If the node is already cordoned, the command determines that no update is required and performs no patch or update operation on the node [2][3]. It then typically prints a message to standard output confirming the node status (e.g., "node/name already cordoned") and exits successfully [4][3]. Because the command encounters no actual error or failure when the node is already in the target state, it does not trigger an error exit code [3]. This behavior ensures that scripts or automation pipelines using kubectl cordon do not break if the command is executed repeatedly or on nodes that were cordoned in previous steps [1].
Citations:
- 1: https://thecodeforge.io/devops/kubernetes-node-management/
- 2: https://github.com/kubernetes/kubectl/blob/master/pkg/drain/cordon.go
- 3: https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/drain/drain.go
- 4: Node Drain behaving unexpectedly kubernetes/kubectl#1568
🌐 Web query:
Kubernetes API optimistic concurrency conditional update resourceVersion JSON patch test spec.unschedulable node
💡 Result:
Kubernetes uses optimistic concurrency control via the resourceVersion field in an object's metadata to prevent lost updates [1][2][3]. By including the current resourceVersion in an update or patch request, a client can ensure the operation only succeeds if the object has not been modified since it was last read [2][4][3]. If the provided resourceVersion does not match the server-side version, the API server rejects the request with a 409 Conflict error [1][2][4]. To perform a conditional update on a node's unschedulable status, you can follow this pattern: 1. Retrieve the latest node object, including its current metadata.resourceVersion [2][3]. 2. Construct your patch payload. While a standard patch might omit the version, you can include the resourceVersion within the metadata field to enforce concurrency control [3]. For example, using a strategic merge patch to set a node to unschedulable: { "metadata": { "resourceVersion": "CURRENT_VERSION_STRING" }, "spec": { "unschedulable": true } } When using JSON Patch (RFC 6902), which uses an array of operations, you cannot directly include the resourceVersion in the same way as a merge patch because the format is strictly defined by the operations [5]. However, the Kubernetes API supports conditional PATCH operations where you can effectively use the resourceVersion to ensure the update remains atomic [6]. Note that standard practice often favors using the replace operation (PUT) when full object concurrency control is required, or ensuring the patch itself is applied to the intended version [5][2]. If you are using kubectl, the command to set a node as unschedulable is: kubectl patch node -p '{"spec":{"unschedulable":true}}' [7][8] To make this conditional, you would typically fetch the resource, verify the version, and use the strategic merge patch approach shown above [3]. The resourceVersion must be treated as an opaque string by clients and passed back exactly as received from the server [2][4].
Citations:
- 1: https://kubernetes.io/docs/reference/using-api/api-concepts/
- 2: https://github.com/kubernetes/community/blob/61f3d0/contributors/devel/sig-architecture/api-conventions.md
- 3: https://oneuptime.com/blog/post/2026-02-09-optimistic-concurrency-resourceversion/view
- 4: https://github.com/kubernetes/community/blob/0e9fa4a1c45203527a7ce35eaff09204d6b7b331/contributors/devel/sig-architecture/api-conventions.md
- 5: Is there a possibilty to lock the Config Map before updating it to avoid race conditions kubernetes-client/java#1269
- 6: Document conditional updates for PATCH operations kubernetes/website#36864
- 7: https://kubernetes.website.cncfstack.com/docs/reference/kubectl/generated/kubectl_patch/
- 8: https://kubernetes.io/docs/reference/kubectl/generated/kubectl_patch/
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import ast
from pathlib import Path
source_path = Path("isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py")
tree = ast.parse(source_path.read_text())
main = next(
node for node in tree.body
if isinstance(node, ast.FunctionDef) and node.name == "main"
)
events = []
for node in ast.walk(main):
if isinstance(node, ast.Assign):
value = node.value
if isinstance(value, ast.Call) and isinstance(value.func, ast.Name):
if value.func.id == "_select_node":
events.append(("select", node.lineno))
elif value.func.id == "_run" and len(value.args) >= 2:
command = value.args[1]
if isinstance(command, ast.Constant) and command.value == "cordon":
events.append(("cordon", node.lineno))
if (
isinstance(value, ast.Name)
and value.id == "node_name"
and any(isinstance(target, ast.Name) and target.id == "cordoned_node" for target in node.targets)
):
events.append(("record_ownership", node.lineno))
if isinstance(node, ast.Assign) and isinstance(node.value, ast.Call):
call = node.value
if isinstance(call.func, ast.Name) and call.func.id == "_cleanup":
events.append(("cleanup", node.lineno))
ordered = sorted(events, key=lambda item: item[1])
print("main event order:", ordered)
expected = ["select", "cordon", "record_ownership", "cleanup"]
actual = [name for name, _ in ordered]
assert actual == expected, (actual, expected)
# Standalone model of the documented idempotent cordon behavior.
node = {"unschedulable": False, "resource_version": "v1"}
selected_version = node["resource_version"]
node["unschedulable"] = True
node["resource_version"] = "v2" # another actor cordons after selection
cordon_returncode = 0 # kubectl cordon treats an already-cordoned node as success
ownership_recorded = cordon_returncode == 0
assert ownership_recorded
assert node["unschedulable"] is True
if ownership_recorded:
node["unschedulable"] = False # _cleanup invokes uncordon unconditionally
print("race result:", {
"selected_resource_version": selected_version,
"current_resource_version": node["resource_version"],
"final_unschedulable": node["unschedulable"],
})
assert node["unschedulable"] is False
PY
printf '\n--- tests covering ownership races ---\n'
rg -n 'concurrent|race|already cordon|precord|cordon' isvctl/tests/test_my_isv_cordon_node.pyRepository: NVIDIA/ai-cloud-validation
Length of output: 403
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import ast
from pathlib import Path
source_path = Path("isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py")
tree = ast.parse(source_path.read_text())
main = next(node for node in tree.body if isinstance(node, ast.FunctionDef) and node.name == "main")
events = []
for node in ast.walk(main):
call = None
if isinstance(node, ast.Expr):
call = node.value
elif isinstance(node, ast.Assign):
call = node.value
if not isinstance(call, ast.Call) or not isinstance(call.func, ast.Name):
continue
if call.func.id == "_select_node":
events.append(("select", node.lineno))
elif call.func.id == "_run" and len(call.args) >= 2:
command = call.args[1]
if isinstance(command, ast.Constant) and command.value == "cordon":
events.append(("cordon", node.lineno))
elif call.func.id == "_cleanup":
events.append(("cleanup", node.lineno))
if (
isinstance(node, ast.Assign)
and isinstance(call, ast.Name)
and call.id == "node_name"
and any(isinstance(target, ast.Name) and target.id == "cordoned_node" for target in node.targets)
):
events.append(("record_ownership", node.lineno))
ordered = sorted(events, key=lambda item: item[1])
print("main event order:", ordered)
expected = ["select", "cordon", "record_ownership", "cleanup"]
actual = [name for name, _ in ordered]
assert actual == expected, (actual, expected)
node = {"unschedulable": False, "resource_version": "v1"}
selected_version = node["resource_version"]
node["unschedulable"] = True
node["resource_version"] = "v2"
cordon_returncode = 0
ownership_recorded = cordon_returncode == 0
assert ownership_recorded
if ownership_recorded:
node["unschedulable"] = False
print("race result:", {
"selected_resource_version": selected_version,
"current_resource_version": node["resource_version"],
"final_unschedulable": node["unschedulable"],
})
assert node["unschedulable"] is False
PY
printf '\n--- ownership-related tests ---\n'
rg -n 'concurrent|race|precord|cordon|uncordon' isvctl/tests/test_my_isv_cordon_node.pyRepository: NVIDIA/ai-cloud-validation
Length of output: 383
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import ast
from pathlib import Path
source_path = Path("isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py")
tree = ast.parse(source_path.read_text())
main = next(node for node in tree.body if isinstance(node, ast.FunctionDef) and node.name == "main")
events = []
for node in ast.walk(main):
if isinstance(node, ast.Assign):
if (
isinstance(node.value, ast.Name)
and node.value.id == "node_name"
and any(isinstance(target, ast.Name) and target.id == "cordoned_node" for target in node.targets)
):
events.append(("record_ownership", node.lineno))
call_nodes = []
if isinstance(node, ast.Expr) and isinstance(node.value, ast.Call):
call_nodes.append(node.value)
elif isinstance(node, ast.Assign):
call_nodes.extend(
child for child in ast.walk(node.value)
if isinstance(child, ast.Call)
)
for call in call_nodes:
if not isinstance(call.func, ast.Name):
continue
if call.func.id == "_select_node":
events.append(("select", call.lineno))
elif call.func.id == "_cleanup":
events.append(("cleanup", call.lineno))
elif call.func.id == "_run" and len(call.args) >= 2:
command = call.args[1]
if isinstance(command, ast.Constant) and command.value == "cordon":
events.append(("cordon", call.lineno))
ordered = sorted(set(events), key=lambda item: item[1])
print("main event order:", ordered)
expected = ["select", "cordon", "record_ownership", "cleanup"]
actual = [name for name, _ in ordered]
assert actual == expected, (actual, expected)
node = {"unschedulable": False, "resource_version": "v1"}
selected_version = node["resource_version"]
node["unschedulable"] = True
node["resource_version"] = "v2"
cordon_returncode = 0
ownership_recorded = cordon_returncode == 0
assert ownership_recorded
if ownership_recorded:
node["unschedulable"] = False
print("race result:", {
"selected_resource_version": selected_version,
"current_resource_version": node["resource_version"],
"final_unschedulable": node["unschedulable"],
})
assert node["unschedulable"] is False
PY
printf '\n--- ownership-related tests ---\n'
rg -n 'concurrent|race|precord|cordon|uncordon' isvctl/tests/test_my_isv_cordon_node.pyRepository: NVIDIA/ai-cloud-validation
Length of output: 2610
Make cordon ownership atomic.
kubectl cordon succeeds when the node is already cordoned. Use a conditional update with the node's metadata.resourceVersion that requires spec.unschedulable to be false before setting it to true. Record cleanup ownership only after that update succeeds. Uncordon conditionally so a later actor's cordon is preserved. Add tests for concurrent cordon attempts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@isvctl/configs/providers/my-isv/scripts/breakfix/cordon_node.py` at line 255,
Update the node cordon flow around _select_node and the subsequent kubectl
operations to atomically claim ownership: conditionally update the node using
its metadata.resourceVersion, requiring spec.unschedulable to be false before
setting it true, and record cleanup ownership only after that update succeeds.
Make uncordon conditional on the ownership established by that update so a later
actor’s cordon remains intact, and add coverage for concurrent cordon attempts.
Signed-off-by: Hasan Khan <hasank@nvidia.com>
Signed-off-by: Hasan Khan <hasank@nvidia.com>
Signed-off-by: Hasan Khan <hasank@nvidia.com>
1 participant
Summary
my-isvas a renameable provider scaffold and put the real BFX01-04 implementation underproviders/sharedkubernetes-breakfix.yamlconfiguration instead of mutating ordinary Minikube runsSafety
ISVTEST_BREAKFIX_ALLOW_MUTATION=1before anykubectlcallISVTEST_BREAKFIX_NODEon multi-node clusters; only a single-node cluster may auto-selectresourceVersion, expected schedulability, and a unique owner annotationValidation
uvx pre-commit run -a(all hooks passed)make test(3,056 passed, 175 deselected)uv run isvctl test validate -f isvctl/configs/providers/kubernetes-breakfix.yaml(valid)CordonNodeCheckpassed; afterward the node was schedulable, the owner annotation was absent, and no BFX01-04 pods remainedaz51-dev4-dh1-cp-6022:CordonNodeCheckpassed; before and after, the node was Ready and schedulable with no owner annotation; no BFX01-04 pods remainedBFX01-04 is exercised through the Kubernetes API. This change does not claim that the NICo tenant REST API exposes a cordon operation. Result upload was intentionally disabled for local/staging validation.
Closes #209