From f702f7f57c66f1ba1db2e20f5e072b5637e60d84 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Mon, 5 Feb 2024 13:26:18 +0530 Subject: [PATCH 1/7] Remove sensitive params (VmPassword, etc) from VMWork log (#8553) --- .../src/main/java/com/cloud/vm/VmWork.java | 39 +++++++++++++ .../com/cloud/vm/VmWorkJobHandlerProxy.java | 10 ++-- .../main/java/com/cloud/vm/VmWorkReboot.java | 9 +++ .../main/java/com/cloud/vm/VmWorkStart.java | 9 +++ .../java/com/cloud/vm/VmWorkRebootTest.java | 42 ++++++++++++++ .../java/com/cloud/vm/VmWorkStartTest.java | 57 +++++++++++++++++++ 6 files changed, 160 insertions(+), 6 deletions(-) create mode 100644 engine/orchestration/src/test/java/com/cloud/vm/VmWorkRebootTest.java create mode 100644 engine/orchestration/src/test/java/com/cloud/vm/VmWorkStartTest.java diff --git a/engine/components-api/src/main/java/com/cloud/vm/VmWork.java b/engine/components-api/src/main/java/com/cloud/vm/VmWork.java index a0b5e979524..33338297e26 100644 --- a/engine/components-api/src/main/java/com/cloud/vm/VmWork.java +++ b/engine/components-api/src/main/java/com/cloud/vm/VmWork.java @@ -17,9 +17,21 @@ package com.cloud.vm; import java.io.Serializable; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; + +import com.cloud.serializer.GsonHelper; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.gson.Gson; public class VmWork implements Serializable { private static final long serialVersionUID = -6946320465729853589L; + private static final Gson gsonLogger = GsonHelper.getGsonLogger(); long userId; long accountId; @@ -56,4 +68,31 @@ public class VmWork implements Serializable { public String getHandlerName() { return handlerName; } + + @Override + public String toString() { + return gsonLogger.toJson(this); + } + + protected String toStringAfterRemoveParams(String paramsObjName, List params) { + String ObjJsonStr = gsonLogger.toJson(this); + if (StringUtils.isBlank(ObjJsonStr) || StringUtils.isBlank(paramsObjName) || CollectionUtils.isEmpty(params)) { + return ObjJsonStr; + } + + try { + Map ObjMap = new ObjectMapper().readValue(ObjJsonStr, HashMap.class); + if (ObjMap != null && ObjMap.containsKey(paramsObjName)) { + for (String param : params) { + ((Map)ObjMap.get(paramsObjName)).remove(param); + } + String resultJson = new ObjectMapper().writeValueAsString(ObjMap); + return resultJson; + } + } catch (final JsonProcessingException e) { + // Ignore json exception + } + + return ObjJsonStr; + } } diff --git a/engine/components-api/src/main/java/com/cloud/vm/VmWorkJobHandlerProxy.java b/engine/components-api/src/main/java/com/cloud/vm/VmWorkJobHandlerProxy.java index ce10a83c7cd..a542da6cf8e 100644 --- a/engine/components-api/src/main/java/com/cloud/vm/VmWorkJobHandlerProxy.java +++ b/engine/components-api/src/main/java/com/cloud/vm/VmWorkJobHandlerProxy.java @@ -21,15 +21,13 @@ import java.lang.reflect.Method; import java.util.HashMap; import java.util.Map; -import org.apache.log4j.Logger; - -import com.google.gson.Gson; - import org.apache.cloudstack.framework.jobs.impl.JobSerializerHelper; import org.apache.cloudstack.jobs.JobInfo; +import org.apache.log4j.Logger; import com.cloud.serializer.GsonHelper; import com.cloud.utils.Pair; +import com.google.gson.Gson; /** * VmWorkJobHandlerProxy can not be used as standalone due to run-time @@ -102,12 +100,12 @@ public class VmWorkJobHandlerProxy implements VmWorkJobHandler { try { if (s_logger.isDebugEnabled()) - s_logger.debug("Execute VM work job: " + work.getClass().getName() + _gsonLogger.toJson(work)); + s_logger.debug("Execute VM work job: " + work.getClass().getName() + work); Object obj = method.invoke(_target, work); if (s_logger.isDebugEnabled()) - s_logger.debug("Done executing VM work job: " + work.getClass().getName() + _gsonLogger.toJson(work)); + s_logger.debug("Done executing VM work job: " + work.getClass().getName() + work); assert (obj instanceof Pair); return (Pair)obj; diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VmWorkReboot.java b/engine/orchestration/src/main/java/com/cloud/vm/VmWorkReboot.java index 6a903b3d781..3813a8dc207 100644 --- a/engine/orchestration/src/main/java/com/cloud/vm/VmWorkReboot.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VmWorkReboot.java @@ -17,7 +17,9 @@ package com.cloud.vm; import java.io.Serializable; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.cloudstack.framework.jobs.impl.JobSerializerHelper; @@ -62,4 +64,11 @@ public class VmWorkReboot extends VmWork { } } } + + @Override + public String toString() { + List params = new ArrayList<>(); + params.add(VirtualMachineProfile.Param.VmPassword.getName()); + return super.toStringAfterRemoveParams("rawParams", params); + } } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VmWorkStart.java b/engine/orchestration/src/main/java/com/cloud/vm/VmWorkStart.java index 1b2a7194d17..5a7acdd9edb 100644 --- a/engine/orchestration/src/main/java/com/cloud/vm/VmWorkStart.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VmWorkStart.java @@ -18,7 +18,9 @@ package com.cloud.vm; import java.io.Serializable; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.cloudstack.context.CallContext; @@ -135,4 +137,11 @@ public class VmWorkStart extends VmWork { } } } + + @Override + public String toString() { + List params = new ArrayList<>(); + params.add(VirtualMachineProfile.Param.VmPassword.getName()); + return super.toStringAfterRemoveParams("rawParams", params); + } } diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VmWorkRebootTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VmWorkRebootTest.java new file mode 100644 index 00000000000..75ffd94600a --- /dev/null +++ b/engine/orchestration/src/test/java/com/cloud/vm/VmWorkRebootTest.java @@ -0,0 +1,42 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.vm; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.cloudstack.framework.jobs.impl.JobSerializerHelper; +import org.junit.Assert; +import org.junit.Test; + +public class VmWorkRebootTest { + + @Test + public void testToString() { + VmWork vmWork = new VmWork(1l, 1l, 1l, "testhandler"); + Map params = new HashMap<>(); + String lastHost = "rO0ABXQABHRydWU"; + String lastHostSerialized = JobSerializerHelper.toObjectSerializedString(lastHost); + params.put(VirtualMachineProfile.Param.ConsiderLastHost, lastHost); + params.put(VirtualMachineProfile.Param.VmPassword, "rO0ABXQADnNhdmVkX3Bhc3N3b3Jk"); + VmWorkReboot workInfo = new VmWorkReboot(vmWork, params); + String expectedVmWorkRebootStr = "{\"accountId\":1,\"vmId\":1,\"handlerName\":\"testhandler\",\"userId\":1,\"rawParams\":{\"ConsiderLastHost\":\"" + lastHostSerialized + "\"}}"; + + String vmWorkRebootStr = workInfo.toString(); + Assert.assertEquals(expectedVmWorkRebootStr, vmWorkRebootStr); + } +} diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VmWorkStartTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VmWorkStartTest.java new file mode 100644 index 00000000000..a0644b1976f --- /dev/null +++ b/engine/orchestration/src/test/java/com/cloud/vm/VmWorkStartTest.java @@ -0,0 +1,57 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.vm; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.cloudstack.framework.jobs.impl.JobSerializerHelper; +import org.junit.Assert; +import org.junit.Test; + +public class VmWorkStartTest { + + @Test + public void testToStringWithParams() { + VmWork vmWork = new VmWork(1l, 1l, 1l, "testhandler"); + VmWorkStart workInfo = new VmWorkStart(vmWork); + Map params = new HashMap<>(); + String lastHost = "rO0ABXQABHRydWU"; + String lastHostSerialized = JobSerializerHelper.toObjectSerializedString(lastHost); + params.put(VirtualMachineProfile.Param.ConsiderLastHost, lastHost); + params.put(VirtualMachineProfile.Param.VmPassword, "rO0ABXQADnNhdmVkX3Bhc3N3b3Jk"); + workInfo.setParams(params); + String expectedVmWorkStartStr = "{\"accountId\":1,\"dcId\":0,\"vmId\":1,\"handlerName\":\"testhandler\",\"userId\":1,\"rawParams\":{\"ConsiderLastHost\":\"" + lastHostSerialized + "\"}}"; + + String vmWorkStartStr = workInfo.toString(); + Assert.assertEquals(expectedVmWorkStartStr, vmWorkStartStr); + } + + @Test + public void testToStringWithRawParams() { + VmWork vmWork = new VmWork(1l, 1l, 1l, "testhandler"); + VmWorkStart workInfo = new VmWorkStart(vmWork); + Map rawParams = new HashMap<>(); + rawParams.put(VirtualMachineProfile.Param.ConsiderLastHost.getName(), "rO0ABXQABHRydWU"); + rawParams.put(VirtualMachineProfile.Param.VmPassword.getName(), "rO0ABXQADnNhdmVkX3Bhc3N3b3Jk"); + workInfo.setRawParams(rawParams); + String expectedVmWorkStartStr = "{\"accountId\":1,\"dcId\":0,\"vmId\":1,\"handlerName\":\"testhandler\",\"userId\":1,\"rawParams\":{\"ConsiderLastHost\":\"rO0ABXQABHRydWU\"}}"; + + String vmWorkStartStr = workInfo.toString(); + Assert.assertEquals(expectedVmWorkStartStr, vmWorkStartStr); + } +} From 2df68021761adbd93eecda20114894c8f2edb8bd Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Mon, 5 Feb 2024 13:28:28 +0530 Subject: [PATCH 2/7] Allocate new ROOT volume (on restore virtual machine operation) only when resource count increment succeeds (#8555) * Allocate new volume on restore virtual machine operation when resource count increment succeeds - keep them in transaction, and fail operation if resource count increment fails * Added some (negative) unit tests for restore vm --- .../java/com/cloud/vm/UserVmManagerImpl.java | 100 ++++++---- .../com/cloud/vm/UserVmManagerImplTest.java | 174 ++++++++++++++++++ 2 files changed, 234 insertions(+), 40 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 2927db638ab..0d3f047809a 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -354,6 +354,7 @@ import com.cloud.utils.db.GlobalLock; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.Transaction; import com.cloud.utils.db.TransactionCallbackNoReturn; +import com.cloud.utils.db.TransactionCallbackWithException; import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn; import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.db.UUIDManager; @@ -7765,49 +7766,68 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir List newVols = new ArrayList<>(); for (VolumeVO root : rootVols) { - if ( !Volume.State.Allocated.equals(root.getState()) || newTemplateId != null ){ - Long templateId = root.getTemplateId(); - boolean isISO = false; - if (templateId == null) { - // Assuming that for a vm deployed using ISO, template ID is set to NULL - isISO = true; - templateId = vm.getIsoId(); - } + if ( !Volume.State.Allocated.equals(root.getState()) || newTemplateId != null ) { + final UserVmVO userVm = vm; + Pair vmAndNewVol = Transaction.execute(new TransactionCallbackWithException, CloudRuntimeException>() { + @Override + public Pair doInTransaction(final TransactionStatus status) throws CloudRuntimeException { + Long templateId = root.getTemplateId(); + boolean isISO = false; + if (templateId == null) { + // Assuming that for a vm deployed using ISO, template ID is set to NULL + isISO = true; + templateId = userVm.getIsoId(); + } - /* If new template/ISO is provided allocate a new volume from new template/ISO otherwise allocate new volume from original template/ISO */ - Volume newVol = null; - if (newTemplateId != null) { - if (isISO) { - newVol = volumeMgr.allocateDuplicateVolume(root, null); - vm.setIsoId(newTemplateId); - vm.setGuestOSId(template.getGuestOSId()); - vm.setTemplateId(newTemplateId); - } else { - newVol = volumeMgr.allocateDuplicateVolume(root, newTemplateId); - vm.setGuestOSId(template.getGuestOSId()); - vm.setTemplateId(newTemplateId); + /* If new template/ISO is provided allocate a new volume from new template/ISO otherwise allocate new volume from original template/ISO */ + Volume newVol = null; + if (newTemplateId != null) { + if (isISO) { + newVol = volumeMgr.allocateDuplicateVolume(root, null); + userVm.setIsoId(newTemplateId); + userVm.setGuestOSId(template.getGuestOSId()); + userVm.setTemplateId(newTemplateId); + } else { + newVol = volumeMgr.allocateDuplicateVolume(root, newTemplateId); + userVm.setGuestOSId(template.getGuestOSId()); + userVm.setTemplateId(newTemplateId); + } + // check and update VM if it can be dynamically scalable with the new template + updateVMDynamicallyScalabilityUsingTemplate(userVm, newTemplateId); + } else { + newVol = volumeMgr.allocateDuplicateVolume(root, null); + } + newVols.add(newVol); + + if (userVmDetailsDao.findDetail(userVm.getId(), VmDetailConstants.ROOT_DISK_SIZE) == null && !newVol.getSize().equals(template.getSize())) { + VolumeVO resizedVolume = (VolumeVO) newVol; + if (template.getSize() != null) { + resizedVolume.setSize(template.getSize()); + _volsDao.update(resizedVolume.getId(), resizedVolume); + } + } + + // 1. Save usage event and update resource count for user vm volumes + try { + _resourceLimitMgr.incrementResourceCount(newVol.getAccountId(), ResourceType.volume, newVol.isDisplay()); + _resourceLimitMgr.incrementResourceCount(newVol.getAccountId(), ResourceType.primary_storage, newVol.isDisplay(), new Long(newVol.getSize())); + } catch (final CloudRuntimeException e) { + throw e; + } catch (final Exception e) { + s_logger.error("Unable to restore VM " + userVm.getUuid(), e); + throw new CloudRuntimeException(e); + } + + // 2. Create Usage event for the newly created volume + UsageEventVO usageEvent = new UsageEventVO(EventTypes.EVENT_VOLUME_CREATE, newVol.getAccountId(), newVol.getDataCenterId(), newVol.getId(), newVol.getName(), newVol.getDiskOfferingId(), template.getId(), newVol.getSize()); + _usageEventDao.persist(usageEvent); + + return new Pair<>(userVm, newVol); } - // check and update VM if it can be dynamically scalable with the new template - updateVMDynamicallyScalabilityUsingTemplate(vm, newTemplateId); - } else { - newVol = volumeMgr.allocateDuplicateVolume(root, null); - } - newVols.add(newVol); + }); - if (userVmDetailsDao.findDetail(vm.getId(), VmDetailConstants.ROOT_DISK_SIZE) == null && !newVol.getSize().equals(template.getSize())) { - VolumeVO resizedVolume = (VolumeVO) newVol; - if (template.getSize() != null) { - resizedVolume.setSize(template.getSize()); - _volsDao.update(resizedVolume.getId(), resizedVolume); - } - } - - // 1. Save usage event and update resource count for user vm volumes - _resourceLimitMgr.incrementResourceCount(newVol.getAccountId(), ResourceType.volume, newVol.isDisplay()); - _resourceLimitMgr.incrementResourceCount(newVol.getAccountId(), ResourceType.primary_storage, newVol.isDisplay(), new Long(newVol.getSize())); - // 2. Create Usage event for the newly created volume - UsageEventVO usageEvent = new UsageEventVO(EventTypes.EVENT_VOLUME_CREATE, newVol.getAccountId(), newVol.getDataCenterId(), newVol.getId(), newVol.getName(), newVol.getDiskOfferingId(), template.getId(), newVol.getSize()); - _usageEventDao.persist(usageEvent); + vm = vmAndNewVol.first(); + Volume newVol = vmAndNewVol.second(); handleManagedStorage(vm, root); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 16607df7a9f..e2c2b8ef9e2 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -30,6 +30,7 @@ import com.cloud.exception.InsufficientAddressCapacityException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InsufficientServerCapacityException; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.host.Host; @@ -47,6 +48,8 @@ import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.GuestOSVO; import com.cloud.storage.ScopeType; +import com.cloud.storage.Snapshot; +import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; @@ -54,6 +57,7 @@ import com.cloud.storage.VolumeApiService; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSDao; +import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.template.VirtualMachineTemplate; @@ -66,6 +70,7 @@ import com.cloud.user.UserData; import com.cloud.user.UserDataVO; import com.cloud.user.UserVO; import com.cloud.user.dao.AccountDao; +import com.cloud.user.dao.UserDao; import com.cloud.user.dao.UserDataDao; import com.cloud.uservm.UserVm; import com.cloud.utils.Pair; @@ -74,10 +79,14 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.UserVmDetailsDao; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; + import org.apache.cloudstack.api.BaseCmd.HTTPMethod; import org.apache.cloudstack.api.command.user.vm.DeployVMCmd; import org.apache.cloudstack.api.command.user.vm.DeployVnfApplianceCmd; import org.apache.cloudstack.api.command.user.vm.ResetVMUserDataCmd; +import org.apache.cloudstack.api.command.user.vm.RestoreVMCmd; import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd; import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd; import org.apache.cloudstack.context.CallContext; @@ -191,6 +200,9 @@ public class UserVmManagerImplTest { @Mock private AccountDao accountDao; + @Mock + private UserDao userDao; + @Mock ResourceLimitService resourceLimitMgr; @@ -218,6 +230,12 @@ public class UserVmManagerImplTest { @Mock private VolumeDao volumeDaoMock; + @Mock + private SnapshotDao snapshotDaoMock; + + @Mock + private VMSnapshotDao vmSnapshotDaoMock; + @Mock AccountVO account; @@ -1221,4 +1239,160 @@ public class UserVmManagerImplTest { Mockito.verify(userVmVoMock).setLastHostId(2L); Mockito.verify(userVmVoMock).setState(VirtualMachine.State.Running); } + + @Test(expected = InvalidParameterValueException.class) + public void testRestoreVMNoVM() throws ResourceUnavailableException, InsufficientCapacityException { + CallContext callContextMock = Mockito.mock(CallContext.class); + Mockito.lenient().doReturn(accountMock).when(callContextMock).getCallingAccount(); + + RestoreVMCmd cmd = Mockito.mock(RestoreVMCmd.class); + when(cmd.getVmId()).thenReturn(vmId); + when(cmd.getTemplateId()).thenReturn(2L); + when(userVmDao.findById(vmId)).thenReturn(null); + + userVmManagerImpl.restoreVM(cmd); + } + + @Test(expected = CloudRuntimeException.class) + public void testRestoreVMWithVolumeSnapshots() throws ResourceUnavailableException, InsufficientCapacityException { + CallContext callContextMock = Mockito.mock(CallContext.class); + Mockito.lenient().doReturn(accountMock).when(callContextMock).getCallingAccount(); + Mockito.lenient().doNothing().when(accountManager).checkAccess(accountMock, null, true, userVmVoMock); + + RestoreVMCmd cmd = Mockito.mock(RestoreVMCmd.class); + when(cmd.getVmId()).thenReturn(vmId); + when(cmd.getTemplateId()).thenReturn(2L); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + + List volumes = new ArrayList<>(); + long rootVolumeId = 1l; + VolumeVO rootVolumeOfVm = Mockito.mock(VolumeVO.class); + Mockito.when(rootVolumeOfVm.getId()).thenReturn(rootVolumeId); + volumes.add(rootVolumeOfVm); + when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.ROOT)).thenReturn(volumes); + + List snapshots = new ArrayList<>(); + SnapshotVO snapshot = Mockito.mock(SnapshotVO.class); + snapshots.add(snapshot); + when(snapshotDaoMock.listByStatus(rootVolumeId, Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp)).thenReturn(snapshots); + + userVmManagerImpl.restoreVM(cmd); + } + + @Test(expected = InvalidParameterValueException.class) + public void testRestoreVirtualMachineNoOwner() throws ResourceUnavailableException, InsufficientCapacityException { + long userId = 1l; + long accountId = 2l; + long newTemplateId = 2l; + when(accountMock.getId()).thenReturn(userId); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getAccountId()).thenReturn(accountId); + when(accountDao.findById(accountId)).thenReturn(null); + + userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId); + } + + @Test(expected = PermissionDeniedException.class) + public void testRestoreVirtualMachineOwnerDisabled() throws ResourceUnavailableException, InsufficientCapacityException { + long userId = 1l; + long accountId = 2l; + long newTemplateId = 2l; + when(accountMock.getId()).thenReturn(userId); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getAccountId()).thenReturn(accountId); + when(accountDao.findById(accountId)).thenReturn(callerAccount); + when(callerAccount.getState()).thenReturn(Account.State.DISABLED); + + userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId); + } + + @Test(expected = CloudRuntimeException.class) + public void testRestoreVirtualMachineNotInRightState() throws ResourceUnavailableException, InsufficientCapacityException { + long userId = 1l; + long accountId = 2l; + long newTemplateId = 2l; + when(accountMock.getId()).thenReturn(userId); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getAccountId()).thenReturn(accountId); + when(userVmVoMock.getUuid()).thenReturn("a967643d-7633-4ab4-ac26-9c0b63f50cc1"); + when(accountDao.findById(accountId)).thenReturn(callerAccount); + when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Starting); + + userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId); + } + + @Test(expected = InvalidParameterValueException.class) + public void testRestoreVirtualMachineNoRootVolume() throws ResourceUnavailableException, InsufficientCapacityException { + long userId = 1l; + long accountId = 2l; + long currentTemplateId = 1l; + long newTemplateId = 2l; + when(accountMock.getId()).thenReturn(userId); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getAccountId()).thenReturn(accountId); + when(userVmVoMock.getUuid()).thenReturn("a967643d-7633-4ab4-ac26-9c0b63f50cc1"); + when(accountDao.findById(accountId)).thenReturn(callerAccount); + when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running); + when(userVmVoMock.getTemplateId()).thenReturn(currentTemplateId); + + VMTemplateVO currentTemplate = Mockito.mock(VMTemplateVO.class); + when(templateDao.findById(currentTemplateId)).thenReturn(currentTemplate); + when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.ROOT)).thenReturn(new ArrayList()); + + userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId); + } + + @Test(expected = InvalidParameterValueException.class) + public void testRestoreVirtualMachineMoreThanOneRootVolume() throws ResourceUnavailableException, InsufficientCapacityException { + long userId = 1l; + long accountId = 2l; + long currentTemplateId = 1l; + long newTemplateId = 2l; + when(accountMock.getId()).thenReturn(userId); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getAccountId()).thenReturn(accountId); + when(userVmVoMock.getUuid()).thenReturn("a967643d-7633-4ab4-ac26-9c0b63f50cc1"); + when(accountDao.findById(accountId)).thenReturn(callerAccount); + when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running); + when(userVmVoMock.getTemplateId()).thenReturn(currentTemplateId); + + VMTemplateVO currentTemplate = Mockito.mock(VMTemplateVO.class); + when(currentTemplate.isDeployAsIs()).thenReturn(false); + when(templateDao.findById(currentTemplateId)).thenReturn(currentTemplate); + List volumes = new ArrayList<>(); + VolumeVO rootVolume1 = Mockito.mock(VolumeVO.class); + volumes.add(rootVolume1); + VolumeVO rootVolume2 = Mockito.mock(VolumeVO.class); + volumes.add(rootVolume2); + when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.ROOT)).thenReturn(volumes); + + userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId); + } + + @Test(expected = InvalidParameterValueException.class) + public void testRestoreVirtualMachineWithVMSnapshots() throws ResourceUnavailableException, InsufficientCapacityException { + long userId = 1l; + long accountId = 2l; + long currentTemplateId = 1l; + long newTemplateId = 2l; + when(accountMock.getId()).thenReturn(userId); + when(userVmDao.findById(vmId)).thenReturn(userVmVoMock); + when(userVmVoMock.getAccountId()).thenReturn(accountId); + when(accountDao.findById(accountId)).thenReturn(callerAccount); + when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running); + when(userVmVoMock.getTemplateId()).thenReturn(currentTemplateId); + + VMTemplateVO currentTemplate = Mockito.mock(VMTemplateVO.class); + when(templateDao.findById(currentTemplateId)).thenReturn(currentTemplate); + List volumes = new ArrayList<>(); + VolumeVO rootVolumeOfVm = Mockito.mock(VolumeVO.class); + volumes.add(rootVolumeOfVm); + when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.ROOT)).thenReturn(volumes); + List vmSnapshots = new ArrayList<>(); + VMSnapshotVO vmSnapshot = Mockito.mock(VMSnapshotVO.class); + vmSnapshots.add(vmSnapshot); + when(vmSnapshotDaoMock.findByVm(vmId)).thenReturn(vmSnapshots); + + userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId); + } } From 9f1b34aeb22f497dd65b2097ff970997a64a1a22 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Mon, 5 Feb 2024 01:00:10 -0700 Subject: [PATCH 3/7] Fix libvirt domain event listener by properly processing events (#8437) * Fix libvirt domain event listener by properly processing events * Add javadoc for setupEventListener --------- Co-authored-by: Marcus Sorensen --- .../resource/LibvirtComputingResource.java | 16 +------- .../kvm/resource/LibvirtConnection.java | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 60e6bcffeb6..fef5d4aa6de 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -85,7 +85,6 @@ import org.libvirt.DomainInfo; import org.libvirt.DomainInfo.DomainState; import org.libvirt.DomainInterfaceStats; import org.libvirt.DomainSnapshot; -import org.libvirt.Library; import org.libvirt.LibvirtException; import org.libvirt.MemoryStatistic; import org.libvirt.Network; @@ -3694,20 +3693,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } private void setupLibvirtEventListener() { - final Thread libvirtListenerThread = new Thread(() -> { - try { - Library.runEventLoop(); - } catch (LibvirtException e) { - s_logger.error("LibvirtException was thrown in event loop: ", e); - } catch (InterruptedException e) { - s_logger.error("Libvirt event loop was interrupted: ", e); - } - }); - try { - libvirtListenerThread.setDaemon(true); - libvirtListenerThread.start(); - Connect conn = LibvirtConnection.getConnection(); conn.addLifecycleListener(this::onDomainLifecycleChange); @@ -3727,7 +3713,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv * Checking for this helps us differentiate between events where cloudstack or admin stopped the VM vs guest * initiated, and avoid pushing extra updates for actions we are initiating without a need for extra tracking */ DomainEventDetail detail = domainEvent.getDetail(); - if (StoppedDetail.SHUTDOWN.equals(detail) || StoppedDetail.CRASHED.equals(detail)) { + if (StoppedDetail.SHUTDOWN.equals(detail) || StoppedDetail.CRASHED.equals(detail) || StoppedDetail.FAILED.equals(detail)) { s_logger.info("Triggering out of band status update due to completed self-shutdown or crash of VM"); _agentStatusUpdater.triggerUpdate(); } else { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtConnection.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtConnection.java index 7563f964759..1f4ab23a43f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtConnection.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtConnection.java @@ -23,6 +23,7 @@ import com.cloud.agent.properties.AgentProperties; import com.cloud.agent.properties.AgentPropertiesFileHandler; import org.apache.log4j.Logger; import org.libvirt.Connect; +import org.libvirt.Library; import org.libvirt.LibvirtException; import com.cloud.hypervisor.Hypervisor; @@ -34,6 +35,7 @@ public class LibvirtConnection { static private Connect s_connection; static private String s_hypervisorURI; + static private Thread libvirtEventThread; static public Connect getConnection() throws LibvirtException { return getConnection(s_hypervisorURI); @@ -45,6 +47,8 @@ public class LibvirtConnection { if (conn == null) { s_logger.info("No existing libvirtd connection found. Opening a new one"); + + setupEventListener(); conn = new Connect(hypervisorURI, false); s_logger.debug("Successfully connected to libvirt at: " + hypervisorURI); s_connections.put(hypervisorURI, conn); @@ -53,7 +57,15 @@ public class LibvirtConnection { conn.getVersion(); } catch (LibvirtException e) { s_logger.error("Connection with libvirtd is broken: " + e.getMessage()); + + try { + conn.close(); + } catch (LibvirtException closeEx) { + s_logger.debug("Ignoring error while trying to close broken connection:" + closeEx.getMessage()); + } + s_logger.debug("Opening a new libvirtd connection to: " + hypervisorURI); + setupEventListener(); conn = new Connect(hypervisorURI, false); s_connections.put(hypervisorURI, conn); } @@ -101,4 +113,33 @@ public class LibvirtConnection { return "qemu:///system"; } + + /** + * Set up Libvirt event handling and polling. This is not specific to a connection object instance, but needs + * to be done prior to creating connections. See the Libvirt documentation for virEventRegisterDefaultImpl and + * virEventRunDefaultImpl or the libvirt-java Library Javadoc for more information. + * @throws LibvirtException + */ + private static synchronized void setupEventListener() throws LibvirtException { + if (libvirtEventThread == null || !libvirtEventThread.isAlive()) { + // Registers a default event loop, must be called before connecting to hypervisor + Library.initEventLoop(); + libvirtEventThread = new Thread(() -> { + while (true) { + try { + // This blocking call contains a loop of its own that will process events until the event loop is stopped or exception is thrown. + Library.runEventLoop(); + } catch (LibvirtException e) { + s_logger.error("LibvirtException was thrown in event loop: ", e); + } catch (InterruptedException e) { + s_logger.error("Libvirt event loop was interrupted: ", e); + } + } + }); + + // Process events in separate thread. Failure to run event loop regularly will cause connections to close due to keepalive timeout. + libvirtEventThread.setDaemon(true); + libvirtEventThread.start(); + } + } } From 5361b415e6afb7593da5f928c64c570be36a9502 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 5 Feb 2024 03:47:29 -0500 Subject: [PATCH 4/7] Image Store: View Access status of the image store and view events (#8467) --- api/src/main/java/com/cloud/event/EventTypes.java | 2 ++ server/src/main/java/com/cloud/storage/StorageManagerImpl.java | 2 ++ ui/src/components/view/ListView.vue | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/com/cloud/event/EventTypes.java b/api/src/main/java/com/cloud/event/EventTypes.java index 67fed5500ee..5d525229095 100644 --- a/api/src/main/java/com/cloud/event/EventTypes.java +++ b/api/src/main/java/com/cloud/event/EventTypes.java @@ -400,6 +400,7 @@ public class EventTypes { public static final String EVENT_IMAGE_STORE_DATA_MIGRATE = "IMAGE.STORE.MIGRATE.DATA"; public static final String EVENT_IMAGE_STORE_RESOURCES_MIGRATE = "IMAGE.STORE.MIGRATE.RESOURCES"; public static final String EVENT_IMAGE_STORE_OBJECT_DOWNLOAD = "IMAGE.STORE.OBJECT.DOWNLOAD"; + public static final String EVENT_UPDATE_IMAGE_STORE_ACCESS_STATE = "IMAGE.STORE.ACCESS.UPDATED"; // Configuration Table public static final String EVENT_CONFIGURATION_VALUE_EDIT = "CONFIGURATION.VALUE.EDIT"; @@ -1164,6 +1165,7 @@ public class EventTypes { entityEventDetails.put(EVENT_IMAGE_STORE_DATA_MIGRATE, ImageStore.class); entityEventDetails.put(EVENT_IMAGE_STORE_OBJECT_DOWNLOAD, ImageStore.class); + entityEventDetails.put(EVENT_UPDATE_IMAGE_STORE_ACCESS_STATE, ImageStore.class); entityEventDetails.put(EVENT_LIVE_PATCH_SYSTEMVM, "SystemVMs"); //Object Store diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 04889d0b2a8..615c7b6760e 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -3261,6 +3261,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } @Override + @ActionEvent(eventType = EventTypes.EVENT_UPDATE_IMAGE_STORE_ACCESS_STATE, + eventDescription = "image store access updated") public ImageStore updateImageStoreStatus(Long id, Boolean readonly) { // Input validation ImageStoreVO imageStoreVO = _imageStoreDao.findById(id); diff --git a/ui/src/components/view/ListView.vue b/ui/src/components/view/ListView.vue index a61f1930f71..093e7d663a0 100644 --- a/ui/src/components/view/ListView.vue +++ b/ui/src/components/view/ListView.vue @@ -340,7 +340,7 @@ -