[cifmw_setup] Retry nova discover_hosts to handle registration race - #4156
[cifmw_setup] Retry nova discover_hosts to handle registration race#4156sbauza wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
PR Summary by QodoRetry Nova host discovery until a compute host is mapped
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1. _nova_discover_hosts lacks namespace
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| -n {{ cifmw_openstack_namespace }} | ||
| nova-cell0-conductor-0 | ||
| nova-manage cell_v2 discover_hosts --verbose | ||
| register: _nova_discover_hosts |
There was a problem hiding this comment.
1. _nova_discover_hosts lacks namespace 📘 Rule violation ⚙ Maintainability
The new registered variable does not match the required cifmw_setup role namespace. This violates the mandated cifmw_<role_name>_<variable_name> structure.
Agent Prompt
## Issue description
The registered variable `_nova_discover_hosts` does not follow the required `cifmw_setup` role-variable naming pattern.
## Issue Context
Rename it consistently in both `register` and the retry condition, using a name such as `cifmw_setup_nova_discover_hosts`.
## Fix Focus Areas
- roles/cifmw_setup/tasks/deploy_architecture.yml[314-327]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| nova-manage cell_v2 discover_hosts --verbose | ||
| oc rsh -n {{ cifmw_openstack_namespace }} \ | ||
| nova-cell0-conductor-0 \ | ||
| nova-manage cell_v2 list_hosts | tr -d '\r' | \ | ||
| grep '^|' | grep -vc 'Cell Name' |
There was a problem hiding this comment.
2. Discovery task combines operations 📘 Rule violation ◔ Observability
One shell task performs host discovery, host listing, output normalization, and host counting without structured rescue diagnostics. A failure in any stage is reported only as a failure of the combined retry task, obscuring which operation failed.
Agent Prompt
## Issue description
The retry task combines multiple separable operations and lacks a structured rescue path that reports discovery and host-listing results before explicitly failing.
## Issue Context
Preserve the retry behavior while separating discovery from verification. Wrap the workflow in appropriate `block`/`rescue` handling, register relevant command results, report them in the rescue path, and terminate with `ansible.builtin.fail` after retries are exhausted.
## Fix Focus Areas
- roles/cifmw_setup/tasks/deploy_architecture.yml[308-327]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| nova-manage cell_v2 list_hosts | tr -d '\r' | \ | ||
| grep '^|' | grep -vc 'Cell Name' |
There was a problem hiding this comment.
3. Partial host discovery exits early 🐞 Bug ≡ Correctness
The retry succeeds as soon as list_hosts contains one host, so if one compute registers before the others, discovery stops and later computes remain unmapped. Supported multi-compute scenarios can therefore proceed with missing hypervisors despite this retry reporting success.
Agent Prompt
## Issue description
The discovery retry stops after finding any mapped host. In multi-compute deployments, one early registration can end the loop while slower compute services remain undiscovered.
## Issue Context
The repository supports deployments containing multiple computes, and the existing Nova wait hook compares discovered hosts against an expected compute count before stopping.
## Fix Focus Areas
- roles/cifmw_setup/tasks/deploy_architecture.yml[321-327]
- hooks/playbooks/nova_wait_for_compute_service.yml[82-87]
- scenarios/reproducers/va-multi.yml[100-124]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
so something like:
- name: Count expected compute hosts from the deployed nodesets
ansible.builtin.command: >-
oc get openstackdataplanenodeset
-n {{ cifmw_openstack_namespace }}
-o jsonpath={.items[*].spec.nodes.*.hostName}
register: _nova_expected_hosts
changed_when: false
So in the modified task:
until: >-
_nova_discover_hosts.rc == 0 and
_nova_discover_hosts.stdout | int >= _expected_compute_count
vars:
_expected_compute_count: "{{ _nova_expected_hosts.stdout.split() | length }}"
0181c22 to
fe0de4e
Compare
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
mrkisaolamb
left a comment
mrkisaolamb
left a comment
There was a problem hiding this comment.
Shouldn't we reuse this similar logic instead?
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 53m 11s |
fe0de4e to
2fad5c0
Compare
Good point, thanks for the pointer. The v2 adopts the same two-phase pattern from that hook: Wait for all expected nova-compute services to appear in the API (openstack compute service list) The reason I kept it inline in deploy_architecture.yml instead of wiring the existing hook is that the hook is opt-in — only scenarios that explicitly add it to their post_deploy or post_admin_setup hooks get the protection. The inline approach makes every deployment go through the wait+discover sequence, which is what we want since discover_hosts is already called unconditionally here. The first test run on testproject confirmed the gap between EDPM Ready and compute services registering can be ~15 min, so the original 5 min budget was way too short. The service-wait step (90 × 10s) handles that, and the discover_hosts retry (10 × 10s) only needs to cover the brief mapping delay once services are up. |
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 44m 06s |
2fad5c0 to
2ed3419
Compare
nova-manage cell_v2 discover_hosts is a point-in-time snapshot: it only maps compute hosts that have already registered with the message queue. When the command runs before all nova-compute services finish starting, some hosts are silently missed. The subsequent Tempest run then fails because the scheduler sees zero hypervisors. The gap between openstackdataplanedeployment Ready and nova-compute services actually registering can exceed 15 minutes, so a simple short retry on discover_hosts alone is insufficient. Adopt the same two-phase approach used by the nova_wait_for_compute_service hook but inline in deploy_architecture, so all deployments benefit without opting into a hook: 1. Wait for at least one nova-compute service to appear in the Nova API via the openstackclient pod (90 retries x 10 s = 15 min budget). 2. Run discover_hosts and verify that list_hosts reports at least as many mapped hosts as registered compute services (10 retries x 10 s, enough once services are registered). The service count from phase 1 is reused as the expected host count in phase 2, avoiding any dependency on nodeset CRs which include non-compute nodes (e.g. ceph-nodes). All retry values are hardcoded: they are a Nova-specific workaround in a generic role, and exposing them as variables would add unnecessary API surface. Resolves: OSPCIX-1493 Co-authored-by: Cursor <cursoragent@cursor.com>
2ed3419 to
38bc0ac
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 4h 25m 23s |
3 participants
Problem
nova-manage cell_v2 discover_hostsis a point-in-time snapshot: it onlymaps compute hosts that have already registered with the message queue.
When the command runs before all nova-compute services finish starting,
some hosts are silently missed. The subsequent Tempest run then fails
because the scheduler sees zero hypervisors.
This race condition has been consistently hitting the
uni04deltajobsince around Aug 27, causing widespread Tempest failures
(NoValidHost / 0 hypervisors).
Fix
Replace the single fire-and-forget
discover_hostscommand with a retryloop that:
discover_hosts --verbose.list_hostsand counts mapped hosts (excluding the header).at least one host appears.
The retry values are hardcoded rather than exposed as role variables
because this is a Nova-specific workaround in a generic role; making
them configurable would add unnecessary API surface. 30 × 10s gives
a 5-minute window, which is generous for compute services to register
while keeping CI wall-clock impact minimal on healthy runs (the loop
exits on the first success).
Resolves: OSPCIX-1493
Assisted-By: Cursor Claude 4.6 Opus
Made with Cursor