This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.16
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.16 by this push:
new 028d338 remove VmWorkJob after adding a nic to a vm (#5658)
028d338 is described below
commit 028d338aaa3587d9caf2b0d83f4b030ac09b4871
Author: dahn <[email protected]>
AuthorDate: Mon Jan 10 16:31:50 2022 +0100
remove VmWorkJob after adding a nic to a vm (#5658)
Co-authored-by: Daan Hoogland <[email protected]>
Co-authored-by: Suresh Kumar Anaparti <[email protected]>
Co-authored-by: Wei Zhou <[email protected]>
---
.../com/cloud/vm/VirtualMachineManagerImpl.java | 89 ++++++++++++++++------
.../resources/META-INF/db/schema-41600to41610.sql | 4 +-
.../framework/jobs/dao/VmWorkJobDao.java | 2 +
.../framework/jobs/dao/VmWorkJobDaoImpl.java | 14 ++++
.../cloudstack/framework/jobs/impl/AsyncJobVO.java | 2 +-
.../framework/jobs/impl/VmWorkJobVO.java | 24 ++++++
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 21 +++--
7 files changed, 124 insertions(+), 32 deletions(-)
diff --git
a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
index 42cdb88..706b665 100755
---
a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
+++
b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -42,6 +42,7 @@ import java.util.concurrent.TimeUnit;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import javax.persistence.EntityExistsException;
import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
import org.apache.cloudstack.annotation.AnnotationService;
@@ -191,7 +192,6 @@ import com.cloud.offering.NetworkOffering;
import com.cloud.offering.ServiceOffering;
import com.cloud.offerings.NetworkOfferingVO;
import com.cloud.offerings.dao.NetworkOfferingDao;
-import com.cloud.offerings.dao.NetworkOfferingDetailsDao;
import com.cloud.org.Cluster;
import com.cloud.resource.ResourceManager;
import com.cloud.resource.ResourceState;
@@ -357,8 +357,6 @@ public class VirtualMachineManagerImpl extends ManagerBase
implements VirtualMac
@Inject
private StorageManager storageMgr;
@Inject
- private NetworkOfferingDetailsDao networkOfferingDetailsDao;
- @Inject
private NetworkDetailsDao networkDetailsDao;
@Inject
private SecurityGroupManager _securityGroupManager;
@@ -4002,9 +4000,7 @@ public class VirtualMachineManagerImpl extends
ManagerBase implements VirtualMac
final AsyncJobExecutionContext jobContext =
AsyncJobExecutionContext.getCurrentExecutionContext();
if
(jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {
- // avoid re-entrance
- VmWorkJobVO placeHolder = null;
- placeHolder = createPlaceHolderWork(vm.getId());
+ VmWorkJobVO placeHolder = createPlaceHolderWork(vm.getId(),
network.getUuid());
try {
return orchestrateAddVmToNetwork(vm, network, requested);
} finally {
@@ -4040,7 +4036,19 @@ public class VirtualMachineManagerImpl extends
ManagerBase implements VirtualMac
}
}
- throw new RuntimeException("Unexpected job execution result");
+ throw new RuntimeException("null job execution result");
+ }
+ }
+
+ /**
+ * duplicated in {@see UserVmManagerImpl} for a {@see UserVmVO}
+ */
+ private void checkIfNetworkExistsForVM(VirtualMachine virtualMachine,
Network network) {
+ List<NicVO> allNics = _nicsDao.listByVmId(virtualMachine.getId());
+ for (NicVO nic : allNics) {
+ if (nic.getNetworkId() == network.getId()) {
+ throw new CloudRuntimeException("A NIC already exists for VM:"
+ virtualMachine.getInstanceName() + " in network: " + network.getUuid());
+ }
}
}
@@ -4048,6 +4056,7 @@ public class VirtualMachineManagerImpl extends
ManagerBase implements VirtualMac
InsufficientCapacityException {
final CallContext cctx = CallContext.current();
+ checkIfNetworkExistsForVM(vm, network);
s_logger.debug("Adding vm " + vm + " to network " + network + ";
requested nic profile " + requested);
final VMInstanceVO vmVO = _vmDao.findById(vm.getId());
final ReservationContext context = new ReservationContextImpl(null,
null, cctx.getCallingUser(), cctx.getCallingAccount());
@@ -5631,35 +5640,62 @@ public class VirtualMachineManagerImpl extends
ManagerBase implements VirtualMac
final List<VmWorkJobVO> pendingWorkJobs =
_workJobDao.listPendingWorkJobs(
VirtualMachine.Type.Instance, vm.getId(),
- VmWorkAddVmToNetwork.class.getName());
+ VmWorkAddVmToNetwork.class.getName(), network.getUuid());
VmWorkJobVO workJob = null;
if (pendingWorkJobs != null && pendingWorkJobs.size() > 0) {
- assert pendingWorkJobs.size() == 1;
+ if (pendingWorkJobs.size() > 1) {
+ throw new CloudRuntimeException(String.format("The number of
jobs to add network %s to vm %s are %d", network.getUuid(),
vm.getInstanceName(), pendingWorkJobs.size()));
+ }
workJob = pendingWorkJobs.get(0);
} else {
+ if (s_logger.isTraceEnabled()) {
+ s_logger.trace(String.format("no jobs to add network %s for vm
%s yet", network, vm));
+ }
- workJob = new VmWorkJobVO(context.getContextId());
+ workJob = createVmWorkJobToAddNetwork(vm, network, requested,
context, user, account);
+ }
+
AsyncJobExecutionContext.getCurrentExecutionContext().joinJob(workJob.getId());
- workJob.setDispatcher(VmWorkConstants.VM_WORK_JOB_DISPATCHER);
- workJob.setCmd(VmWorkAddVmToNetwork.class.getName());
+ return new VmJobVirtualMachineOutcome(workJob, vm.getId());
+ }
- workJob.setAccountId(account.getId());
- workJob.setUserId(user.getId());
- workJob.setVmType(VirtualMachine.Type.Instance);
- workJob.setVmInstanceId(vm.getId());
- workJob.setRelated(AsyncJobExecutionContext.getOriginJobId());
+ private VmWorkJobVO createVmWorkJobToAddNetwork(
+ VirtualMachine vm,
+ Network network,
+ NicProfile requested,
+ CallContext context,
+ User user,
+ Account account) {
+ VmWorkJobVO workJob;
+ workJob = new VmWorkJobVO(context.getContextId());
- // save work context info (there are some duplications)
- final VmWorkAddVmToNetwork workInfo = new
VmWorkAddVmToNetwork(user.getId(), account.getId(), vm.getId(),
- VirtualMachineManagerImpl.VM_WORK_JOB_HANDLER,
network.getId(), requested);
- workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo));
+ workJob.setDispatcher(VmWorkConstants.VM_WORK_JOB_DISPATCHER);
+ workJob.setCmd(VmWorkAddVmToNetwork.class.getName());
+
+ workJob.setAccountId(account.getId());
+ workJob.setUserId(user.getId());
+ workJob.setVmType(VirtualMachine.Type.Instance);
+ workJob.setVmInstanceId(vm.getId());
+ workJob.setRelated(AsyncJobExecutionContext.getOriginJobId());
+ workJob.setSecondaryObjectIdentifier(network.getUuid());
+
+ // save work context info as there might be some duplicates
+ final VmWorkAddVmToNetwork workInfo = new
VmWorkAddVmToNetwork(user.getId(), account.getId(), vm.getId(),
+ VirtualMachineManagerImpl.VM_WORK_JOB_HANDLER,
network.getId(), requested);
+ workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo));
+ try {
_jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE,
vm.getId());
+ } catch (CloudRuntimeException e) {
+ if (e.getCause() instanceof EntityExistsException) {
+ String msg = String.format("A job to add a nic for network %s
to vm %s already exists", network.getUuid(), vm.getUuid());
+ s_logger.warn(msg, e);
+ }
+ throw e;
}
-
AsyncJobExecutionContext.getCurrentExecutionContext().joinJob(workJob.getId());
- return new VmJobVirtualMachineOutcome(workJob, vm.getId());
+ return workJob;
}
public Outcome<VirtualMachine> removeNicFromVmThroughJobQueue(
@@ -5968,6 +6004,10 @@ public class VirtualMachineManagerImpl extends
ManagerBase implements VirtualMac
}
private VmWorkJobVO createPlaceHolderWork(final long instanceId) {
+ return createPlaceHolderWork(instanceId, null);
+ }
+
+ private VmWorkJobVO createPlaceHolderWork(final long instanceId, String
secondaryObjectIdentifier) {
final VmWorkJobVO workJob = new VmWorkJobVO("");
workJob.setDispatcher(VmWorkConstants.VM_WORK_JOB_PLACEHOLDER);
@@ -5979,6 +6019,9 @@ public class VirtualMachineManagerImpl extends
ManagerBase implements VirtualMac
workJob.setStep(VmWorkJobVO.Step.Starting);
workJob.setVmType(VirtualMachine.Type.Instance);
workJob.setVmInstanceId(instanceId);
+
if(org.apache.commons.lang3.StringUtils.isNotBlank(secondaryObjectIdentifier)) {
+ workJob.setSecondaryObjectIdentifier(secondaryObjectIdentifier);
+ }
workJob.setInitMsid(ManagementServerNode.getManagementServerId());
_workJobDao.persist(workJob);
diff --git
a/engine/schema/src/main/resources/META-INF/db/schema-41600to41610.sql
b/engine/schema/src/main/resources/META-INF/db/schema-41600to41610.sql
index 24c5b79..04bdbca 100644
--- a/engine/schema/src/main/resources/META-INF/db/schema-41600to41610.sql
+++ b/engine/schema/src/main/resources/META-INF/db/schema-41600to41610.sql
@@ -17,4 +17,6 @@
--;
-- Schema upgrade from 4.16.0.0 to 4.16.1.0
---;
\ No newline at end of file
+--;
+
+ALTER TABLE `cloud`.`vm_work_job` ADD COLUMN `secondary_object` char(100)
COMMENT 'any additional item that must be checked during queueing' AFTER
`vm_instance_id`;
diff --git
a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDao.java
b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDao.java
index 44e39e4..89601e6 100644
---
a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDao.java
+++
b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDao.java
@@ -32,6 +32,8 @@ public interface VmWorkJobDao extends GenericDao<VmWorkJobVO,
Long> {
List<VmWorkJobVO> listPendingWorkJobs(VirtualMachine.Type type, long
instanceId, String jobCmd);
+ List<VmWorkJobVO> listPendingWorkJobs(VirtualMachine.Type type, long
instanceId, String jobCmd, String secondaryObjectIdentifier);
+
void updateStep(long workJobId, Step step);
void expungeCompletedWorkJobs(Date cutDate);
diff --git
a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDaoImpl.java
b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDaoImpl.java
index e81ab1e..4a10727 100644
---
a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDaoImpl.java
+++
b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/VmWorkJobDaoImpl.java
@@ -67,6 +67,7 @@ public class VmWorkJobDaoImpl extends
GenericDaoBase<VmWorkJobVO, Long> implemen
PendingWorkJobByCommandSearch.and("jobStatus",
PendingWorkJobByCommandSearch.entity().getStatus(), Op.EQ);
PendingWorkJobByCommandSearch.and("vmType",
PendingWorkJobByCommandSearch.entity().getVmType(), Op.EQ);
PendingWorkJobByCommandSearch.and("vmInstanceId",
PendingWorkJobByCommandSearch.entity().getVmInstanceId(), Op.EQ);
+ PendingWorkJobByCommandSearch.and("secondaryObjectIdentifier",
PendingWorkJobByCommandSearch.entity().getSecondaryObjectIdentifier(), Op.EQ);
PendingWorkJobByCommandSearch.and("step",
PendingWorkJobByCommandSearch.entity().getStep(), Op.NEQ);
PendingWorkJobByCommandSearch.and("cmd",
PendingWorkJobByCommandSearch.entity().getCmd(), Op.EQ);
PendingWorkJobByCommandSearch.done();
@@ -120,6 +121,19 @@ public class VmWorkJobDaoImpl extends
GenericDaoBase<VmWorkJobVO, Long> implemen
}
@Override
+ public List<VmWorkJobVO> listPendingWorkJobs(VirtualMachine.Type type,
long instanceId, String jobCmd, String secondaryObjectIdentifier) {
+ SearchCriteria<VmWorkJobVO> sc =
PendingWorkJobByCommandSearch.create();
+ sc.setParameters("jobStatus", JobInfo.Status.IN_PROGRESS);
+ sc.setParameters("vmType", type);
+ sc.setParameters("vmInstanceId", instanceId);
+ sc.setParameters("secondaryObjectIdentifier",
secondaryObjectIdentifier);
+ sc.setParameters("cmd", jobCmd);
+
+ Filter filter = new Filter(VmWorkJobVO.class, "created", true, null,
null);
+ return this.listBy(sc, filter);
+ }
+
+ @Override
public void updateStep(long workJobId, Step step) {
VmWorkJobVO jobVo = findById(workJobId);
jobVo.setStep(step);
diff --git
a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java
b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java
index 9d30c2c..8f3c033 100644
---
a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java
+++
b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobVO.java
@@ -384,7 +384,7 @@ public class AsyncJobVO implements AsyncJob, JobInfo {
@Override
public String toString() {
StringBuffer sb = new StringBuffer();
- sb.append("AsyncJobVO {id:").append(getId());
+ sb.append("AsyncJobVO: {id:").append(getId());
sb.append(", userId: ").append(getUserId());
sb.append(", accountId: ").append(getAccountId());
sb.append(", instanceType: ").append(getInstanceType());
diff --git
a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/VmWorkJobVO.java
b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/VmWorkJobVO.java
index ef0ac7d..41eaac5 100644
---
a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/VmWorkJobVO.java
+++
b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/VmWorkJobVO.java
@@ -58,6 +58,9 @@ public class VmWorkJobVO extends AsyncJobVO {
@Column(name = "vm_instance_id")
long vmInstanceId;
+ @Column(name = "secondary_object")
+ String secondaryObjectIdentifier;
+
protected VmWorkJobVO() {
}
@@ -89,4 +92,25 @@ public class VmWorkJobVO extends AsyncJobVO {
public void setVmInstanceId(long vmInstanceId) {
this.vmInstanceId = vmInstanceId;
}
+
+ public String getSecondaryObjectIdentifier() {
+ return secondaryObjectIdentifier;
+ }
+
+ public void setSecondaryObjectIdentifier(String secondaryObjectIdentifier)
{
+ this.secondaryObjectIdentifier = secondaryObjectIdentifier;
+ }
+
+ @Override
+ public String toString() {
+ StringBuffer sb = new StringBuffer();
+ sb.append("VmWorkJobVO : {").
+ append(", step: ").append(getStep()).
+ append(", vmType: ").append(getVmType()).
+ append(", vmInstanceId: ").append(getVmInstanceId()).
+ append(", secondaryObjectIdentifier:
").append(getSecondaryObjectIdentifier()).
+ append(super.toString()).
+ append("}");
+ return sb.toString();
+ }
}
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index c152546..28ee655 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -1386,12 +1386,7 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
Account vmOwner = _accountMgr.getAccount(vmInstance.getAccountId());
_networkModel.checkNetworkPermissions(vmOwner, network);
- List<NicVO> allNics = _nicDao.listByVmId(vmInstance.getId());
- for (NicVO nic : allNics) {
- if (nic.getNetworkId() == network.getId()) {
- throw new CloudRuntimeException("A NIC already exists for VM:"
+ vmInstance.getInstanceName() + " in network: " + network.getUuid());
- }
- }
+ checkIfNetExistsForVM(vmInstance, network);
macAddress = validateOrReplaceMacAddress(macAddress, network.getId());
@@ -1458,11 +1453,23 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
}
}
CallContext.current().putContextParameter(Nic.class,
guestNic.getUuid());
- s_logger.debug("Successful addition of " + network + " from " +
vmInstance);
+ s_logger.debug(String.format("Successful addition of %s from %s
through %s", network, vmInstance, guestNic));
return _vmDao.findById(vmInstance.getId());
}
/**
+ * duplicated in {@see VirtualMachineManagerImpl} for a {@see VMInstanceVO}
+ */
+ private void checkIfNetExistsForVM(VirtualMachine virtualMachine, Network
network) {
+ List<NicVO> allNics = _nicDao.listByVmId(virtualMachine.getId());
+ for (NicVO nic : allNics) {
+ if (nic.getNetworkId() == network.getId()) {
+ throw new CloudRuntimeException("A NIC already exists for VM:"
+ virtualMachine.getInstanceName() + " in network: " + network.getUuid());
+ }
+ }
+ }
+
+ /**
* If the given MAC address is invalid it replaces the given MAC with the
next available MAC address
*/
protected String validateOrReplaceMacAddress(String macAddress, long
networkId) {
