Skip to content

Skip ProxyJump for baremetal nodes when direct SSH works #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fonta-rh wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Skip ProxyJump for baremetal nodes when direct SSH works #91

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion deploy/openshift-clusters/roles/common/tasks/update-baremetal-inventory.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,31 @@
label: "{{ item.item.name }}"
when: ssh_key_propagation is defined

- name: Test direct SSH access to first node from laptop
delegate_to: localhost
become: false
shell: |
ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
-o ConnectTimeout=5 -o BatchMode=yes \
"core@{{ parsed_vm_entries[0].ip }}" "echo ok" 2>/dev/null
register: direct_ssh_test
failed_when: false
changed_when: false
when: ssh_key_valid | default(false)
Comment on lines +95 to +105

@coderabbitai coderabbitai Bot Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Guard against empty parsed_vm_entries before indexing [0].

parsed_vm_entries[0].ip will raise a Jinja error if the baremetal_nodes group ends up empty, aborting the whole play with an unclear error instead of a meaningful message.

🛡️ Proposed guard 
+- name: Fail if no baremetal nodes found
+  fail:
+    msg: "No entries found in parsed_vm_entries; cannot test direct SSH access."
+  when: parsed_vm_entries | length == 0
+
 - name: Test direct SSH access to first node from laptop
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Test direct SSH access to first node from laptop
delegate_to: localhost
become: false
shell: |
ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
-o ConnectTimeout=5 -o BatchMode=yes \
"core@{{ parsed_vm_entries[0].ip }}" "echo ok" 2>/dev/null
register: direct_ssh_test
failed_when: false
changed_when: false
when: ssh_key_valid | default(false)
- name: Fail if no baremetal nodes found
fail:
msg: "No entries found in parsed_vm_entries; cannot test direct SSH access."
when: parsed_vm_entries | length == 0
- name: Test direct SSH access to first node from laptop
delegate_to: localhost
become: false
shell: |
ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
-o ConnectTimeout=5 -o BatchMode=yes \
"core@{{ parsed_vm_entries[0].ip }}" "echo ok" 2>/dev/null
register: direct_ssh_test
failed_when: false
changed_when: false
when: ssh_key_valid | default(false)
🤖 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 `@deploy/openshift-clusters/roles/common/tasks/update-baremetal-inventory.yml`
around lines 95 - 105, The direct SSH test in the task that uses
parsed_vm_entries[0].ip needs a guard for an empty baremetal_nodes result before
indexing the first entry. Update the when condition or add a prior check in the
update-baremetal-inventory workflow so the SSH probe only runs when
parsed_vm_entries is defined and has at least one item, and otherwise skip or
fail with a clear message. Keep the fix local to the existing direct_ssh_test
task and its surrounding conditions.


- name: Set direct access fact
set_fact:
direct_node_access: "{{ (direct_ssh_test.rc | default(1)) == 0 }}"

- name: Display connectivity mode
debug:
msg: >-
{% if direct_node_access | bool %}
Direct SSH to nodes works — inventory will use direct access (no ProxyJump).
{% else %}
Direct SSH to nodes unavailable — inventory will use ProxyJump via {{ provisioning_host_user }}@{{ provisioning_host_addr }}.
{% endif %}

- name: Update inventory file with cluster nodes
delegate_to: localhost
become: false
Expand Down Expand Up @@ -123,7 +148,11 @@
{% endfor %}

[cluster_nodes:vars]
{% if direct_node_access | bool %}
ansible_ssh_common_args="-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null"
{% else %}
ansible_ssh_common_args="-o ProxyJump={{ provisioning_host_user }}@{{ provisioning_host_addr }} -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null"
{% endif %}
ansible_user=core

- name: Write updated inventory
Expand All @@ -140,6 +169,10 @@
- {{ entry.name }}: {{ entry.ip }}
{% endfor %}

Nodes are accessible via ProxyJump through {{ provisioning_host_user }}@{{ provisioning_host_addr }}
{% if direct_node_access | bool %}
Nodes are accessible directly from this machine (no ProxyJump).
{% else %}
Nodes are accessible via ProxyJump through {{ provisioning_host_user }}@{{ provisioning_host_addr }}.
{% endif %}

Test: ansible -i inventory_baremetal.ini -m ping cluster_nodes