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

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -5938,14 +5938,25 @@ public Outcome<VirtualMachine> removeNicFromVmThroughJobQueue(
             final VirtualMachine vm, final Nic nic) {
         Long vmId = vm.getId();
         String commandName = VmWorkRemoveNicFromVm.class.getName();
-        Pair<VmWorkJobVO, Long> pendingWorkJob = retrievePendingWorkJob(vmId, commandName);
 
-        VmWorkJobVO workJob = pendingWorkJob.first();
+        // The nic uuid must be part of the pending-job lookup key. Without it, a concurrent request
+        // to remove a different nic from the same vm matches this still-pending job and joins it
+        // instead of submitting its own, so only one nic is removed while both callers wait on the
+        // single job and both receive its success. Mirrors the symmetric addVmToNetworkThroughJobQueue.
+        final List<VmWorkJobVO> pendingWorkJobs = _workJobDao.listPendingWorkJobs(
+                VirtualMachine.Type.Instance, vmId, commandName, nic.getUuid());
 
-        if (workJob == null) {
+        VmWorkJobVO workJob;
+        if (pendingWorkJobs != null && pendingWorkJobs.size() > 0) {
+            if (pendingWorkJobs.size() > 1) {
+                throw new CloudRuntimeException(String.format("The number of jobs to remove nic %s from vm %s are %d", nic.getUuid(), vm.getInstanceName(), pendingWorkJobs.size()));
+            }
+            workJob = pendingWorkJobs.get(0);
+        } else {
             Pair<VmWorkJobVO, VmWork> newVmWorkJobAndInfo = createWorkJobAndWorkInfo(commandName, vmId);
 
             workJob = newVmWorkJobAndInfo.first();
+            workJob.setSecondaryObjectIdentifier(nic.getUuid());
             VmWorkRemoveNicFromVm workInfo = new VmWorkRemoveNicFromVm(newVmWorkJobAndInfo.second(), nic.getId());
 
             setCmdInfoAndSubmitAsyncJob(workJob, workInfo, vmId);

engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java

@@ -70,6 +70,7 @@
 import org.apache.cloudstack.framework.extensions.dao.ExtensionDetailsDao;
 import org.apache.cloudstack.framework.extensions.manager.ExtensionsManager;
 import org.apache.cloudstack.framework.extensions.vo.ExtensionDetailsVO;
+import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext;
 import org.apache.cloudstack.framework.jobs.dao.VmWorkJobDao;
 import org.apache.cloudstack.framework.jobs.impl.VmWorkJobVO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
@@ -1865,6 +1866,97 @@ public void testUnmanagePendingWorkJobs() {
         virtualMachineManagerImpl.unmanage(vmMockUuid, null);
     }
 
+    /**
+     * A pending remove-nic job for a different nic on the same vm must not swallow the removal of
+     * this nic: the pending-job lookup has to be keyed on the nic uuid so that a new job is
+     * submitted for this nic instead of joining the other nic's job. Regression test for the
+     * concurrent-removeNic collapse.
+     */
+    @Test
+    public void removeNicFromVmThroughJobQueueDoesNotJoinAnotherNicsPendingJob() {
+        String commandName = VmWorkRemoveNicFromVm.class.getName();
+        String nicUuid = UUID.randomUUID().toString();
+
+        Nic nic = mock(Nic.class);
+        when(nic.getId()).thenReturn(42L);
+        when(nic.getUuid()).thenReturn(nicUuid);
+
+        // A pending remove-nic job exists for the vm (e.g. for another nic); the nic-agnostic
+        // lookup would return it, but the nic-keyed lookup finds nothing for this nic. This stub is
+        // lenient because the fixed code never consults the nic-agnostic 3-arg lookup - only the
+        // buggy code does, where this job is what it wrongly joins.
+        Mockito.lenient().when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId, commandName))
+                .thenReturn(Collections.singletonList(mock(VmWorkJobVO.class)));
+        when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId, commandName, nicUuid))
+                .thenReturn(Collections.emptyList());
+
+        VmWorkJobVO newJob = mock(VmWorkJobVO.class);
+        when(newJob.getId()).thenReturn(100L);
+        doReturn(new Pair<VmWorkJobVO, VmWork>(newJob, mock(VmWork.class)))
+                .when(virtualMachineManagerImpl).createWorkJobAndWorkInfo(commandName, vmInstanceVoMockId);
+        doNothing().when(virtualMachineManagerImpl).setCmdInfoAndSubmitAsyncJob(any(), any(), anyLong());
+
+        AsyncJobExecutionContext execContext = mock(AsyncJobExecutionContext.class);
+        try (MockedStatic<AsyncJobExecutionContext> ignored = Mockito.mockStatic(AsyncJobExecutionContext.class)) {
+            when(AsyncJobExecutionContext.getCurrentExecutionContext()).thenReturn(execContext);
+
+            virtualMachineManagerImpl.removeNicFromVmThroughJobQueue(vmInstanceMock, nic);
+
+            // A new job must be created for this nic, stamped with the nic uuid, submitted and joined.
+            verify(virtualMachineManagerImpl, times(1)).createWorkJobAndWorkInfo(commandName, vmInstanceVoMockId);
+            verify(newJob, times(1)).setSecondaryObjectIdentifier(nicUuid);
+            verify(virtualMachineManagerImpl, times(1)).setCmdInfoAndSubmitAsyncJob(eq(newJob), any(), eq(vmInstanceVoMockId));
+            verify(execContext, times(1)).joinJob(100L);
+        }
+    }
+
+    /**
+     * When a pending remove-nic job already exists for this same nic, the request must join it
+     * rather than submit a duplicate: per-nic deduplication still holds.
+     */
+    @Test
+    public void removeNicFromVmThroughJobQueueJoinsExistingJobForSameNic() {
+        String commandName = VmWorkRemoveNicFromVm.class.getName();
+        String nicUuid = UUID.randomUUID().toString();
+
+        Nic nic = mock(Nic.class);
+        when(nic.getUuid()).thenReturn(nicUuid);
+
+        VmWorkJobVO existingJob = mock(VmWorkJobVO.class);
+        when(existingJob.getId()).thenReturn(77L);
+        when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId, commandName, nicUuid))
+                .thenReturn(Collections.singletonList(existingJob));
+
+        AsyncJobExecutionContext execContext = mock(AsyncJobExecutionContext.class);
+        try (MockedStatic<AsyncJobExecutionContext> ignored = Mockito.mockStatic(AsyncJobExecutionContext.class)) {
+            when(AsyncJobExecutionContext.getCurrentExecutionContext()).thenReturn(execContext);
+
+            virtualMachineManagerImpl.removeNicFromVmThroughJobQueue(vmInstanceMock, nic);
+
+            verify(virtualMachineManagerImpl, never()).createWorkJobAndWorkInfo(anyString(), anyLong());
+            verify(virtualMachineManagerImpl, never()).setCmdInfoAndSubmitAsyncJob(any(), any(), anyLong());
+            verify(execContext, times(1)).joinJob(77L);
+        }
+    }
+
+    /**
+     * More than one pending remove-nic job for the same nic is an inconsistent state and must fail
+     * fast rather than pick one arbitrarily. Mirrors the guard in addVmToNetworkThroughJobQueue.
+     */
+    @Test(expected = CloudRuntimeException.class)
+    public void removeNicFromVmThroughJobQueueThrowsWhenMultiplePendingJobsForSameNic() {
+        String commandName = VmWorkRemoveNicFromVm.class.getName();
+        String nicUuid = UUID.randomUUID().toString();
+
+        Nic nic = mock(Nic.class);
+        when(nic.getUuid()).thenReturn(nicUuid);
+
+        when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId, commandName, nicUuid))
+                .thenReturn(Arrays.asList(mock(VmWorkJobVO.class), mock(VmWorkJobVO.class)));
+
+        virtualMachineManagerImpl.removeNicFromVmThroughJobQueue(vmInstanceMock, nic);
+    }
+
     @Test
     public void testUnmanageHostNotFoundAfterTransaction() {
         when(vmInstanceMock.getHostId()).thenReturn(hostMockId);