Fix preflight check failure caused by incorrect ansible_facts lookup …

[swatithombe-blip]

…for interfaces with hyphens (e.g., br-ext → ansible_br_ext)

We have identified the root cause of the issue.

The failure occurs due to how Ansible stores network interface facts. Interfaces
containing hyphens (e.g., br-ext, bond-mgmt) are internally represented with
underscores in ansible_facts (e.g., ansible_br_ext). The existing logic attempted
to access these facts using the original interface names, resulting in undefined
variable errors during preflight checks.

We have implemented a fix to normalize interface names before accessing
ansible_facts and validated the changes in our setup.

Tracker: (https://tracker.ceph.com/issues/75549)

[ceph-jenkins]

Thank you for your contribution. Since you, the author, are not a member of the Ceph GitHub Org yet, our CI will not automatically run. Any member of the Ceph Org may comment "ok to test" to allow the Jenkins jobs to run.

[ceph-jenkins]

Thank you for your contribution. Since you, the author, are not a member of the Ceph GitHub Org yet, our CI will not automatically run. Any member of the Ceph Org may comment "ok to test" to allow the Jenkins jobs to run.

[ceph-jenkins]

Thank you for your contribution. Since you, the author, are not a member of the Ceph GitHub Org yet, our CI will not automatically run. Any member of the Ceph Org may comment "ok to test" to allow the Jenkins jobs to run.

[ceph-jenkins]

Thank you for your contribution. Since you, the author, are not a member of the Ceph GitHub Org yet, our CI will not automatically run. Any member of the Ceph Org may comment "ok to test" to allow the Jenkins jobs to run.

[pdvian]

@swatithombe-blip Could you squash both commits into a single commit?

[Copilot]

Pull request overview

Updates the preflight “NIC Configuration” check to handle network interfaces with hyphens by normalizing interface names before looking up their speed facts, avoiding undefined-variable failures during report generation.

Changes:

• Normalize interface names (- → _) and prefix with ansible_ before extracting per-interface speed facts.
• Add a default speed value (-1) when the speed attribute is missing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[swatithombe-blip]

@swatithombe-blip Could you squash both commits into a single commit?

Yes, Sure !

[guits]

hello @akraitman

the CI is broken:

+ update_vagrant_boxes
++ vagrant box outdated --global
/tmp/jenkins6517480887680605581.sh: line 1164: vagrant: command not found

do we need to update something with the jenkins worker?

[ceph-jenkins]

Thank you for your contribution. Since you, the author, are not a member of the Ceph GitHub Org yet, our CI will not automatically run. Any member of the Ceph Org may comment "ok - to - test" (without the dashes) to allow the Jenkins jobs to run.

[asm0deuz]

jenkins test el9-functional

[asm0deuz]

jenkins test el10-functional

[asm0deuz]

jenkins test el9-functional

[asm0deuz]

@swatithombe-blip Hi,

Normalizing interface names before looking them up is the right fix.

ansible_facts['interfaces'] can show names with hyphens (e.g. br-ext) while per-interface facts are keyed with hyphens turned into underscores (e.g. br_ext).

I don't understand the reason why you add ansible_ and uses hostvars[inventory_hostname].ansible_facts.

I would rather use replace('-', '_'), keep map('extract', ansible_facts), and only add default for missing speed on virtual/bridge devices. For example, replace the “Speeds (Mbps)” part with something like:

' | Speeds (Mbps): ' ~
(network_interfaces | default([]) | map('replace', '-', '_')
| map('extract', ansible_facts) | map(attribute='speed', default=-1) | list | join(', '))},

Could you update the PR and re-check on a host with a hyphenated interface and a normal ens/eth host?

[akraitman]

hello @akraitman

the CI is broken:

+ update_vagrant_boxes
++ vagrant box outdated --global
/tmp/jenkins6517480887680605581.sh: line 1164: vagrant: command not found

do we need to update something with the jenkins worker?

Which builder is it ?

[guits]

hello @akraitman
the CI is broken:

+ update_vagrant_boxes
++ vagrant box outdated --global
/tmp/jenkins6517480887680605581.sh: line 1164: vagrant: command not found

do we need to update something with the jenkins worker?

Which builder is it ?

@akraitman https://jenkins.ceph.com/job/cephadm-ansible-prs-el9-functional/57/console braggi03
but it was 2 weeks ago, i'm not sure whether it was fixed since

[swatithombe-blip]

@swatithombe-blip Hi,

Normalizing interface names before looking them up is the right fix.

ansible_facts['interfaces'] can show names with hyphens (e.g. br-ext) while per-interface facts are keyed with hyphens turned into underscores (e.g. br_ext).

I don't understand the reason why you add ansible_ and uses hostvars[inventory_hostname].ansible_facts.

I would rather use replace('-', '_'), keep map('extract', ansible_facts), and only add default for missing speed on virtual/bridge devices. For example, replace the “Speeds (Mbps)” part with something like:

' | Speeds (Mbps): ' ~
(network_interfaces | default([]) | map('replace', '-', '_')
| map('extract', ansible_facts) | map(attribute='speed', default=-1) | list | join(', '))},

Could you update the PR and re-check on a host with a hyphenated interface and a normal ens/eth host?

@asm0deuz I've made changes and verified locally. It is not working as expected; however the previous changes was fixing the issue here is the reason.
Does not work because of how the extract filter operates in Ansible/Jinja2:

extract filter syntax issue: The extract filter expects the format extract(container, key) where it looks up container[key]. When you use map('extract', ansible_facts), it tries to use each interface name as a key to look up in ansible_facts, but:

ansible_facts['br-ext'] doesn't exist (the hyphenated version)
ansible_facts['br_ext'] is what exists (the underscore version)
Even after replacing hyphens with underscores, ansible_facts['br_ext'] returns the entire interface fact dictionary, not a direct lookup

---- Earlier code chnages working properly because, This works because:

It properly constructs the full fact name: ansible_br_ext
It accesses it through hostvars[inventory_hostname] which contains all the flattened ansible facts
The interface facts are stored as ansible_<interface_name> at the top level of hostvars
Normalizes interface names: br-ext → br_ext
Constructs proper fact key: ansible_br_ext
Accesses via hostvars to get the interface dictionary
Extracts speed with default fallback for virtual/bridge devices