bhouse-nexthop commented on code in PR #14207:
URL: https://github.com/apache/cloudstack/pull/14207#discussion_r4208617505
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -5452,11 +5468,81 @@ private void
handlePowerOnReportWithNoPendingJobsOnVM(final VMInstanceVO vm) {
case Error:
default:
logger.info("Receive power on report when Instance is in error or
unexpected state. Instance: {}, state: {}.", vm, vm.getState());
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SYNC,
vm.getDataCenterId(), vm.getPodIdToDeployIn(),
+ VM_SYNC_ALERT_SUBJECT, String.format("Instance %s is
reported running by host %s but is in %s state. "
+ + "It is not managed by CloudStack and may need to
be stopped on the host.",
+ vm.getInstanceName(), vm.getPowerHostId(),
vm.getState()));
break;
}
}
- private void handlePowerOffReportWithNoPendingJobsOnVM(final VMInstanceVO
vm) {
+ /**
+ * Send a StopCommand for an instance to the last host it is known to have
run on, whatever the database state
+ * says. Used before expunging, where the instance's addresses and volumes
are about to be released and a domain
+ * left running on the host would keep using them.
+ *
+ * External instances are skipped: their teardown is done by their
extension in finalizeExpunge, and a
+ * StopCommand issued from here would not carry the details that path
needs.
+ */
+ protected void ensureInstanceIsStoppedOnLastKnownHost(final VMInstanceVO
vm) {
+ if (vm == null ||
HypervisorType.External.equals(vm.getHypervisorType())) {
+ return;
+ }
+ final Long hostId = vm.getHostId() != null ? vm.getHostId() :
vm.getLastHostId();
+ if (hostId == null) {
+ return;
+ }
+ try {
+ // checkBeforeCleanup is false on purpose: a running domain is
what has to be removed here.
+ final StopCommand stop = buildStopCommand(vm, new
VirtualMachineProfileImpl(vm), false);
+ final Answer answer = _agentMgr.send(hostId, stop);
+ if (answer != null && answer.getResult()) {
+ return;
+ }
+ logger.warn("Unable to confirm instance {} is stopped on host {}
before expunging: {}", vm, hostId,
+ answer == null ? "no answer from host" :
answer.getDetails());
+ } catch (final AgentUnavailableException | OperationTimedoutException
e) {
+ logger.warn("Unable to confirm instance {} is stopped on host {}
before expunging.", vm, hostId, e);
+ }
+ }
+
+ /**
+ * The host reports an instance as powered on that the database considers
destroyed or expunged. It will never be
+ * managed again, and its addresses and storage have already been handed
back, so stop it on the host that
+ * reported it instead of leaving it running unmanaged.
+ *
+ * External instances are skipped, as in
ensureInstanceIsStoppedOnLastKnownHost().
+ */
+ protected void stopUnmanagedInstanceOnReportingHost(final VMInstanceVO vm)
{
+ if (HypervisorType.External.equals(vm.getHypervisorType())) {
+ return;
+ }
+ final Long powerHostId = vm.getPowerHostId();
+ if (powerHostId == null) {
+ logger.warn("Instance {} is reported powered on but no reporting
host is recorded, cannot stop it.", vm);
+ return;
+ }
+ try {
+ // checkBeforeCleanup must be false: the instance is known to be
running, and that is exactly what
+ // has to be stopped. With it set, the host would refuse and
answer "vm is still running on host".
Review Comment:
Went with the wrapper in 92b8d1d0dd: `buildUnconditionalStopCommand(vm)`.
This call site and the one in `ensureInstanceIsStoppedOnLastKnownHost()`
carried the same explanation, so it now lives once in the helper's javadoc and
neither call site needs a comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
