From 6d35b520d3e6307d930d9ad7542a7fc40cf3535c Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 7 Jan 2022 08:43:48 +0100 Subject: [PATCH 01/11] server: fix vm can be recovered by other accounts (#5822) --- .../apache/cloudstack/api/command/admin/vm/RecoverVMCmd.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RecoverVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RecoverVMCmd.java index 4ad09171540..1a123ecfcdb 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RecoverVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RecoverVMCmd.java @@ -18,6 +18,8 @@ package org.apache.cloudstack.api.command.admin.vm; import org.apache.log4j.Logger; +import org.apache.cloudstack.acl.SecurityChecker.AccessType; +import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -43,6 +45,7 @@ public class RecoverVMCmd extends BaseCmd { //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// + @ACL(accessType = AccessType.OperateEntry) @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = UserVmResponse.class, required = true, description = "The ID of the virtual machine") private Long id; From 84b9b61b9ba079406e2c0c57e07153cff2a44b76 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 7 Jan 2022 08:49:26 +0100 Subject: [PATCH 02/11] api: fix typo in Volume Destroy state - volume can be recovered (#5833) * api: fix type Destroy volume can be recovered * Update api/src/main/java/com/cloud/storage/Volume.java Co-authored-by: dahn --- api/src/main/java/com/cloud/storage/Volume.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/com/cloud/storage/Volume.java b/api/src/main/java/com/cloud/storage/Volume.java index 9036fa5d6c4..a863c3e989b 100644 --- a/api/src/main/java/com/cloud/storage/Volume.java +++ b/api/src/main/java/com/cloud/storage/Volume.java @@ -47,8 +47,8 @@ public interface Volume extends ControlledEntity, Identity, InternalIdentity, Ba RevertSnapshotting("There is a snapshot created on this volume, the volume is being reverting from snapshot"), Resizing("The volume is being resized"), Expunging("The volume is being expunging"), - Expunged("The volume has been expunged"), - Destroy("The volume is destroyed, and can't be recovered."), + Expunged("The volume has been expunged, and can no longer be recovered"), + Destroy("The volume is destroyed, and can be recovered."), Destroying("The volume is destroying, and can't be recovered."), UploadOp("The volume upload operation is in progress or in short the volume is on secondary storage"), Copying("Volume is copying from image store to primary, in case it's an uploaded volume"), From feb4343abec70f5db8671e661671b808ed05a9c1 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Sat, 8 Jan 2022 08:41:58 +0530 Subject: [PATCH 03/11] ui: fix create network/vpc offering form (#5840) * ui: fix create network/vpc offering form Fixes #5838 Signed-off-by: Abhishek Kumar * fix inlinemode Signed-off-by: Abhishek Kumar --- ui/src/views/offering/AddNetworkOffering.vue | 29 +++++++------------- ui/src/views/offering/AddVpcOffering.vue | 28 +++++++------------ 2 files changed, 20 insertions(+), 37 deletions(-) diff --git a/ui/src/views/offering/AddNetworkOffering.vue b/ui/src/views/offering/AddNetworkOffering.vue index 3c38c5a64a2..d114ceb269f 100644 --- a/ui/src/views/offering/AddNetworkOffering.vue +++ b/ui/src/views/offering/AddNetworkOffering.vue @@ -253,7 +253,7 @@ - + Date: Mon, 10 Jan 2022 20:31:53 +0700 Subject: [PATCH 06/11] UI: Add s3 provider option to create secondary storage (#5726) * add s3 provider option to create secondary storage * fixes label name * add storagepolicy for swift provider --- ui/public/locales/en.json | 3 + ui/src/views/infra/AddSecondaryStorage.vue | 178 +++++++++++++++++- ui/src/views/infra/zone/StaticInputsForm.vue | 24 ++- .../infra/zone/ZoneWizardAddResources.vue | 33 +++- .../views/infra/zone/ZoneWizardLaunchZone.vue | 6 + 5 files changed, 212 insertions(+), 32 deletions(-) diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 9dbbd854a1d..ad017345221 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -2949,6 +2949,9 @@ "message.error.retrieve.kubeconfig": "Unable to retrieve Kubernetes cluster config", "message.error.s3nfs.path": "Please enter S3 NFS Path", "message.error.s3nfs.server": "Please enter S3 NFS Server", +"message.error.swift.account": "Please enter account", +"message.error.swift.key": "Please enter key", +"message.error.swift.username": "Please enter username", "message.error.save.setting": "There was an error saving this setting.", "message.error.sbdomain": "Please enter SMB Domain", "message.error.sbdomain.password": "Please enter SMB Domain Password", diff --git a/ui/src/views/infra/AddSecondaryStorage.vue b/ui/src/views/infra/AddSecondaryStorage.vue index 0d34e6f4575..f7b6203a84e 100644 --- a/ui/src/views/infra/AddSecondaryStorage.vue +++ b/ui/src/views/infra/AddSecondaryStorage.vue @@ -42,7 +42,7 @@ >{{ prov }} -
+
+ + + + + + + {{ zone.name }} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+ + + + + + +
{{ $t('label.cancel') }} {{ $t('label.ok') }} @@ -189,11 +274,12 @@ export default { inject: ['parentFetchData'], data () { return { - providers: ['NFS', 'SMB/CIFS', 'Swift'], + providers: ['NFS', 'SMB/CIFS', 'S3', 'Swift'], provider: '', zones: [], zoneSelected: '', - loading: false + loading: false, + secondaryStorageNFSStaging: false } }, beforeCreate () { @@ -245,7 +331,7 @@ export default { handleSubmit (e) { e.preventDefault() if (this.loading) return - this.form.validateFieldsAndScroll((err, values) => { + this.form.validateFieldsAndScroll(async (err, values) => { if (err) { return } @@ -283,26 +369,100 @@ export default { data['details[' + index.toString() + '].key'] = key data['details[' + index.toString() + '].value'] = swiftParams[key] }) + } else if (provider === 'S3') { + let detailIdx = 0 + const s3Params = { + accesskey: values.secondaryStorageAccessKey, + secretkey: values.secondaryStorageSecretKey, + bucket: values.secondaryStorageBucket, + usehttps: values.secondaryStorageHttps ? values.secondaryStorageHttps : false + } + Object.keys(s3Params).forEach((key, index) => { + data['details[' + index.toString() + '].key'] = key + data['details[' + index.toString() + '].value'] = s3Params[key] + detailIdx = index + }) + + if (values.secondaryStorageEndpoint && values.secondaryStorageEndpoint.length > 0) { + detailIdx++ + data['details[' + detailIdx.toString() + '].key'] = 'endpoint' + data['details[' + detailIdx.toString() + '].value'] = values.secondaryStorageEndpoint + } + + if (values.secondaryStorageConnectionTimeout && values.secondaryStorageConnectionTimeout.length > 0) { + detailIdx++ + data['details[' + detailIdx.toString() + '].key'] = 'connectiontimeout' + data['details[' + detailIdx.toString() + '].value'] = values.secondaryStorageConnectionTimeout + } + + if (values.secondaryStorageMaxError && values.secondaryStorageMaxError.length > 0) { + detailIdx++ + data['details[' + detailIdx.toString() + '].key'] = 'maxerrorretry' + data['details[' + detailIdx.toString() + '].value'] = values.secondaryStorageMaxError + } + + if (values.secondaryStorageSocketTimeout && values.secondaryStorageSocketTimeout.length > 0) { + detailIdx++ + data['details[' + detailIdx.toString() + '].key'] = 'sockettimeout' + data['details[' + detailIdx.toString() + '].value'] = values.secondaryStorageSocketTimeout + } } - data.url = url + if (provider !== 'S3') { + data.url = url + } data.provider = provider - if (values.zone && provider !== 'Swift') { + if (values.zone && !['Swift', 'S3'].includes(provider)) { data.zoneid = values.zone } + const nfsParams = {} + if (values.secondaryStorageNFSStaging) { + const nfsServer = values.secondaryStorageNFSServer + const path = values.secondaryStorageNFSPath + const nfsUrl = this.nfsURL(nfsServer, path) + + nfsParams.provider = 'nfs' + nfsParams.zoneid = values.zone + nfsParams.url = nfsUrl + } + this.loading = true - api('addImageStore', data).then(json => { + + try { + await this.addImageStore(data) + + if (values.secondaryStorageNFSStaging) { + await this.createSecondaryStagingStore(nfsParams) + } this.$notification.success({ message: this.$t('label.add.secondary.storage'), description: this.$t('label.add.secondary.storage') }) + this.loading = false this.closeModal() this.parentFetchData() - }).catch(error => { + } catch (error) { this.$notifyError(error) - }).finally(() => { this.loading = false + } + }) + }, + addImageStore (params) { + return new Promise((resolve, reject) => { + api('addImageStore', params).then(json => { + resolve() + }).catch(error => { + reject(error) + }) + }) + }, + createSecondaryStagingStore (params) { + return new Promise((resolve, reject) => { + api('createSecondaryStagingStore', params).then(json => { + resolve() + }).catch(error => { + reject(error) }) }) } diff --git a/ui/src/views/infra/zone/StaticInputsForm.vue b/ui/src/views/infra/zone/StaticInputsForm.vue index 97590a320ab..dc93c090701 100644 --- a/ui/src/views/infra/zone/StaticInputsForm.vue +++ b/ui/src/views/infra/zone/StaticInputsForm.vue @@ -216,21 +216,19 @@ export default { if (!conditions || Object.keys(conditions).length === 0) { return true } - let isShow = false + let isShow = true Object.keys(conditions).forEach(key => { - const condition = conditions[key] - const fieldVal = this.form.getFieldValue(key) - ? this.form.getFieldValue(key) - : (this.prefillContent[key] ? this.prefillContent[key].value : null) - if (Array.isArray(condition) && condition.includes(fieldVal)) { - isShow = true - return false - } else if (!Array.isArray(condition) && fieldVal === condition) { - isShow = true - return false + if (isShow) { + const condition = conditions[key] + const fieldVal = this.form.getFieldValue(key) + ? this.form.getFieldValue(key) + : (this.prefillContent[key] ? this.prefillContent[key].value : null) + if (Array.isArray(condition) && !condition.includes(fieldVal)) { + isShow = false + } else if (!Array.isArray(condition) && fieldVal !== condition) { + isShow = false + } } - - return true }) return isShow diff --git a/ui/src/views/infra/zone/ZoneWizardAddResources.vue b/ui/src/views/infra/zone/ZoneWizardAddResources.vue index 5804676fe9f..ee16e26ca5c 100644 --- a/ui/src/views/infra/zone/ZoneWizardAddResources.vue +++ b/ui/src/views/infra/zone/ZoneWizardAddResources.vue @@ -570,7 +570,7 @@ export default { } }, { - title: 'label.s3.access_key', + title: 'label.s3.access.key', key: 'secondaryStorageAccessKey', required: true, placeHolder: 'message.error.access.key', @@ -579,7 +579,7 @@ export default { } }, { - title: 'label.s3.secret_key', + title: 'label.s3.secret.key', key: 'secondaryStorageSecretKey', required: true, placeHolder: 'message.error.secret.key', @@ -605,7 +605,7 @@ export default { } }, { - title: 'label.s3.use_https', + title: 'label.s3.use.https', key: 'secondaryStorageHttps', required: false, switch: true, @@ -615,7 +615,7 @@ export default { } }, { - title: 'label.s3.connection_timeoutt', + title: 'label.s3.connection.timeout', key: 'secondaryStorageConnectionTimeout', required: false, display: { @@ -623,7 +623,7 @@ export default { } }, { - title: 'label.s3.max_error_retry', + title: 'label.s3.max.error.retry', key: 'secondaryStorageMaxError', required: false, display: { @@ -631,7 +631,7 @@ export default { } }, { - title: 'label.s3.socket_timeout', + title: 'label.s3.socket.timeout', key: 'secondaryStorageSocketTimeout', required: false, display: { @@ -653,7 +653,8 @@ export default { required: true, placeHolder: 'message.error.s3nfs.server', display: { - secondaryStorageProvider: ['S3'] + secondaryStorageProvider: ['S3'], + secondaryStorageNFSStaging: true } }, { @@ -662,7 +663,8 @@ export default { required: true, placeHolder: 'message.error.s3nfs.path', display: { - secondaryStorageProvider: ['S3'] + secondaryStorageProvider: ['S3'], + secondaryStorageNFSStaging: true } }, { @@ -677,7 +679,8 @@ export default { { title: 'label.account', key: 'secondaryStorageAccount', - required: false, + required: true, + placeHolder: 'message.error.swift.account', display: { secondaryStorageProvider: ['Swift'] } @@ -685,7 +688,8 @@ export default { { title: 'label.username', key: 'secondaryStorageUsername', - required: false, + required: true, + placeHolder: 'message.error.swift.username', display: { secondaryStorageProvider: ['Swift'] } @@ -693,6 +697,15 @@ export default { { title: 'label.key', key: 'secondaryStorageKey', + required: true, + placeHolder: 'message.error.swift.key', + display: { + secondaryStorageProvider: ['Swift'] + } + }, + { + title: 'label.storagepolicy', + key: 'secondaryStoragePolicy', required: false, display: { secondaryStorageProvider: ['Swift'] diff --git a/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue b/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue index eb18181aa88..d22cd35c8fd 100644 --- a/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue +++ b/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue @@ -1471,6 +1471,12 @@ export default { params['details[' + index.toString() + '].value'] = this.prefillContent.secondaryStorageKey.value index++ } + if (this.prefillContent.secondaryStoragePolicy && + this.prefillContent.secondaryStoragePolicy.value.length > 0) { + params['details[' + index.toString() + '].key'] = 'storagepolicy' + params['details[' + index.toString() + '].value'] = this.prefillContent.secondaryStoragePolicy.value + index++ + } } try { From 4916f3c90d1aee8c5eadff05b51c3e3ed150dd1c Mon Sep 17 00:00:00 2001 From: Hoang Nguyen Date: Mon, 10 Jan 2022 20:34:46 +0700 Subject: [PATCH 07/11] UI - Fix Locked "Override Root Disk Size" switch (#5843) * Fix Locked "Override Root Disk Size" switch * fixes ut --- ui/src/views/compute/DeployVM.vue | 3 ++- ui/tests/unit/views/AutogenView.spec.js | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ui/src/views/compute/DeployVM.vue b/ui/src/views/compute/DeployVM.vue index bc59d58ce51..979343a8731 100644 --- a/ui/src/views/compute/DeployVM.vue +++ b/ui/src/views/compute/DeployVM.vue @@ -1133,7 +1133,7 @@ export default { this.vm.hostname = host.name } - if (this.serviceOffering.rootdisksize) { + if (this.serviceOffering?.rootdisksize) { this.vm.disksizetotalgb = this.serviceOffering.rootdisksize } else if (this.diskSize) { this.vm.disksizetotalgb = this.diskSize @@ -2097,6 +2097,7 @@ export default { this.updateComputeOffering(null) }, updateTemplateConfigurationOfferingDetails (offeringId) { + this.rootDiskSizeFixed = 0 var offering = this.serviceOffering if (!offering || offering.id !== offeringId) { offering = _.find(this.options.serviceOfferings, (option) => option.id === offeringId) diff --git a/ui/tests/unit/views/AutogenView.spec.js b/ui/tests/unit/views/AutogenView.spec.js index 9a20a0a3306..99df7d9fa42 100644 --- a/ui/tests/unit/views/AutogenView.spec.js +++ b/ui/tests/unit/views/AutogenView.spec.js @@ -57,7 +57,11 @@ mocks = { return option }), info: jest.fn((option) => { - return option + return { + message: option.message, + description: 'test-description', + duration: option.duration + } }), success: jest.fn((option) => { return option From 028d338aaa3587d9caf2b0d83f4b030ac09b4871 Mon Sep 17 00:00:00 2001 From: dahn Date: Mon, 10 Jan 2022 16:31:50 +0100 Subject: [PATCH 08/11] remove VmWorkJob after adding a nic to a vm (#5658) Co-authored-by: Daan Hoogland Co-authored-by: Suresh Kumar Anaparti Co-authored-by: Wei Zhou --- .../cloud/vm/VirtualMachineManagerImpl.java | 95 ++++++++++++++----- .../META-INF/db/schema-41600to41610.sql | 4 +- .../framework/jobs/dao/VmWorkJobDao.java | 2 + .../framework/jobs/dao/VmWorkJobDaoImpl.java | 14 +++ .../framework/jobs/impl/AsyncJobVO.java | 2 +- .../framework/jobs/impl/VmWorkJobVO.java | 24 +++++ .../java/com/cloud/vm/UserVmManagerImpl.java | 21 ++-- 7 files changed, 127 insertions(+), 35 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 42cdb88980d..706b665c286 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 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,37 +5640,64 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac final List 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.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()); - - // 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)); - - _jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vm.getId()); + workJob = createVmWorkJobToAddNetwork(vm, network, requested, context, user, account); } AsyncJobExecutionContext.getCurrentExecutionContext().joinJob(workJob.getId()); return new VmJobVirtualMachineOutcome(workJob, vm.getId()); } + private VmWorkJobVO createVmWorkJobToAddNetwork( + VirtualMachine vm, + Network network, + NicProfile requested, + CallContext context, + User user, + Account account) { + VmWorkJobVO workJob; + workJob = new VmWorkJobVO(context.getContextId()); + + 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; + } + + return workJob; + } + public Outcome removeNicFromVmThroughJobQueue( final VirtualMachine vm, final Nic nic) { @@ -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 24c5b79e658..04bdbca56b1 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 44e39e40291..89601e6b5d2 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 { List listPendingWorkJobs(VirtualMachine.Type type, long instanceId, String jobCmd); + List 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 e81ab1ebbf7..4a10727546e 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 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(); @@ -119,6 +120,19 @@ public class VmWorkJobDaoImpl extends GenericDaoBase implemen return this.listBy(sc, filter); } + @Override + public List listPendingWorkJobs(VirtualMachine.Type type, long instanceId, String jobCmd, String secondaryObjectIdentifier) { + SearchCriteria 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); 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 9d30c2c87b9..8f3c0337837 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 ef0ac7daddf..41eaac598bf 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 c152546d528..28ee6556203 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 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,10 +1453,22 @@ 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 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 */ From fadd74aaca0d9356abc0f642b0a7e0e7ea5e9537 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Tue, 11 Jan 2022 07:46:00 +0100 Subject: [PATCH 09/11] network: fix vm can be deployed on L2 network of other accounts (#5784) * Update #5769: fix domain admin can deploy vm on L2 network of other users * test: fix test_storage_policy.py * Update #5784: revert part of changes in #2420 --- .../main/java/com/cloud/network/NetworkModelImpl.java | 9 ++++----- test/integration/smoke/test_storage_policy.py | 2 ++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java index 3e3c3e0a756..36acaca5690 100644 --- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java @@ -1659,8 +1659,7 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi throw new CloudRuntimeException("cannot check permissions on (Network) "); } // Perform account permission check - if ((network.getGuestType() != GuestType.Shared && network.getGuestType() != GuestType.L2) || - (network.getGuestType() == GuestType.Shared && network.getAclType() == ACLType.Account)) { + if (network.getGuestType() != GuestType.Shared || network.getAclType() == ACLType.Account) { AccountVO networkOwner = _accountDao.findById(network.getAccountId()); if (networkOwner == null) throw new PermissionDeniedException("Unable to use network with id= " + ((NetworkVO)network).getUuid() + @@ -1838,14 +1837,14 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi public boolean isNetworkAvailableInDomain(long networkId, long domainId) { Long networkDomainId = null; Network network = getNetwork(networkId); - if (network.getGuestType() != GuestType.Shared && network.getGuestType() != GuestType.L2) { - s_logger.trace("Network id=" + networkId + " is not shared or L2"); + if (network.getGuestType() != GuestType.Shared) { + s_logger.trace("Network id=" + networkId + " is not shared"); return false; } NetworkDomainVO networkDomainMap = _networkDomainDao.getDomainNetworkMapByNetworkId(networkId); if (networkDomainMap == null) { - s_logger.trace("Network id=" + networkId + " is shared or L2, but not domain specific"); + s_logger.trace("Network id=" + networkId + " is shared, but not domain specific"); return true; } else { networkDomainId = networkDomainMap.getDomainId(); diff --git a/test/integration/smoke/test_storage_policy.py b/test/integration/smoke/test_storage_policy.py index ea35b4db69b..dac610d70c6 100644 --- a/test/integration/smoke/test_storage_policy.py +++ b/test/integration/smoke/test_storage_policy.py @@ -192,6 +192,8 @@ class TestVMWareStoragePolicies(cloudstackTestCase): self.apiclient, self.testdata["l2-network"], zoneid=self.zone.id, + accountid=self.account.name, + domainid=self.account.domainid, networkofferingid=self.network_offering.id ) self.cleanup.append(l2_network) From 7ea2cbd889ac887f6ad08c9ce1c513d323bc46fe Mon Sep 17 00:00:00 2001 From: dahn Date: Tue, 11 Jan 2022 07:48:55 +0100 Subject: [PATCH 10/11] Storage pool absent (#5841) * simple null check Co-authored-by: Daan Hoogland Co-authored-by: Rohit Yadav Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com> --- .../resource/wrapper/LibvirtGetStorageStatsCommandWrapper.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetStorageStatsCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetStorageStatsCommandWrapper.java index 294af5376a4..3e44db088b0 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetStorageStatsCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetStorageStatsCommandWrapper.java @@ -37,6 +37,9 @@ public final class LibvirtGetStorageStatsCommandWrapper extends CommandWrapper Date: Tue, 11 Jan 2022 07:52:59 +0100 Subject: [PATCH 11/11] server: fix enable/disable static nat if userdata is not supported (#5839) * server: fix enable/disable static nat if userdata is not supported * Update #5839: rename applyUserData to applyUserDataIfNeeded * Update server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com> --- .../cloud/network/rules/RulesManagerImpl.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java index b743863edc7..b18fe10a041 100644 --- a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java @@ -24,6 +24,7 @@ import java.util.Set; import javax.inject.Inject; +import com.cloud.exception.UnsupportedServiceException; import com.cloud.network.element.UserDataServiceProvider; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.dao.VMTemplateDao; @@ -605,7 +606,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules // enable static nat on the backend s_logger.trace("Enabling static nat for ip address " + ipAddress + " and vm id=" + vmId + " on the backend"); if (applyStaticNatForIp(ipId, false, caller, false)) { - applyUserData(vmId, network, guestNic); + applyUserDataIfNeeded(vmId, network, guestNic); performedIpAssoc = false; // ignor unassignIPFromVpcNetwork in finally block return true; } else { @@ -629,8 +630,14 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules return false; } - protected void applyUserData(long vmId, Network network, Nic guestNic) throws ResourceUnavailableException { - UserDataServiceProvider element = _networkModel.getUserDataUpdateProvider(network); + protected void applyUserDataIfNeeded(long vmId, Network network, Nic guestNic) throws ResourceUnavailableException { + UserDataServiceProvider element = null; + try { + element = _networkModel.getUserDataUpdateProvider(network); + } catch (UnsupportedServiceException ex) { + s_logger.info(String.format("%s is not supported by network %s, skipping.", Service.UserData.getName(), network)); + return; + } if (element == null) { s_logger.error("Can't find network element for " + Service.UserData.getName() + " provider needed for UserData update"); } else { @@ -1144,7 +1151,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules Nic guestNic = _networkModel.getNicInNetwork(vmId, guestNetwork.getId()); if (applyStaticNatForIp(ipId, false, caller, true)) { if (ipAddress.getState() == IpAddress.State.Releasing) { - applyUserData(vmId, guestNetwork, guestNic); + applyUserDataIfNeeded(vmId, guestNetwork, guestNic); } } else { success = false; @@ -1289,7 +1296,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules if (disableStaticNat(ipId, caller, ctx.getCallingUserId(), false)) { Nic guestNic = _networkModel.getNicInNetworkIncludingRemoved(vmId, guestNetwork.getId()); - applyUserData(vmId, guestNetwork, guestNic); + applyUserDataIfNeeded(vmId, guestNetwork, guestNic); return true; } return false;