fix: address CodeRabbit review findings for sno-to-3node role

- handlers/main.yml: emit explicit WARNING debug if oc project fails after
  all retries, keeping failed_when: false to avoid play abortion
- tasks/config.yml: add SNO_IPV4 to the AGENT_E2E_TEST_SCENARIO guidance
  message so all three supported scenarios are listed
- boot-nodes.yml: add --nvram to virsh undefine so UEFI varstore is fully
  removed before virt-install recreates the domain
- clean.yml: remove NetworkManager dnsmasq override file and reload NM
  after libvirt dnsmasq reload to clear stale local DNS entries
- create-vms.yml: remove failed_when: false from virsh net-update dns-host
  add so DNS update failures surface instead of being silently swallowed
- ignition.yml: add --request-timeout=30s to oc get configmap and
  --max-time 10 to curl so probes fail quickly on API/DNS stalls
- mco-rollout.yml: tighten drain auto-fix gate to only fire on nodes with
  a stale MCO desiredDrain annotation (drain-* prefix, != lastAppliedDrain);
  change sno_auto_fix_drain default to false (opt-in); restart MCC only once
  per cycle rather than unconditionally
- preflight.yml: indent Python snippet inside shell: | block to column 4+
  so all lines remain within the block scalar per YAML spec
- update-dns.yml: tighten API DNS check to require a known node IP via
  intersect filter; replace failed_when: false on apps check with a real
  assertion against sno_all_node_ips[0]; add retries to both checks
- verify.yml: add Assert all nodes Ready assertion before the success
  message so NotReady nodes cause a hard failure instead of passing silently

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: ehila <ehila@redhat.com>

Author: eggfoobar

deploy/openshift-clusters/roles/dev-scripts/install-dev/handlers/main.yml

@@ -19,6 +19,18 @@
   failed_when: false
   listen: Set OCP project
 
+- name: Warn if oc project failed after all retries
+  debug:
+    msg: >-
+      WARNING: 'oc project openshift-machine-api' still failing after all retries
+      (rc={{ oc_project_result.rc | default('n/a') }}).
+      stderr: {{ oc_project_result.stderr | default('') | trim }}
+  when:
+    - kubeconfig_path is defined
+    - kubeconfig_stat.stat.exists | default(false)
+    - oc_project_result.rc | default(0) != 0
+  listen: Set OCP project
+
 - name: Warn about missing kubeconfig
   debug:
     msg: >-

deploy/openshift-clusters/roles/dev-scripts/install-dev/tasks/config.yml

@@ -28,8 +28,8 @@
     msg: >-
       Config file {{ config_file[method] }} does not set AGENT_E2E_TEST_SCENARIO,
       which is required for the agent installation method. Add
-      AGENT_E2E_TEST_SCENARIO="TNF_IPV4" (fencing) or "TNA_IPV4" (arbiter) to
-      the config file.
+      AGENT_E2E_TEST_SCENARIO="TNF_IPV4" (fencing), "TNA_IPV4" (arbiter), or
+      "SNO_IPV4" (sno) to the config file.
   when:
     - method == "agent"
     - config_scenario | length == 0

deploy/openshift-clusters/roles/mutable-topology/sno-to-3node/tasks/boot-nodes.yml

@@ -55,7 +55,7 @@
     MAC="{{ sno_node_macs[item.hostname] }}"
 
     sudo virsh destroy "$VM_NAME" 2>/dev/null || true
-    sudo virsh undefine "$VM_NAME" 2>/dev/null || true
+    sudo virsh undefine "$VM_NAME" --nvram 2>/dev/null || true
 
     sudo virt-install \
       --name "$VM_NAME" \

deploy/openshift-clusters/roles/mutable-topology/sno-to-3node/tasks/clean.yml

@@ -117,6 +117,20 @@
   changed_when: true
   failed_when: false
 
+- name: "[clean] Remove NetworkManager dnsmasq override"
+  file:
+    path: "{{ sno_nm_dnsmasq_conf }}"
+    state: absent
+  become: true
+  failed_when: false
+
+- name: "[clean] Reload NetworkManager to clear stale DNS overrides"
+  systemd:
+    name: NetworkManager
+    state: reloaded
+  become: true
+  failed_when: false
+
 - name: "[clean] Cleanup complete"
   debug:
     msg: >-

deploy/openshift-clusters/roles/mutable-topology/sno-to-3node/tasks/create-vms.yml

@@ -133,7 +133,6 @@
       "<host ip='{{ item.ip }}'><hostname>{{ item.hostname }}.{{ sno_cluster_domain }}</hostname></host>" \
       --live --config
   loop: "{{ sno_new_nodes }}"
-  failed_when: false
 
 - name: "[create-vms] Add entries to addnhosts"
   lineinfile:

deploy/openshift-clusters/roles/mutable-topology/sno-to-3node/tasks/ignition.yml

@@ -1,7 +1,8 @@
 ---
 - name: "[ignition] Extract MCS CA certificate"
   shell: |
-    oc get configmap -n openshift-machine-config-operator machine-config-server-ca -o jsonpath='{.data.ca-bundle\.crt}'
+    oc get configmap -n openshift-machine-config-operator machine-config-server-ca \
+      -o jsonpath='{.data.ca-bundle\.crt}' --request-timeout=30s
   environment: "{{ sno_oc_env }}"
   register: mcs_ca_raw
   changed_when: false
@@ -18,7 +19,7 @@
 
 - name: "[ignition] Verify MCS reachability"
   shell: |
-    curl -sk https://api-int.{{ sno_cluster_domain }}:22623/healthz
+    curl -sk --max-time 10 https://api-int.{{ sno_cluster_domain }}:22623/healthz
   register: mcs_health
   changed_when: false
   failed_when: false

deploy/openshift-clusters/roles/mutable-topology/sno-to-3node/tasks/mco-rollout.yml

@@ -39,15 +39,31 @@
       exit 0
     fi
 
-    # Auto-fix drain deadlock for any cordoned node.
-    # On SNO→3node, MCD cordons a node before draining. If MCC cannot reach
-    # the node (api-int flap, RBAC timing) the drain annotation never advances
-    # and the node stays cordoned forever. Fix: uncordon, set lastAppliedDrain
-    # to the drain-<desired> value MCC is waiting for, restart MCC.
-    if [ -n "$CORDONED_NODES" ] && [ "{{ sno_auto_fix_drain | default(true) | bool }}" = "True" ]; then
+    # Auto-fix MCO drain deadlock for nodes with a stale drain annotation.
+    # Only fires when ALL of:
+    #   1. sno_auto_fix_drain is enabled
+    #   2. The node is cordoned (SchedulingDisabled)
+    #   3. MCO owns the cordon: desiredDrain starts with "drain-"
+    #   4. The drain is actually stuck: desiredDrain != lastAppliedDrain
+    # This avoids touching nodes cordoned manually or mid-drain legitimately.
+    if [ -n "$CORDONED_NODES" ] && [ "{{ sno_auto_fix_drain | default(false) | bool }}" = "True" ]; then
       DESIRED=$(oc get mcp master -o jsonpath='{.spec.configuration.name}' 2>/dev/null || echo "")
+      MCC_RESTARTED=0
       for NODE_NAME in $CORDONED_NODES; do
-        echo "DRAIN_FIX: Fixing drain deadlock on $NODE_NAME (desired=${DESIRED})..."
+        DESIRED_DRAIN=$(oc get node "$NODE_NAME" \
+          -o jsonpath='{.metadata.annotations.machineconfiguration\.openshift\.io/desiredDrain}' \
+          2>/dev/null || true)
+        LAST_DRAIN=$(oc get node "$NODE_NAME" \
+          -o jsonpath='{.metadata.annotations.machineconfiguration\.openshift\.io/lastAppliedDrain}' \
+          2>/dev/null || true)
+
+        # Skip if this is not an MCO-managed drain or drain is already advancing
+        if ! echo "$DESIRED_DRAIN" | grep -q "^drain-" || [ "$DESIRED_DRAIN" = "$LAST_DRAIN" ]; then
+          echo "DRAIN_FIX: $NODE_NAME cordoned but not a stale MCO drain (desired=${DESIRED_DRAIN}, last=${LAST_DRAIN}), skipping"
+          continue
+        fi
+
+        echo "DRAIN_FIX: Stale MCO drain on $NODE_NAME (desired=${DESIRED_DRAIN}, last=${LAST_DRAIN})"
 
         oc adm uncordon "$NODE_NAME" 2>&1 || true
 
@@ -57,11 +73,14 @@
             --overwrite 2>&1 || true
           echo "DRAIN_FIX: Set lastAppliedDrain=drain-${DESIRED} on $NODE_NAME"
         fi
+        MCC_RESTARTED=1
       done
 
-      oc delete pod -n openshift-machine-config-operator \
-        -l k8s-app=machine-config-controller 2>&1 || true
-      echo "DRAIN_FIX: Restarted MCC pod"
+      if [ "$MCC_RESTARTED" = "1" ]; then
+        oc delete pod -n openshift-machine-config-operator \
+          -l k8s-app=machine-config-controller 2>&1 || true
+        echo "DRAIN_FIX: Restarted MCC pod"
+      fi
     fi
 
     echo "RESULT=pending"

deploy/openshift-clusters/roles/mutable-topology/sno-to-3node/tasks/preflight.yml

@@ -134,13 +134,13 @@
   # previous partial run) doesn't cause the subsequent add to fail.
   shell: |
     CURRENT_IP=$(sudo virsh net-dumpxml {{ sno_libvirt_network }} | python3 -c "
-import xml.etree.ElementTree as ET, sys
-root = ET.parse(sys.stdin).getroot()
-for h in root.findall('.//dns/host'):
-    for hn in h.findall('hostname'):
-        if hn.text == 'api-int.{{ sno_cluster_domain }}':
-            print(h.get('ip', ''))
-" 2>/dev/null || true)
+    import xml.etree.ElementTree as ET, sys
+    root = ET.parse(sys.stdin).getroot()
+    for h in root.findall('.//dns/host'):
+        for hn in h.findall('hostname'):
+            if hn.text == 'api-int.{{ sno_cluster_domain }}':
+                print(h.get('ip', ''))
+    " 2>/dev/null || true)
 
     if [ -n "$CURRENT_IP" ]; then
       sudo virsh net-update {{ sno_libvirt_network }} delete dns-host \

deploy/openshift-clusters/roles/mutable-topology/sno-to-3node/tasks/update-dns.yml

@@ -59,14 +59,24 @@
     dig +short api.{{ sno_cluster_domain }}
   register: dns_api_verify
   changed_when: false
-  failed_when: dns_api_verify.stdout | trim == ""
+  failed_when: >-
+    dns_api_verify.stdout | trim == "" or
+    sno_all_node_ips | intersect(dns_api_verify.stdout_lines) | length == 0
+  retries: 6
+  delay: 5
+  until: >-
+    dns_api_verify.stdout | trim != "" and
+    sno_all_node_ips | intersect(dns_api_verify.stdout_lines) | length > 0
 
 - name: "[dns] Verify apps DNS resolution"
   shell: |
     dig +short test.apps.{{ sno_cluster_domain }}
   register: dns_apps_verify
   changed_when: false
-  failed_when: false
+  failed_when: sno_all_node_ips[0] not in dns_apps_verify.stdout_lines
+  retries: 6
+  delay: 5
+  until: sno_all_node_ips[0] in dns_apps_verify.stdout_lines
 
 - name: "[dns] DNS update complete"
   debug:

deploy/openshift-clusters/roles/mutable-topology/sno-to-3node/tasks/verify.yml

@@ -93,6 +93,11 @@
     that: (verify_nodes.stdout_lines | length) >= 3
     fail_msg: "Expected 3+ nodes, found {{ verify_nodes.stdout_lines | length }}"
 
+- name: "[verify] Assert all nodes Ready"
+  assert:
+    that: "'NotReady' not in verify_nodes.stdout"
+    fail_msg: "One or more nodes are NotReady:\n{{ verify_nodes.stdout }}"
+
 - name: "[verify] Assert HA topology"
   assert:
     that: verify_topo.stdout == "HighlyAvailable"