engine: key removeNic work job lookup on nic uuid to fix concurrent removal race

[Andr0human]

Description

This PR makes the NIC uuid part of the pending work-job lookup key in VirtualMachineManagerImpl.removeNicFromVmThroughJobQueue, so that two concurrent removeNicFromVirtualMachine requests for different NICs on the same VM no longer collapse into a single work job.

Before this change the lookup was NIC-agnostic — retrievePendingWorkJob(vmId, commandName), i.e. the 3-arg VmWorkJobDao.listPendingWorkJobs(type, vmId, cmd). If a second remove-NIC request arrived while the first VmWorkRemoveNicFromVm job was still pending, it matched that job and joined it instead of submitting its own. The job removes only the NIC it was created for, but both callers wait on the same job id and both receive its success — so the second NIC stays attached while its API call reports success.

Functional change in behaviour:

• Before: concurrent removal of NIC-A and NIC-B produces one work job. NIC-A is removed; NIC-B is silently left attached; both callers are told SUCCEEDED.
• After: each request gets its own work job, keyed on its own NIC uuid. Both NICs are removed and both callers are told the truth. Genuine duplicates — two requests for the same NIC — still dedup exactly as before.

This is the mirror of an already-merged fix on the add path. addVmToNetworkThroughJobQueue was fixed in #5658 (issue #5541, 4.16.1.0) by keying on the object uuid; the symmetric remove path never got the same treatment. The change here follows that established pattern line for line:

• look up pending jobs with the 4-arg listPendingWorkJobs(Instance, vmId, VmWorkRemoveNicFromVm.class.getName(), nic.getUuid());
• if more than one pending job matches, fail fast with a CloudRuntimeException (same guard as the add path);
• stamp newly created jobs with workJob.setSecondaryObjectIdentifier(nic.getUuid()) before submitting.

Nic extends Identity, so getUuid() is available — no interface or schema change is required.

Scope is deliberately tight: only removeNicFromVmThroughJobQueue is touched. The sibling removeVmFromNetworkThroughJobQueue still uses the network-agnostic retrievePendingWorkJob(vmId, commandName) lookup and appears to have the same class of race; that is left out of this PR and noted in the issue as a follow-up.

Fixes: #13699

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• Build/CI
• Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

N/A — management-server logic change, no UI surface.

How Has This Been Tested?

1. Unit tests — three regression tests added to VirtualMachineManagerImplTest

test	asserts
removeNicFromVmThroughJobQueueDoesNotJoinAnotherNicsPendingJob	with another NIC's job already pending, a new job is created, stamped with this NIC's uuid, submitted and joined
removeNicFromVmThroughJobQueueJoinsExistingJobForSameNic	per-NIC dedup still works — an existing same-NIC job is joined, nothing new is submitted
removeNicFromVmThroughJobQueueThrowsWhenMultiplePendingJobsForSameNic	the size() > 1 guard throws

mvn -pl engine/orchestration test -Dtest=VirtualMachineManagerImplTest
Tests run: 96, Failures: 0, Errors: 0, Skipped: 0

Red→green was confirmed by reverting the production change and re-running against this branch: Tests run: 96, Failures: 1, Errors: 2 — all three new tests and only those, with the regression test failing at the setCmdInfoAndSubmitAsyncJob verification, i.e. precisely because the second request joined the first NIC's job instead of submitting its own. checkstyle:check on the module reports 0 violations.

2. End-to-end on the CloudStack simulator

Advanced-zone simulator (mvn -Pdeveloper -Dsimulator, tools/docker), VM with three NICs, two of them removed concurrently. UnPlugNicCommand was stalled with configureSimulator name=UnPlugNicCommand value=wait:20000 so the first work job is guaranteed to still be pending when the second request arrives — that is the window the bug lives in. Both halves were built through the same two-module pipeline (-pl engine/orchestration then -pl client), so the only difference between the runs is the fix itself. (This end-to-end run was done on a simulator build of main; the diff proposed here is identical on both branches, and the unit tests above were run on this 4.22 branch.)

	before (unfixed)	after (this PR)
vm_work_job rows for VmWorkRemoveNicFromVm	1	2
secondary_object (the per-NIC key)	NULL	two distinct NIC uuids
call A	SUCCEEDED — NIC removed	SUCCEEDED — NIC removed
call B	SUCCEEDED — NIC still attached	SUCCEEDED — NIC removed
NICs remaining on the VM	2 (default + orphan)	1 (default)

The unfixed run reports the contradiction directly — the caller is told the removal succeeded while listVirtualMachines still shows the NIC attached:

call B: asked to remove nic 6a005f6b-f852-4231-8dc7-938f9b7c1800
    jobstatus     = 1 (SUCCEEDED)
    jobresultcode = 0
    nics on the returned VM = ['98179dff-...-d08ea0dc6014', '6a005f6b-...-938f9b7c1800']
    *** caller reported SUCCEEDED but the nic is STILL ATTACHED ***

and the same query after the fix shows the two per-NIC jobs the dedup key now produces:

+----+----------------+--------------------------------------+-----------------------+------------+
| id | vm_instance_id | secondary_object                     | cmd                   | job_status |
+----+----------------+--------------------------------------+-----------------------+------------+
| 70 |             19 | fa062278-26e3-4286-a534-cdb4ebd994c7 | VmWorkRemoveNicFromVm |          1 |
| 72 |             19 | a49f67b8-a9b9-4e4b-bb10-dc7d048a9b11 | VmWorkRemoveNicFromVm |          1 |
+----+----------------+--------------------------------------+-----------------------+------------+

How did you try to break this feature and the system with this change?

• Genuine duplicate requests. Two removals of the same NIC still collapse into one job — the narrower key must not disable dedup, only scope it. Covered by removeNicFromVmThroughJobQueueJoinsExistingJobForSameNic.
• More than one pending job for the same NIC. Should never happen, but the add path treats it as a hard error rather than silently picking one, so this does too. Covered by removeNicFromVmThroughJobQueueThrowsWhenMultiplePendingJobsForSameNic.
• Sequential (non-concurrent) removals. Unaffected: with no pending job the lookup returns empty and a new job is created exactly as before.
• Persistence of the new key. secondary_object is an existing vm_work_job column already written by the add path since remove VmWorkJob after adding a nic to a vm #5658 — no schema change, and it is confirmed populated in the simulator run above.
• Blast radius. setSecondaryObjectIdentifier is only ever set here for jobs whose cmd is VmWorkRemoveNicFromVm, and the 4-arg listPendingWorkJobs overload already exists and is already used by addVmToNetworkThroughJobQueue. No other work-job path changes behaviour; the full VirtualMachineManagerImplTest suite (96 tests) passes.

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:

• In case of a new feature add useful documentation (raise doc PR at https://github.com/apache/cloudstack-documentation)
• Be patient and persistent. It might take some time to get a review or get the final approval from the committers.
• Pay attention to the quality of your code, ensure tests are passing and your PR doesn't have conflicts.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Issues, Mailing list and Slack.
• Be sure to read the CloudStack Coding Conventions.
  Apache CloudStack is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@cloudstack.apache.org (https://cloudstack.apache.org/mailing-lists.html)
  Slack: https://apachecloudstack.slack.com/