Skip to content

Commit 1a11311

Browse files
vishesh92sureshanaparti
vishesh92
and
sureshanaparti
authored
Fixup vm powerstate update (apache#8545)
Co-authored-by: Suresh Kumar Anaparti <[email protected]>
1 parent 275abaf commit 1a11311

File tree

3 files changed

+161
-21
lines changed

3 files changed

+161
-21
lines changed

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,8 @@
213213
import com.cloud.service.dao.ServiceOfferingDao;
214214
import com.cloud.storage.DiskOfferingVO;
215215
import com.cloud.storage.ScopeType;
216-
import com.cloud.storage.Storage.ImageFormat;
217216
import com.cloud.storage.Storage;
217+
import com.cloud.storage.Storage.ImageFormat;
218218
import com.cloud.storage.StorageManager;
219219
import com.cloud.storage.StoragePool;
220220
import com.cloud.storage.VMTemplateVO;
@@ -2208,9 +2208,11 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl
22082208

22092209
boolean result = stateTransitTo(vm, Event.OperationSucceeded, null);
22102210
if (result) {
2211+
vm.setPowerState(PowerState.PowerOff);
2212+
_vmDao.update(vm.getId(), vm);
22112213
if (VirtualMachine.Type.User.equals(vm.type) && ResourceCountRunningVMsonly.value()) {
22122214
ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId());
2213-
resourceCountDecrement(vm.getAccountId(),new Long(offering.getCpu()), new Long(offering.getRamSize()));
2215+
resourceCountDecrement(vm.getAccountId(), offering.getCpu().longValue(), offering.getRamSize().longValue());
22142216
}
22152217
} else {
22162218
throw new CloudRuntimeException("unable to stop " + vm);
@@ -2761,6 +2763,7 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
27612763
}
27622764

27632765
vm.setLastHostId(srcHostId);
2766+
_vmDao.resetVmPowerStateTracking(vm.getId());
27642767
try {
27652768
if (vm.getHostId() == null || vm.getHostId() != srcHostId || !changeState(vm, Event.MigrationRequested, dstHostId, work, Step.Migrating)) {
27662769
_networkMgr.rollbackNicForMigration(vmSrc, profile);

‎engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java

+18-5
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
public class VMInstanceDaoImpl extends GenericDaoBase<VMInstanceVO, Long> implements VMInstanceDao {
6767

6868
public static final Logger s_logger = Logger.getLogger(VMInstanceDaoImpl.class);
69-
private static final int MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT = 3;
69+
static final int MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT = 3;
7070

7171
protected SearchBuilder<VMInstanceVO> VMClusterSearch;
7272
protected SearchBuilder<VMInstanceVO> LHVMClusterSearch;
@@ -897,17 +897,19 @@ public List<VMInstanceVO> listStartingWithNoHostId() {
897897

898898
@Override
899899
public boolean updatePowerState(final long instanceId, final long powerHostId, final VirtualMachine.PowerState powerState, Date wisdomEra) {
900-
return Transaction.execute(new TransactionCallback<Boolean>() {
900+
return Transaction.execute(new TransactionCallback<>() {
901901
@Override
902902
public Boolean doInTransaction(TransactionStatus status) {
903903
boolean needToUpdate = false;
904904
VMInstanceVO instance = findById(instanceId);
905905
if (instance != null
906-
&& (null == instance.getPowerStateUpdateTime()
906+
&& (null == instance.getPowerStateUpdateTime()
907907
|| instance.getPowerStateUpdateTime().before(wisdomEra))) {
908908
Long savedPowerHostId = instance.getPowerHostId();
909-
if (instance.getPowerState() != powerState || savedPowerHostId == null
910-
|| savedPowerHostId.longValue() != powerHostId) {
909+
if (instance.getPowerState() != powerState
910+
|| savedPowerHostId == null
911+
|| savedPowerHostId != powerHostId
912+
|| !isPowerStateInSyncWithInstanceState(powerState, powerHostId, instance)) {
911913
instance.setPowerState(powerState);
912914
instance.setPowerHostId(powerHostId);
913915
instance.setPowerStateUpdateCount(1);
@@ -929,6 +931,17 @@ public Boolean doInTransaction(TransactionStatus status) {
929931
});
930932
}
931933

934+
private boolean isPowerStateInSyncWithInstanceState(final VirtualMachine.PowerState powerState, final long powerHostId, final VMInstanceVO instance) {
935+
State instanceState = instance.getState();
936+
if ((powerState == VirtualMachine.PowerState.PowerOff && instanceState == State.Running)
937+
|| (powerState == VirtualMachine.PowerState.PowerOn && instanceState == State.Stopped)) {
938+
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",
939+
instance.getId(), instance.getHostId(), powerHostId, instanceState, powerState));
940+
return false;
941+
}
942+
return true;
943+
}
944+
932945
@Override
933946
public boolean isPowerStateUpToDate(final long instanceId) {
934947
VMInstanceVO instance = findById(instanceId);

‎engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDaoImplTest.java

+138-14
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,32 @@
1717

1818
package com.cloud.vm.dao;
1919

20-
import com.cloud.utils.Pair;
21-
import com.cloud.vm.VirtualMachine;
20+
import static com.cloud.vm.VirtualMachine.State.Running;
21+
import static com.cloud.vm.VirtualMachine.State.Stopped;
22+
import static com.cloud.vm.dao.VMInstanceDaoImpl.MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT;
23+
import static org.junit.Assert.assertFalse;
24+
import static org.junit.Assert.assertTrue;
25+
import static org.mockito.ArgumentMatchers.any;
26+
import static org.mockito.ArgumentMatchers.anyLong;
27+
import static org.mockito.Mockito.doReturn;
28+
import static org.mockito.Mockito.never;
29+
import static org.mockito.Mockito.times;
30+
import static org.mockito.Mockito.verify;
31+
import static org.mockito.Mockito.when;
32+
33+
import java.util.Date;
34+
2235
import org.joda.time.DateTime;
2336
import org.junit.Before;
2437
import org.junit.Test;
25-
import org.junit.Assert;
2638
import org.mockito.Mock;
27-
28-
import static com.cloud.vm.VirtualMachine.State.Running;
29-
import static com.cloud.vm.VirtualMachine.State.Stopped;
30-
31-
import static org.mockito.Mockito.when;
32-
import com.cloud.vm.VMInstanceVO;
3339
import org.mockito.MockitoAnnotations;
3440
import org.mockito.Spy;
3541

42+
import com.cloud.utils.Pair;
43+
import com.cloud.vm.VMInstanceVO;
44+
import com.cloud.vm.VirtualMachine;
45+
3646
/**
3747
* Created by sudharma_jain on 3/2/17.
3848
*/
@@ -55,16 +65,130 @@ public void setUp() throws Exception {
5565
}
5666

5767
@Test
58-
public void testUpdateState() throws Exception {
68+
public void testUpdateState() {
5969
Long destHostId = null;
60-
Pair<Long, Long> opaqueMock = new Pair<Long, Long>(new Long(1), destHostId);
70+
Pair<Long, Long> opaqueMock = new Pair<>(1L, destHostId);
6171
vmInstanceDao.updateState(Stopped, VirtualMachine.Event.FollowAgentPowerOffReport, Stopped, vm , opaqueMock);
6272
}
6373

6474
@Test
65-
public void testIfStateAndHostUnchanged() throws Exception {
66-
Assert.assertEquals(vmInstanceDao.ifStateUnchanged(Stopped, Stopped, null, null), true);
67-
Assert.assertEquals(vmInstanceDao.ifStateUnchanged(Stopped, Running, null, null), false);
75+
public void testIfStateAndHostUnchanged() {
76+
assertTrue(vmInstanceDao.ifStateUnchanged(Stopped, Stopped, null, null));
77+
assertFalse(vmInstanceDao.ifStateUnchanged(Stopped, Running, null, null));
78+
}
79+
80+
@Test
81+
public void testUpdatePowerStateDifferentPowerState() {
82+
when(vm.getPowerStateUpdateTime()).thenReturn(null);
83+
when(vm.getPowerHostId()).thenReturn(1L);
84+
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
85+
doReturn(vm).when(vmInstanceDao).findById(anyLong());
86+
doReturn(true).when(vmInstanceDao).update(anyLong(), any());
87+
88+
boolean result = vmInstanceDao.updatePowerState(1L, 1L, VirtualMachine.PowerState.PowerOff, new Date());
89+
90+
verify(vm, times(1)).setPowerState(VirtualMachine.PowerState.PowerOff);
91+
verify(vm, times(1)).setPowerHostId(1L);
92+
verify(vm, times(1)).setPowerStateUpdateCount(1);
93+
verify(vm, times(1)).setPowerStateUpdateTime(any(Date.class));
94+
95+
assertTrue(result);
96+
}
97+
98+
@Test
99+
public void testUpdatePowerStateVmNotFound() {
100+
when(vm.getPowerStateUpdateTime()).thenReturn(null);
101+
when(vm.getPowerHostId()).thenReturn(1L);
102+
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
103+
doReturn(null).when(vmInstanceDao).findById(anyLong());
104+
105+
boolean result = vmInstanceDao.updatePowerState(1L, 1L, VirtualMachine.PowerState.PowerOff, new Date());
106+
107+
verify(vm, never()).setPowerState(any());
108+
verify(vm, never()).setPowerHostId(anyLong());
109+
verify(vm, never()).setPowerStateUpdateCount(any(Integer.class));
110+
verify(vm, never()).setPowerStateUpdateTime(any(Date.class));
111+
112+
assertFalse(result);
113+
}
114+
115+
@Test
116+
public void testUpdatePowerStateNoChangeFirstUpdate() {
117+
when(vm.getPowerStateUpdateTime()).thenReturn(null);
118+
when(vm.getPowerHostId()).thenReturn(1L);
119+
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
120+
when(vm.getState()).thenReturn(Running);
121+
when(vm.getPowerStateUpdateCount()).thenReturn(1);
122+
doReturn(vm).when(vmInstanceDao).findById(anyLong());
123+
doReturn(true).when(vmInstanceDao).update(anyLong(), any());
124+
125+
boolean result = vmInstanceDao.updatePowerState(1L, 1L, VirtualMachine.PowerState.PowerOn, new Date());
126+
127+
verify(vm, never()).setPowerState(any());
128+
verify(vm, never()).setPowerHostId(anyLong());
129+
verify(vm, times(1)).setPowerStateUpdateCount(2);
130+
verify(vm, times(1)).setPowerStateUpdateTime(any(Date.class));
131+
132+
assertTrue(result);
133+
}
134+
135+
@Test
136+
public void testUpdatePowerStateNoChangeMaxUpdatesValidState() {
137+
when(vm.getPowerStateUpdateTime()).thenReturn(null);
138+
when(vm.getPowerHostId()).thenReturn(1L);
139+
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
140+
when(vm.getPowerStateUpdateCount()).thenReturn(MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT);
141+
when(vm.getState()).thenReturn(Running);
142+
doReturn(vm).when(vmInstanceDao).findById(anyLong());
143+
doReturn(true).when(vmInstanceDao).update(anyLong(), any());
144+
145+
boolean result = vmInstanceDao.updatePowerState(1L, 1L, VirtualMachine.PowerState.PowerOn, new Date());
146+
147+
verify(vm, never()).setPowerState(any());
148+
verify(vm, never()).setPowerHostId(anyLong());
149+
verify(vm, never()).setPowerStateUpdateCount(any(Integer.class));
150+
verify(vm, never()).setPowerStateUpdateTime(any(Date.class));
151+
152+
assertFalse(result);
68153
}
69154

155+
@Test
156+
public void testUpdatePowerStateNoChangeMaxUpdatesInvalidStateVmStopped() {
157+
when(vm.getPowerStateUpdateTime()).thenReturn(null);
158+
when(vm.getPowerHostId()).thenReturn(1L);
159+
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
160+
when(vm.getPowerStateUpdateCount()).thenReturn(MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT);
161+
when(vm.getState()).thenReturn(Stopped);
162+
doReturn(vm).when(vmInstanceDao).findById(anyLong());
163+
doReturn(true).when(vmInstanceDao).update(anyLong(), any());
164+
165+
boolean result = vmInstanceDao.updatePowerState(1L, 1L, VirtualMachine.PowerState.PowerOn, new Date());
166+
167+
verify(vm, times(1)).setPowerState(any());
168+
verify(vm, times(1)).setPowerHostId(anyLong());
169+
verify(vm, times(1)).setPowerStateUpdateCount(1);
170+
verify(vm, times(1)).setPowerStateUpdateTime(any(Date.class));
171+
172+
assertTrue(result);
173+
}
174+
175+
@Test
176+
public void testUpdatePowerStateNoChangeMaxUpdatesInvalidStateVmRunning() {
177+
when(vm.getPowerStateUpdateTime()).thenReturn(null);
178+
when(vm.getPowerHostId()).thenReturn(1L);
179+
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOff);
180+
when(vm.getPowerStateUpdateCount()).thenReturn(MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT);
181+
when(vm.getState()).thenReturn(Running);
182+
doReturn(vm).when(vmInstanceDao).findById(anyLong());
183+
doReturn(true).when(vmInstanceDao).update(anyLong(), any());
184+
185+
boolean result = vmInstanceDao.updatePowerState(1L, 1L, VirtualMachine.PowerState.PowerOff, new Date());
186+
187+
verify(vm, times(1)).setPowerState(any());
188+
verify(vm, times(1)).setPowerHostId(anyLong());
189+
verify(vm, times(1)).setPowerStateUpdateCount(1);
190+
verify(vm, times(1)).setPowerStateUpdateTime(any(Date.class));
191+
192+
assertTrue(result);
193+
}
70194
}

0 commit comments

Comments
 (0)