This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push:
new 1a1131154ef Fixup vm powerstate update (#8545)
1a1131154ef is described below
commit 1a1131154ef27e0f5491ceb249eded7fd810c021
Author: Vishesh <vishes...@gmail.com>
AuthorDate: Mon Feb 19 18:26:21 2024 +0530
Fixup vm powerstate update (#8545)
Co-authored-by: Suresh Kumar Anaparti <suresh.anapa...@shapeblue.com>
---
.../com/cloud/vm/VirtualMachineManagerImpl.java | 7 +-
.../java/com/cloud/vm/dao/VMInstanceDaoImpl.java | 23 +++-
.../com/cloud/vm/dao/VMInstanceDaoImplTest.java | 152 +++++++++++++++++++--
3 files changed, 161 insertions(+), 21 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 29c60698398..59d129bc065 100755
---
a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
+++
b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -213,8 +213,8 @@ import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.storage.DiskOfferingVO;
import com.cloud.storage.ScopeType;
-import com.cloud.storage.Storage.ImageFormat;
import com.cloud.storage.Storage;
+import com.cloud.storage.Storage.ImageFormat;
import com.cloud.storage.StorageManager;
import com.cloud.storage.StoragePool;
import com.cloud.storage.VMTemplateVO;
@@ -2208,9 +2208,11 @@ public class VirtualMachineManagerImpl extends
ManagerBase implements VirtualMac
boolean result = stateTransitTo(vm, Event.OperationSucceeded,
null);
if (result) {
+ vm.setPowerState(PowerState.PowerOff);
+ _vmDao.update(vm.getId(), vm);
if (VirtualMachine.Type.User.equals(vm.type) &&
ResourceCountRunningVMsonly.value()) {
ServiceOfferingVO offering =
_offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
- resourceCountDecrement(vm.getAccountId(),new
Long(offering.getCpu()), new Long(offering.getRamSize()));
+ resourceCountDecrement(vm.getAccountId(),
offering.getCpu().longValue(), offering.getRamSize().longValue());
}
} else {
throw new CloudRuntimeException("unable to stop " + vm);
@@ -2761,6 +2763,7 @@ public class VirtualMachineManagerImpl extends
ManagerBase implements VirtualMac
}
vm.setLastHostId(srcHostId);
+ _vmDao.resetVmPowerStateTracking(vm.getId());
try {
if (vm.getHostId() == null || vm.getHostId() != srcHostId ||
!changeState(vm, Event.MigrationRequested, dstHostId, work, Step.Migrating)) {
_networkMgr.rollbackNicForMigration(vmSrc, profile);
diff --git
a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java
b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java
index 916687baeb4..322895f0ec5 100755
--- a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java
@@ -66,7 +66,7 @@ import com.cloud.vm.VirtualMachine.Type;
public class VMInstanceDaoImpl extends GenericDaoBase<VMInstanceVO, Long>
implements VMInstanceDao {
public static final Logger s_logger =
Logger.getLogger(VMInstanceDaoImpl.class);
- private static final int MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT = 3;
+ static final int MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT = 3;
protected SearchBuilder<VMInstanceVO> VMClusterSearch;
protected SearchBuilder<VMInstanceVO> LHVMClusterSearch;
@@ -897,17 +897,19 @@ public class VMInstanceDaoImpl extends
GenericDaoBase<VMInstanceVO, Long> implem
@Override
public boolean updatePowerState(final long instanceId, final long
powerHostId, final VirtualMachine.PowerState powerState, Date wisdomEra) {
- return Transaction.execute(new TransactionCallback<Boolean>() {
+ return Transaction.execute(new TransactionCallback<>() {
@Override
public Boolean doInTransaction(TransactionStatus status) {
boolean needToUpdate = false;
VMInstanceVO instance = findById(instanceId);
if (instance != null
- && (null == instance.getPowerStateUpdateTime()
+ && (null == instance.getPowerStateUpdateTime()
||
instance.getPowerStateUpdateTime().before(wisdomEra))) {
Long savedPowerHostId = instance.getPowerHostId();
- if (instance.getPowerState() != powerState ||
savedPowerHostId == null
- || savedPowerHostId.longValue() != powerHostId) {
+ if (instance.getPowerState() != powerState
+ || savedPowerHostId == null
+ || savedPowerHostId != powerHostId
+ ||
!isPowerStateInSyncWithInstanceState(powerState, powerHostId, instance)) {
instance.setPowerState(powerState);
instance.setPowerHostId(powerHostId);
instance.setPowerStateUpdateCount(1);
@@ -929,6 +931,17 @@ public class VMInstanceDaoImpl extends
GenericDaoBase<VMInstanceVO, Long> implem
});
}
+ private boolean isPowerStateInSyncWithInstanceState(final
VirtualMachine.PowerState powerState, final long powerHostId, final
VMInstanceVO instance) {
+ State instanceState = instance.getState();
+ if ((powerState == VirtualMachine.PowerState.PowerOff && instanceState
== State.Running)
+ || (powerState == VirtualMachine.PowerState.PowerOn &&
instanceState == State.Stopped)) {
+ s_logger.debug(String.format("VM id: %d on host id: %d and power
host id: %d is in %s state, but power state is %s",
+ instance.getId(), instance.getHostId(), powerHostId,
instanceState, powerState));
+ return false;
+ }
+ return true;
+ }
+
@Override
public boolean isPowerStateUpToDate(final long instanceId) {
VMInstanceVO instance = findById(instanceId);
diff --git
a/engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDaoImplTest.java
b/engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDaoImplTest.java
index 767b41420b7..9dc773cd7d6 100644
--- a/engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDaoImplTest.java
+++ b/engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDaoImplTest.java
@@ -17,22 +17,32 @@
package com.cloud.vm.dao;
-import com.cloud.utils.Pair;
-import com.cloud.vm.VirtualMachine;
+import static com.cloud.vm.VirtualMachine.State.Running;
+import static com.cloud.vm.VirtualMachine.State.Stopped;
+import static
com.cloud.vm.dao.VMInstanceDaoImpl.MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Date;
+
import org.joda.time.DateTime;
import org.junit.Before;
import org.junit.Test;
-import org.junit.Assert;
import org.mockito.Mock;
-
-import static com.cloud.vm.VirtualMachine.State.Running;
-import static com.cloud.vm.VirtualMachine.State.Stopped;
-
-import static org.mockito.Mockito.when;
-import com.cloud.vm.VMInstanceVO;
import org.mockito.MockitoAnnotations;
import org.mockito.Spy;
+import com.cloud.utils.Pair;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.VirtualMachine;
+
/**
* Created by sudharma_jain on 3/2/17.
*/
@@ -55,16 +65,130 @@ public class VMInstanceDaoImplTest {
}
@Test
- public void testUpdateState() throws Exception {
+ public void testUpdateState() {
Long destHostId = null;
- Pair<Long, Long> opaqueMock = new Pair<Long, Long>(new Long(1),
destHostId);
+ Pair<Long, Long> opaqueMock = new Pair<>(1L, destHostId);
vmInstanceDao.updateState(Stopped,
VirtualMachine.Event.FollowAgentPowerOffReport, Stopped, vm , opaqueMock);
}
@Test
- public void testIfStateAndHostUnchanged() throws Exception {
- Assert.assertEquals(vmInstanceDao.ifStateUnchanged(Stopped, Stopped,
null, null), true);
- Assert.assertEquals(vmInstanceDao.ifStateUnchanged(Stopped, Running,
null, null), false);
+ public void testIfStateAndHostUnchanged() {
+ assertTrue(vmInstanceDao.ifStateUnchanged(Stopped, Stopped, null,
null));
+ assertFalse(vmInstanceDao.ifStateUnchanged(Stopped, Running, null,
null));
+ }
+
+ @Test
+ public void testUpdatePowerStateDifferentPowerState() {
+ when(vm.getPowerStateUpdateTime()).thenReturn(null);
+ when(vm.getPowerHostId()).thenReturn(1L);
+ when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
+ doReturn(vm).when(vmInstanceDao).findById(anyLong());
+ doReturn(true).when(vmInstanceDao).update(anyLong(), any());
+
+ boolean result = vmInstanceDao.updatePowerState(1L, 1L,
VirtualMachine.PowerState.PowerOff, new Date());
+
+ verify(vm, times(1)).setPowerState(VirtualMachine.PowerState.PowerOff);
+ verify(vm, times(1)).setPowerHostId(1L);
+ verify(vm, times(1)).setPowerStateUpdateCount(1);
+ verify(vm, times(1)).setPowerStateUpdateTime(any(Date.class));
+
+ assertTrue(result);
+ }
+
+ @Test
+ public void testUpdatePowerStateVmNotFound() {
+ when(vm.getPowerStateUpdateTime()).thenReturn(null);
+ when(vm.getPowerHostId()).thenReturn(1L);
+ when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
+ doReturn(null).when(vmInstanceDao).findById(anyLong());
+
+ boolean result = vmInstanceDao.updatePowerState(1L, 1L,
VirtualMachine.PowerState.PowerOff, new Date());
+
+ verify(vm, never()).setPowerState(any());
+ verify(vm, never()).setPowerHostId(anyLong());
+ verify(vm, never()).setPowerStateUpdateCount(any(Integer.class));
+ verify(vm, never()).setPowerStateUpdateTime(any(Date.class));
+
+ assertFalse(result);
+ }
+
+ @Test
+ public void testUpdatePowerStateNoChangeFirstUpdate() {
+ when(vm.getPowerStateUpdateTime()).thenReturn(null);
+ when(vm.getPowerHostId()).thenReturn(1L);
+ when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
+ when(vm.getState()).thenReturn(Running);
+ when(vm.getPowerStateUpdateCount()).thenReturn(1);
+ doReturn(vm).when(vmInstanceDao).findById(anyLong());
+ doReturn(true).when(vmInstanceDao).update(anyLong(), any());
+
+ boolean result = vmInstanceDao.updatePowerState(1L, 1L,
VirtualMachine.PowerState.PowerOn, new Date());
+
+ verify(vm, never()).setPowerState(any());
+ verify(vm, never()).setPowerHostId(anyLong());
+ verify(vm, times(1)).setPowerStateUpdateCount(2);
+ verify(vm, times(1)).setPowerStateUpdateTime(any(Date.class));
+
+ assertTrue(result);
+ }
+
+ @Test
+ public void testUpdatePowerStateNoChangeMaxUpdatesValidState() {
+ when(vm.getPowerStateUpdateTime()).thenReturn(null);
+ when(vm.getPowerHostId()).thenReturn(1L);
+ when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
+
when(vm.getPowerStateUpdateCount()).thenReturn(MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT);
+ when(vm.getState()).thenReturn(Running);
+ doReturn(vm).when(vmInstanceDao).findById(anyLong());
+ doReturn(true).when(vmInstanceDao).update(anyLong(), any());
+
+ boolean result = vmInstanceDao.updatePowerState(1L, 1L,
VirtualMachine.PowerState.PowerOn, new Date());
+
+ verify(vm, never()).setPowerState(any());
+ verify(vm, never()).setPowerHostId(anyLong());
+ verify(vm, never()).setPowerStateUpdateCount(any(Integer.class));
+ verify(vm, never()).setPowerStateUpdateTime(any(Date.class));
+
+ assertFalse(result);
}
+ @Test
+ public void testUpdatePowerStateNoChangeMaxUpdatesInvalidStateVmStopped() {
+ when(vm.getPowerStateUpdateTime()).thenReturn(null);
+ when(vm.getPowerHostId()).thenReturn(1L);
+ when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
+
when(vm.getPowerStateUpdateCount()).thenReturn(MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT);
+ when(vm.getState()).thenReturn(Stopped);
+ doReturn(vm).when(vmInstanceDao).findById(anyLong());
+ doReturn(true).when(vmInstanceDao).update(anyLong(), any());
+
+ boolean result = vmInstanceDao.updatePowerState(1L, 1L,
VirtualMachine.PowerState.PowerOn, new Date());
+
+ verify(vm, times(1)).setPowerState(any());
+ verify(vm, times(1)).setPowerHostId(anyLong());
+ verify(vm, times(1)).setPowerStateUpdateCount(1);
+ verify(vm, times(1)).setPowerStateUpdateTime(any(Date.class));
+
+ assertTrue(result);
+ }
+
+ @Test
+ public void testUpdatePowerStateNoChangeMaxUpdatesInvalidStateVmRunning() {
+ when(vm.getPowerStateUpdateTime()).thenReturn(null);
+ when(vm.getPowerHostId()).thenReturn(1L);
+
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOff);
+
when(vm.getPowerStateUpdateCount()).thenReturn(MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT);
+ when(vm.getState()).thenReturn(Running);
+ doReturn(vm).when(vmInstanceDao).findById(anyLong());
+ doReturn(true).when(vmInstanceDao).update(anyLong(), any());
+
+ boolean result = vmInstanceDao.updatePowerState(1L, 1L,
VirtualMachine.PowerState.PowerOff, new Date());
+
+ verify(vm, times(1)).setPowerState(any());
+ verify(vm, times(1)).setPowerHostId(anyLong());
+ verify(vm, times(1)).setPowerStateUpdateCount(1);
+ verify(vm, times(1)).setPowerStateUpdateTime(any(Date.class));
+
+ assertTrue(result);
+ }
}
