[update] Fix floating IP leak in DPDK workload cleanup

[mnietoji]

When workload_dpdk is true, the sanity_teardown function only deletes the DPDK port but does not clean up the floating IP created by set_vm_ip. The VM is deleted (which disassociates the FIP) but the floating IP remains allocated, leaking one IP per update run.

Over multiple runs this exhausts the access network's IP pool, causing subsequent tempest tests to fail with
IpAddressGenerationFailure: No more IP addresses available.

Add floating IP cleanup to the DPDK branch of sanity_teardown, matching the existing cleanup logic in the default (non-SRIOV, non-DPDK) branch.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ciecierski for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• roles/update/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[qodo-code-review]

PR Summary by Qodo

Fix floating IP leak during DPDK workload teardown

🐞 Bug fix 🕐 Less than 10 minutes

AI Description

• Add floating IP removal/deletion to the DPDK teardown path.
• Prevent access network IP pool exhaustion across repeated update runs.
• Align DPDK cleanup behavior with the existing default teardown logic.

Diagram

graph TD
  A["update workload script"] --> B["sanity_teardown()"] --> C{DPDK workload?}
  C -->|Yes| D["delete DPDK port"] --> E{"INSTANCE_FIP set?"}
  E -->|Yes| F["remove FIP from VM"] --> G["delete floating IP"] --> H[("OpenStack: Neutron")]
  E -->|No| I["skip FIP cleanup"]
  C -->|No| J["existing non-DPDK cleanup"]

  subgraph Legend
    direction LR
    _proc["Script/Function"] ~~~ _dec{"Decision"} ~~~ _cloud[("OpenStack API")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Track created FIPs via tags/description and garbage-collect

• ➕ Recovers from partial failures where INSTANCE_FIP isn’t persisted
• ➕ Can clean up leftovers from earlier buggy runs automatically
• ➖ Requires additional conventions (tags/description) and discovery queries
• ➖ Higher complexity and risk of deleting the wrong resource if tagging is inconsistent

2. Centralize cleanup with a trap/EXIT handler

• ➕ Ensures cleanup runs on early exits/errors, reducing leaks further
• ➕ Keeps teardown logic unified instead of per-branch duplication
• ➖ More refactor-heavy; increases review surface area
• ➖ Must carefully order cleanup to avoid breaking other teardown paths

Recommendation: The PR’s approach is the right minimal fix: it mirrors existing non-DPDK cleanup and closes the specific leak by deleting the allocated floating IP. Consider a follow-up to add defensive garbage-collection or an EXIT trap if teardown interruptions are common, but that’s not necessary for this targeted bug fix.

Files changed (1) +6 / -0

Bug fix (1) +6 / -0

workload_launch.sh.j2Delete allocated floating IP during DPDK sanity teardown +6/-0

Delete allocated floating IP during DPDK sanity teardown

• Extend the DPDK branch of sanity_teardown to remove the floating IP from the instance and delete the floating IP allocation when INSTANCE_FIP is set. This prevents floating IP leaks across repeated update runs that can exhaust the access network IP pool.

roles/update/templates/workload_launch.sh.j2

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Stale vm_info.sh remains 🐞 Bug ☼ Reliability

Description

In the DPDK teardown path, the floating IP is removed and deleted but the persisted
${INSTANCE_FILE} (vm_info.sh) is not removed, unlike the default IPv4 teardown. This can allow
later connectivity checks to source an old VM_IP and run against a deallocated/reused address,
producing misleading results across runs.

Code

roles/update/templates/workload_launch.sh.j2[R222-226]

+    if [ -n "${INSTANCE_FIP}" ]; then
+        echo "Remove ${INSTANCE_FIP} from ${INSTANCE_NAME}"
+        openstack server remove floating ip ${INSTANCE_NAME} ${INSTANCE_FIP} || echo "Warning: Failed to remove floating IP from instance"
+        echo "Delete floating ip ${INSTANCE_FIP}"
+        openstack floating ip delete ${INSTANCE_FIP} || echo "Warning: Failed to delete floating IP ${INSTANCE_FIP}"

Evidence

The PR adds FIP cleanup for the DPDK path but does not remove ${INSTANCE_FILE}. The default IPv4
teardown explicitly removes ${INSTANCE_FILE} when it contains the floating IP, and the ping-start
script relies on this file to obtain VM_IP, so leaving it behind can cause subsequent checks to
use stale values.

roles/update/templates/workload_launch.sh.j2[222-227]
roles/update/templates/workload_launch.sh.j2[242-251]
roles/update/templates/workload_launch.sh.j2[596-599]
roles/update/templates/l3_agent_start_ping.sh.j2[6-14]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The DPDK branch of `sanity_teardown` deletes the floating IP but leaves `${INSTANCE_FILE}` (vm_info.sh) behind. This differs from the default IPv4 branch, which removes `${INSTANCE_FILE}` when it contains the floating IP, preventing stale state from being reused.

### Issue Context
`vm_info.sh` is later sourced by the L3 connectivity ping start script to obtain `VM_IP`. If a workload launch fails early (or teardown runs) but the file persists, a subsequent run can mistakenly ping an old IP.

### Fix Focus Areas
- roles/update/templates/workload_launch.sh.j2[222-227]
- roles/update/templates/workload_launch.sh.j2[242-251]
- roles/update/templates/workload_launch.sh.j2[596-599]
- roles/update/templates/l3_agent_start_ping.sh.j2[6-14]

### Suggested change
In the DPDK teardown block, mirror the default IPv4 logic:
- After removing the floating IP from the server, if `${INSTANCE_FILE}` exists and contains `${INSTANCE_FIP}`, remove `${INSTANCE_FILE}`.
- (Optional hardening) quote `${INSTANCE_NAME}` and `${INSTANCE_FIP}` in the `openstack` commands for consistency/safety.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[evallesp]

/lgtm