Skip to content

Commit b01e74e

Browse files
vishesh92vishesh92
committed
Minor fixups
1 parent 785c064 commit b01e74e

File tree

28 files changed

+183
-141
lines changed

28 files changed

+183
-141
lines changed

‎engine/components-api/src/main/java/com/cloud/network/rules/RulesManager.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.cloud.exception.NetworkRuleConflictException;
2323
import com.cloud.exception.ResourceUnavailableException;
2424
import com.cloud.network.IpAddress;
25+
import com.cloud.network.Network;
2526
import com.cloud.user.Account;
2627
import com.cloud.uservm.UserVm;
2728
import com.cloud.vm.Nic;
@@ -47,7 +48,7 @@ public interface RulesManager extends RulesService {
4748
FirewallRule[] reservePorts(IpAddress ip, String protocol, FirewallRule.Purpose purpose, boolean openFirewall, Account caller, int... ports)
4849
throws NetworkRuleConflictException;
4950

50-
boolean applyStaticNatsForNetwork(long networkId, boolean continueOnError, Account caller);
51+
boolean applyStaticNatsForNetwork(Network network, boolean continueOnError, Account caller);
5152

5253
void getSystemIpAndEnableStaticNatForVm(VirtualMachine vm, boolean getNewIp) throws InsufficientAddressCapacityException;
5354

@@ -60,7 +61,7 @@ FirewallRule[] reservePorts(IpAddress ip, String protocol, FirewallRule.Purpose
6061
* @param forRevoke
6162
* @return
6263
*/
63-
boolean applyStaticNatForNetwork(long networkId, boolean continueOnError, Account caller, boolean forRevoke);
64+
boolean applyStaticNatForNetwork(Network network, boolean continueOnError, Account caller, boolean forRevoke);
6465

6566
List<FirewallRuleVO> listAssociatedRulesForGuestNic(Nic nic);
6667

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3838,7 +3838,7 @@ public void processConnect(final Host agent, final StartupCommand cmd, final boo
38383838

38393839
logger.debug("Received startup command from hypervisor host. host: {}", agent);
38403840

3841-
_syncMgr.resetHostSyncState(agent.getId());
3841+
_syncMgr.resetHostSyncState(agent);
38423842

38433843
if (forRebalance) {
38443844
logger.debug("Not processing listener {} as connect happens on rebalance process", this);
@@ -5795,18 +5795,20 @@ private Pair<Long, Long> findClusterAndHostIdForVmFromVolumes(long vmId) {
57955795
@Override
57965796
public Pair<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm, boolean skipCurrentHostForStartingVm) {
57975797
Long hostId = null;
5798+
Host host = null;
57985799
if (!skipCurrentHostForStartingVm || !State.Starting.equals(vm.getState())) {
57995800
hostId = vm.getHostId();
58005801
}
58015802
Long clusterId = null;
58025803
if(hostId == null) {
5804+
if (vm.getLastHostId() == null) {
5805+
return findClusterAndHostIdForVmFromVolumes(vm.getId());
5806+
}
58035807
hostId = vm.getLastHostId();
5804-
logger.debug("host id is null, using last host id {}", hostId);
5805-
}
5806-
if (hostId == null) {
5807-
return findClusterAndHostIdForVmFromVolumes(vm.getId());
5808+
host = _hostDao.findById(hostId);
5809+
logger.debug("host id is null, using last host {} with id {}", host, hostId);
58085810
}
5809-
HostVO host = _hostDao.findById(hostId);
5811+
host = host == null ? _hostDao.findById(hostId) : host;
58105812
if (host != null) {
58115813
clusterId = host.getClusterId();
58125814
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,14 @@
1919
import java.util.Map;
2020

2121
import com.cloud.agent.api.HostVmStateReportEntry;
22+
import com.cloud.host.Host;
2223

2324
public interface VirtualMachinePowerStateSync {
2425

25-
void resetHostSyncState(long hostId);
26+
void resetHostSyncState(Host hostId);
2627

2728
void processHostVmStateReport(long hostId, Map<String, HostVmStateReportEntry> report);
2829

2930
// to adapt legacy ping report
3031
void processHostVmStatePingReport(long hostId, Map<String, HostVmStateReportEntry> report, boolean force);
31-
32-
Map<Long, VirtualMachine.PowerState> convertVmStateReport(Map<String, HostVmStateReportEntry> states);
3332
}

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

Lines changed: 41 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@
2424

2525
import javax.inject.Inject;
2626

27+
import com.cloud.host.Host;
28+
import com.cloud.host.HostVO;
29+
import com.cloud.host.dao.HostDao;
30+
import com.cloud.utils.Pair;
2731
import org.apache.cloudstack.framework.messagebus.MessageBus;
2832
import org.apache.cloudstack.framework.messagebus.PublishScope;
2933
import org.apache.logging.log4j.Logger;
@@ -40,54 +44,57 @@ public class VirtualMachinePowerStateSyncImpl implements VirtualMachinePowerStat
4044

4145
@Inject MessageBus _messageBus;
4246
@Inject VMInstanceDao _instanceDao;
47+
@Inject HostDao hostDao;
4348
@Inject ManagementServiceConfiguration mgmtServiceConf;
4449

4550
public VirtualMachinePowerStateSyncImpl() {
4651
}
4752

4853
@Override
49-
public void resetHostSyncState(long hostId) {
50-
logger.info("Reset VM power state sync for host: {}.", hostId);
51-
_instanceDao.resetHostPowerStateTracking(hostId);
54+
public void resetHostSyncState(Host host) {
55+
logger.info("Reset VM power state sync for host: {}", host);
56+
_instanceDao.resetHostPowerStateTracking(host.getId());
5257
}
5358

5459
@Override
5560
public void processHostVmStateReport(long hostId, Map<String, HostVmStateReportEntry> report) {
56-
logger.debug("Process host VM state report. host: {}.", hostId);
61+
HostVO host = hostDao.findById(hostId);
62+
logger.debug("Process host VM state report. host: {}", host);
5763

58-
Map<Long, VirtualMachine.PowerState> translatedInfo = convertVmStateReport(report);
59-
processReport(hostId, translatedInfo, false);
64+
Map<Long, Pair<VirtualMachine.PowerState, VMInstanceVO>> translatedInfo = convertVmStateReport(report);
65+
processReport(host, translatedInfo, false);
6066
}
6167

6268
@Override
6369
public void processHostVmStatePingReport(long hostId, Map<String, HostVmStateReportEntry> report, boolean force) {
64-
logger.debug("Process host VM state report from ping process. host: {}.", hostId);
70+
HostVO host = hostDao.findById(hostId);
71+
logger.debug("Process host VM state report from ping process. host: {}", host);
6572

66-
Map<Long, VirtualMachine.PowerState> translatedInfo = convertVmStateReport(report);
67-
processReport(hostId, translatedInfo, force);
73+
Map<Long, Pair<VirtualMachine.PowerState, VMInstanceVO>> translatedInfo = convertVmStateReport(report);
74+
processReport(host, translatedInfo, force);
6875
}
6976

70-
private void processReport(long hostId, Map<Long, VirtualMachine.PowerState> translatedInfo, boolean force) {
77+
private void processReport(HostVO host, Map<Long, Pair<VirtualMachine.PowerState, VMInstanceVO>> translatedInfo, boolean force) {
7178

72-
logger.debug("Process VM state report. host: {}, number of records in report: {}.", hostId, translatedInfo.size());
79+
logger.debug("Process VM state report. host: {}, number of records in report: {}.", host, translatedInfo.size());
7380

74-
for (Map.Entry<Long, VirtualMachine.PowerState> entry : translatedInfo.entrySet()) {
81+
for (Map.Entry<Long, Pair<VirtualMachine.PowerState, VMInstanceVO>> entry : translatedInfo.entrySet()) {
7582

76-
logger.debug("VM state report. host: {}, vm id: {}, power state: {}.", hostId, entry.getKey(), entry.getValue());
83+
logger.debug("VM state report. host: {}, vm: {}, power state: {}", host, entry.getValue().second(), entry.getValue().first());
7784

78-
if (_instanceDao.updatePowerState(entry.getKey(), hostId, entry.getValue(), DateUtil.currentGMTTime())) {
79-
logger.debug("VM state report is updated. host: {}, vm id: {}, power state: {}.", hostId, entry.getKey(), entry.getValue());
85+
if (_instanceDao.updatePowerState(entry.getKey(), host.getId(), entry.getValue().first(), DateUtil.currentGMTTime())) {
86+
logger.debug("VM state report is updated. host: {}, vm: {}, power state: {}", host, entry.getValue().second(), entry.getValue().first());
8087

8188
_messageBus.publish(null, VirtualMachineManager.Topics.VM_POWER_STATE, PublishScope.GLOBAL, entry.getKey());
8289
} else {
83-
logger.trace("VM power state does not change, skip DB writing. vm id: {}.", entry.getKey());
90+
logger.trace("VM power state does not change, skip DB writing. vm: {}", entry.getValue().second());
8491
}
8592
}
8693

8794
// any state outdates should be checked against the time before this list was retrieved
8895
Date startTime = DateUtil.currentGMTTime();
8996
// for all running/stopping VMs, we provide monitoring of missing report
90-
List<VMInstanceVO> vmsThatAreMissingReport = _instanceDao.findByHostInStates(hostId, VirtualMachine.State.Running,
97+
List<VMInstanceVO> vmsThatAreMissingReport = _instanceDao.findByHostInStates(host.getId(), VirtualMachine.State.Running,
9198
VirtualMachine.State.Stopping, VirtualMachine.State.Starting);
9299
java.util.Iterator<VMInstanceVO> it = vmsThatAreMissingReport.iterator();
93100
while (it.hasNext()) {
@@ -99,7 +106,7 @@ private void processReport(long hostId, Map<Long, VirtualMachine.PowerState> tra
99106
// here we need to be wary of out of band migration as opposed to other, more unexpected state changes
100107
if (vmsThatAreMissingReport.size() > 0) {
101108
Date currentTime = DateUtil.currentGMTTime();
102-
logger.debug("Run missing VM report. current time: {}", currentTime.getTime());
109+
logger.debug("Run missing VM report for host {}. current time: {}", host, currentTime.getTime());
103110

104111
// 2 times of sync-update interval for graceful period
105112
long milliSecondsGracefullPeriod = mgmtServiceConf.getPingInterval() * 2000L;
@@ -109,70 +116,65 @@ private void processReport(long hostId, Map<Long, VirtualMachine.PowerState> tra
109116
// Make sure powerState is up to date for missing VMs
110117
try {
111118
if (!force && !_instanceDao.isPowerStateUpToDate(instance.getId())) {
112-
logger.warn("Detected missing VM but power state is outdated, wait for another process report run for VM id: {}.", instance.getId());
119+
logger.warn("Detected missing VM but power state is outdated, wait for another process report run for VM: {}", instance);
113120
_instanceDao.resetVmPowerStateTracking(instance.getId());
114121
continue;
115122
}
116123
} catch (CloudRuntimeException e) {
117-
logger.warn("Checked for missing powerstate of a none existing vm", e);
124+
logger.warn("Checked for missing powerstate of a none existing vm {}", instance, e);
118125
continue;
119126
}
120127

121128
Date vmStateUpdateTime = instance.getPowerStateUpdateTime();
122129
if (vmStateUpdateTime == null) {
123-
logger.warn("VM power state update time is null, falling back to update time for vm id: {}.", instance.getId());
130+
logger.warn("VM power state update time is null, falling back to update time for vm: {}", instance);
124131
vmStateUpdateTime = instance.getUpdateTime();
125132
if (vmStateUpdateTime == null) {
126-
logger.warn("VM update time is null, falling back to creation time for vm id: {}", instance.getId());
133+
logger.warn("VM update time is null, falling back to creation time for vm: {}", instance);
127134
vmStateUpdateTime = instance.getCreated();
128135
}
129136
}
130137

131138
String lastTime = new SimpleDateFormat("yyyy/MM/dd'T'HH:mm:ss.SSS'Z'").format(vmStateUpdateTime);
132-
logger.debug("Detected missing VM. host: {}, vm id: {}({}), power state: {}, last state update: {}"
133-
, hostId
134-
, instance.getId()
135-
, instance.getUuid()
136-
, VirtualMachine.PowerState.PowerReportMissing
137-
, lastTime);
139+
logger.debug("Detected missing VM. host: {}, vm: {}, power state: {}, last state update: {}",
140+
host, instance, VirtualMachine.PowerState.PowerReportMissing, lastTime);
138141

139142
long milliSecondsSinceLastStateUpdate = currentTime.getTime() - vmStateUpdateTime.getTime();
140143

141144
if (force || milliSecondsSinceLastStateUpdate > milliSecondsGracefullPeriod) {
142-
logger.debug("vm id: {} - time since last state update({}ms) has passed graceful period.", instance.getId(), milliSecondsSinceLastStateUpdate);
145+
logger.debug("vm: {} - time since last state update({}ms) has passed graceful period", instance, milliSecondsSinceLastStateUpdate);
143146

144147
// this is were a race condition might have happened if we don't re-fetch the instance;
145148
// between the startime of this job and the currentTime of this missing-branch
146149
// an update might have occurred that we should not override in case of out of band migration
147-
if (_instanceDao.updatePowerState(instance.getId(), hostId, VirtualMachine.PowerState.PowerReportMissing, startTime)) {
148-
logger.debug("VM state report is updated. host: {}, vm id: {}, power state: PowerReportMissing.", hostId, instance.getId());
150+
if (_instanceDao.updatePowerState(instance.getId(), host.getId(), VirtualMachine.PowerState.PowerReportMissing, startTime)) {
151+
logger.debug("VM state report is updated. host: {}, vm: {}, power state: PowerReportMissing ", host, instance);
149152

150153
_messageBus.publish(null, VirtualMachineManager.Topics.VM_POWER_STATE, PublishScope.GLOBAL, instance.getId());
151154
} else {
152-
logger.debug("VM power state does not change, skip DB writing. vm id: {}", instance.getId());
155+
logger.debug("VM power state does not change, skip DB writing. vm: {}", instance);
153156
}
154157
} else {
155-
logger.debug("vm id: {} - time since last state update({}ms) has not passed graceful period yet.", instance.getId(), milliSecondsSinceLastStateUpdate);
158+
logger.debug("vm: {} - time since last state update({} ms) has not passed graceful period yet", instance, milliSecondsSinceLastStateUpdate);
156159
}
157160
}
158161
}
159162

160-
logger.debug("Done with process of VM state report. host: {}", hostId);
163+
logger.debug("Done with process of VM state report. host: {}", host);
161164
}
162165

163-
@Override
164-
public Map<Long, VirtualMachine.PowerState> convertVmStateReport(Map<String, HostVmStateReportEntry> states) {
165-
final HashMap<Long, VirtualMachine.PowerState> map = new HashMap<Long, VirtualMachine.PowerState>();
166+
public Map<Long, Pair<VirtualMachine.PowerState, VMInstanceVO>> convertVmStateReport(Map<String, HostVmStateReportEntry> states) {
167+
final HashMap<Long, Pair<VirtualMachine.PowerState, VMInstanceVO>> map = new HashMap<>();
166168
if (states == null) {
167169
return map;
168170
}
169171

170172
for (Map.Entry<String, HostVmStateReportEntry> entry : states.entrySet()) {
171173
VMInstanceVO vm = findVM(entry.getKey());
172174
if (vm != null) {
173-
map.put(vm.getId(), entry.getValue().getState());
175+
map.put(vm.getId(), new Pair<>(entry.getValue().getState(), vm));
174176
} else {
175-
logger.debug("Unable to find matched VM in CloudStack DB. name: {}", entry.getKey());
177+
logger.debug("Unable to find matched VM in CloudStack DB. name: {} powerstate: {}", entry.getKey(), entry.getValue());
176178
}
177179
}
178180

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1761,7 +1761,7 @@ protected boolean reprogramNetworkRules(final long networkId, final Account call
17611761

17621762

17631763
// apply static nat
1764-
if (!_rulesMgr.applyStaticNatsForNetwork(networkId, false, caller)) {
1764+
if (!_rulesMgr.applyStaticNatsForNetwork(network, false, caller)) {
17651765
logger.warn("Failed to apply static nats a part of network {} restart", network);
17661766
success = false;
17671767
}
@@ -4215,7 +4215,7 @@ private boolean shutdownNetworkResources(final Network network, final Account ca
42154215
}
42164216

42174217
//release all static nats for the network
4218-
if (!_rulesMgr.applyStaticNatForNetwork(network.getId(), false, caller, true)) {
4218+
if (!_rulesMgr.applyStaticNatForNetwork(network, false, caller, true)) {
42194219
logger.warn("Failed to disable static nats as part of shutdownNetworkRules for network {}", network);
42204220
success = false;
42214221
}

‎engine/schema/src/main/java/com/cloud/storage/SnapshotScheduleVO.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@
3030

3131
import com.cloud.storage.snapshot.SnapshotSchedule;
3232
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
33-
import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
34-
import org.apache.commons.lang3.builder.ToStringStyle;
3533

3634
@Entity
3735
@Table(name = "snapshot_schedule")

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,13 @@
3434
import java.util.Calendar;
3535
import java.util.Date;
3636

37+
import com.cloud.host.dao.HostDao;
3738
import org.joda.time.DateTime;
3839
import org.junit.After;
3940
import org.junit.Before;
4041
import org.junit.Test;
42+
import org.junit.runner.RunWith;
43+
import org.mockito.InjectMocks;
4144
import org.mockito.Mock;
4245
import org.mockito.Mockito;
4346
import org.mockito.MockitoAnnotations;
@@ -49,19 +52,24 @@
4952
import com.cloud.utils.db.SearchCriteria;
5053
import com.cloud.vm.VMInstanceVO;
5154
import com.cloud.vm.VirtualMachine;
55+
import org.mockito.junit.MockitoJUnitRunner;
5256

5357
/**
5458
* Created by sudharma_jain on 3/2/17.
5559
*/
56-
60+
@RunWith(MockitoJUnitRunner.class)
5761
public class VMInstanceDaoImplTest {
5862

63+
@InjectMocks
5964
@Spy
60-
VMInstanceDaoImpl vmInstanceDao = new VMInstanceDaoImpl();
65+
VMInstanceDaoImpl vmInstanceDao;
6166

6267
@Mock
6368
VMInstanceVO vm;
6469

70+
@Mock
71+
HostDao _hostDao;
72+
6573
private AutoCloseable closeable;
6674

6775
@Before
@@ -111,9 +119,6 @@ public void testUpdatePowerStateDifferentPowerState() {
111119

112120
@Test
113121
public void testUpdatePowerStateVmNotFound() {
114-
when(vm.getPowerStateUpdateTime()).thenReturn(null);
115-
when(vm.getPowerHostId()).thenReturn(1L);
116-
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
117122
doReturn(null).when(vmInstanceDao).findById(anyLong());
118123

119124
boolean result = vmInstanceDao.updatePowerState(1L, 1L, VirtualMachine.PowerState.PowerOff, new Date());
@@ -154,7 +159,6 @@ public void testUpdatePowerStateNoChangeMaxUpdatesValidState() {
154159
when(vm.getPowerStateUpdateCount()).thenReturn(MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT);
155160
when(vm.getState()).thenReturn(Running);
156161
doReturn(vm).when(vmInstanceDao).findById(anyLong());
157-
doReturn(true).when(vmInstanceDao).update(anyLong(), any());
158162

159163
boolean result = vmInstanceDao.updatePowerState(1L, 1L, VirtualMachine.PowerState.PowerOn, new Date());
160164

@@ -170,8 +174,8 @@ public void testUpdatePowerStateNoChangeMaxUpdatesValidState() {
170174
public void testUpdatePowerStateNoChangeMaxUpdatesInvalidStateVmStopped() {
171175
when(vm.getPowerStateUpdateTime()).thenReturn(null);
172176
when(vm.getPowerHostId()).thenReturn(1L);
177+
when(vm.getHostId()).thenReturn(1L);
173178
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOn);
174-
when(vm.getPowerStateUpdateCount()).thenReturn(MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT);
175179
when(vm.getState()).thenReturn(Stopped);
176180
doReturn(vm).when(vmInstanceDao).findById(anyLong());
177181
doReturn(true).when(vmInstanceDao).update(anyLong(), any());
@@ -190,8 +194,8 @@ public void testUpdatePowerStateNoChangeMaxUpdatesInvalidStateVmStopped() {
190194
public void testUpdatePowerStateNoChangeMaxUpdatesInvalidStateVmRunning() {
191195
when(vm.getPowerStateUpdateTime()).thenReturn(null);
192196
when(vm.getPowerHostId()).thenReturn(1L);
197+
when(vm.getHostId()).thenReturn(1L);
193198
when(vm.getPowerState()).thenReturn(VirtualMachine.PowerState.PowerOff);
194-
when(vm.getPowerStateUpdateCount()).thenReturn(MAX_CONSECUTIVE_SAME_STATE_UPDATE_COUNT);
195199
when(vm.getState()).thenReturn(Running);
196200
doReturn(vm).when(vmInstanceDao).findById(anyLong());
197201
doReturn(true).when(vmInstanceDao).update(anyLong(), any());

0 commit comments

Comments
 (0)