From f5f2b5276f0519b846a9070b23dbdfafa3f3f770 Mon Sep 17 00:00:00 2001 From: Rakesh Venkatesh Date: Wed, 5 Aug 2020 13:24:03 +0200 Subject: [PATCH 1/4] Dont add host back after cloudstack-agent service restart If a host is removed from cloudstack, it will be added back if we restart the agent service on the host. It should not be added back if manualy removed. Add a global setting "add.host.on.service.restart" with default value of true. If set to false, the zoneid, clusterid and podid in agent.properties will be set to null so that when service is restarted, the agent is not added back If true these values will not be set to null so that it can be still added back on service restart --- agent/src/main/java/com/cloud/agent/Agent.java | 12 ++++++++++++ .../java/com/cloud/agent/api/ShutdownCommand.java | 12 ++++++++++++ .../configuration/ConfigurationManagerImpl.java | 7 ++++++- .../kvm/discoverer/LibvirtServerDiscoverer.java | 6 +++++- 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/Agent.java b/agent/src/main/java/com/cloud/agent/Agent.java index 1f2f4859384e..e09d3ba4cea0 100644 --- a/agent/src/main/java/com/cloud/agent/Agent.java +++ b/agent/src/main/java/com/cloud/agent/Agent.java @@ -420,6 +420,15 @@ protected void cancelTasks() { } } + protected void cleanupAgentZoneProperties() { + // Unset zone, cluster and pod values so that host is not added back + // when service is restarted. This will be set to proper values + // when host is added back + _shell.setPersistentProperty(null, "zone", ""); + _shell.setPersistentProperty(null, "cluster", ""); + _shell.setPersistentProperty(null, "pod", ""); + } + public synchronized void lockStartupTask(final Link link) { _startup = new StartupTask(link); _timer.schedule(_startup, _startupWait); @@ -603,6 +612,9 @@ protected void processRequest(final Request request, final Link link) { final ShutdownCommand shutdown = (ShutdownCommand)cmd; s_logger.debug("Received shutdownCommand, due to: " + shutdown.getReason()); cancelTasks(); + if (shutdown.isRemoveHost()) { + cleanupAgentZoneProperties(); + } _reconnectAllowed = false; answer = new Answer(cmd, true, null); } else if (cmd instanceof ReadyCommand && ((ReadyCommand)cmd).getDetails() != null) { diff --git a/core/src/main/java/com/cloud/agent/api/ShutdownCommand.java b/core/src/main/java/com/cloud/agent/api/ShutdownCommand.java index 3c0571c156c3..d36621d0ff16 100644 --- a/core/src/main/java/com/cloud/agent/api/ShutdownCommand.java +++ b/core/src/main/java/com/cloud/agent/api/ShutdownCommand.java @@ -30,6 +30,7 @@ public class ShutdownCommand extends Command { private String reason; private String detail; + private boolean removeHost; protected ShutdownCommand() { super(); @@ -41,6 +42,13 @@ public ShutdownCommand(String reason, String detail) { this.detail = detail; } + public ShutdownCommand(String reason, String detail, boolean removeHost) { + super(); + this.reason = reason; + this.detail = detail; + this.removeHost = removeHost; + } + /** * @return return the reason the agent shutdown. If Unknown, call getDetail() for any details. */ @@ -52,6 +60,10 @@ public String getDetail() { return detail; } + public boolean isRemoveHost() { + return removeHost; + } + @Override public boolean executeInSequence() { return true; diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 7e9c9d39c2b1..4a6071a4c3ef 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -410,6 +410,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public final static ConfigKey IOPS_MAX_WRITE_LENGTH = new ConfigKey(Long.class, "vm.disk.iops.maximum.write.length", "Advanced", "0", "Maximum IOPS write burst duration (seconds). If '0' (zero) then does not check for maximum burst length.", true, ConfigKey.Scope.Global, null); + public static final ConfigKey AddHostOnServiceRestart = new ConfigKey(Boolean.class, "add.host.on.service.restart", "Advanced", "true", + "Indicates whether the host will be added back to cloudstack after restarting agent service on host. If false it wont be added back even after service restart", + true, ConfigKey.Scope.Global, null); + private static final String IOPS_READ_RATE = "IOPS Read"; private static final String IOPS_WRITE_RATE = "IOPS Write"; private static final String BYTES_READ_RATE = "Bytes Read"; @@ -6378,6 +6382,7 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {SystemVMUseLocalStorage, IOPS_MAX_READ_LENGTH, IOPS_MAX_WRITE_LENGTH, BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH}; + return new ConfigKey[] {SystemVMUseLocalStorage, IOPS_MAX_READ_LENGTH, IOPS_MAX_WRITE_LENGTH, + BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, AddHostOnServiceRestart}; } } diff --git a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java index 904a488c1245..6bb0af6e0e26 100644 --- a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java +++ b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java @@ -67,6 +67,8 @@ import com.cloud.utils.ssh.SSHCmdHelper; import com.trilead.ssh2.Connection; +import static com.cloud.configuration.ConfigurationManagerImpl.AddHostOnServiceRestart; + public abstract class LibvirtServerDiscoverer extends DiscovererBase implements Discoverer, Listener, ResourceStateAdapter { private static final Logger s_logger = Logger.getLogger(LibvirtServerDiscoverer.class); private final int _waitTime = 5; /* wait for 5 minutes */ @@ -348,6 +350,7 @@ private void setupAgentSecurity(final Connection sshConnection, final String age _hostDao.saveDetails(connectedHost); return resources; } catch (DiscoveredWithErrorException e) { + s_logger.error("DiscoveredWithErrorException caught and rethrowing, message: "+ e.getMessage()); throw e; } catch (Exception e) { String msg = " can't setup agent, due to " + e.toString() + " - " + e.getMessage(); @@ -474,7 +477,8 @@ public DeleteHostAnswer deleteHost(HostVO host, boolean isForced, boolean isForc _resourceMgr.deleteRoutingHost(host, isForced, isForceDeleteStorage); try { - ShutdownCommand cmd = new ShutdownCommand(ShutdownCommand.DeleteHost, null); + ShutdownCommand cmd = AddHostOnServiceRestart.value() ? new ShutdownCommand(ShutdownCommand.DeleteHost, null, false) : + new ShutdownCommand(ShutdownCommand.DeleteHost, "Cleaning up zone/pod/cluster details for host", true); agentMgr.send(host.getId(), cmd); } catch (AgentUnavailableException e) { s_logger.warn("Sending ShutdownCommand failed: ", e); From 9d00a6a3d809d286f0fba180d13b0a6abee3b8a0 Mon Sep 17 00:00:00 2001 From: Rakesh Venkatesh Date: Wed, 5 Aug 2020 15:28:37 +0200 Subject: [PATCH 2/4] Simplify ternary operator --- .../hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java index 6bb0af6e0e26..047ca07ea3a5 100644 --- a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java +++ b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java @@ -477,8 +477,7 @@ public DeleteHostAnswer deleteHost(HostVO host, boolean isForced, boolean isForc _resourceMgr.deleteRoutingHost(host, isForced, isForceDeleteStorage); try { - ShutdownCommand cmd = AddHostOnServiceRestart.value() ? new ShutdownCommand(ShutdownCommand.DeleteHost, null, false) : - new ShutdownCommand(ShutdownCommand.DeleteHost, "Cleaning up zone/pod/cluster details for host", true); + ShutdownCommand cmd = new ShutdownCommand(ShutdownCommand.DeleteHost, null, !AddHostOnServiceRestart.value()); agentMgr.send(host.getId(), cmd); } catch (AgentUnavailableException e) { s_logger.warn("Sending ShutdownCommand failed: ", e); From e6eeb0b06b6bbe1bbc47983435057f53d5630a19 Mon Sep 17 00:00:00 2001 From: Rakesh Venkatesh Date: Tue, 11 Aug 2020 10:24:02 +0200 Subject: [PATCH 3/4] Javadoc, naming convention --- agent/src/main/java/com/cloud/agent/Agent.java | 10 +++++++--- .../cloud/configuration/ConfigurationManagerImpl.java | 4 ++-- .../kvm/discoverer/LibvirtServerDiscoverer.java | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/Agent.java b/agent/src/main/java/com/cloud/agent/Agent.java index e09d3ba4cea0..144f7efb687d 100644 --- a/agent/src/main/java/com/cloud/agent/Agent.java +++ b/agent/src/main/java/com/cloud/agent/Agent.java @@ -420,10 +420,14 @@ protected void cancelTasks() { } } + /** + * Cleanup agent zone properties. + * + * Unset zone, cluster and pod values so that host is not added back + * when service is restarted. This will be set to proper values + * when host is added back + */ protected void cleanupAgentZoneProperties() { - // Unset zone, cluster and pod values so that host is not added back - // when service is restarted. This will be set to proper values - // when host is added back _shell.setPersistentProperty(null, "zone", ""); _shell.setPersistentProperty(null, "cluster", ""); _shell.setPersistentProperty(null, "pod", ""); diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 4a6071a4c3ef..31bf32de26f9 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -410,7 +410,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public final static ConfigKey IOPS_MAX_WRITE_LENGTH = new ConfigKey(Long.class, "vm.disk.iops.maximum.write.length", "Advanced", "0", "Maximum IOPS write burst duration (seconds). If '0' (zero) then does not check for maximum burst length.", true, ConfigKey.Scope.Global, null); - public static final ConfigKey AddHostOnServiceRestart = new ConfigKey(Boolean.class, "add.host.on.service.restart", "Advanced", "true", + public static final ConfigKey ADD_HOST_ON_SERVICE_RESTART = new ConfigKey(Boolean.class, "add.host.on.service.restart", "Advanced", "true", "Indicates whether the host will be added back to cloudstack after restarting agent service on host. If false it wont be added back even after service restart", true, ConfigKey.Scope.Global, null); @@ -6383,6 +6383,6 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] {SystemVMUseLocalStorage, IOPS_MAX_READ_LENGTH, IOPS_MAX_WRITE_LENGTH, - BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, AddHostOnServiceRestart}; + BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART}; } } diff --git a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java index 047ca07ea3a5..d031620cab19 100644 --- a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java +++ b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java @@ -67,7 +67,7 @@ import com.cloud.utils.ssh.SSHCmdHelper; import com.trilead.ssh2.Connection; -import static com.cloud.configuration.ConfigurationManagerImpl.AddHostOnServiceRestart; +import static com.cloud.configuration.ConfigurationManagerImpl.ADD_HOST_ON_SERVICE_RESTART; public abstract class LibvirtServerDiscoverer extends DiscovererBase implements Discoverer, Listener, ResourceStateAdapter { private static final Logger s_logger = Logger.getLogger(LibvirtServerDiscoverer.class); @@ -477,7 +477,7 @@ public DeleteHostAnswer deleteHost(HostVO host, boolean isForced, boolean isForc _resourceMgr.deleteRoutingHost(host, isForced, isForceDeleteStorage); try { - ShutdownCommand cmd = new ShutdownCommand(ShutdownCommand.DeleteHost, null, !AddHostOnServiceRestart.value()); + ShutdownCommand cmd = new ShutdownCommand(ShutdownCommand.DeleteHost, null, !ADD_HOST_ON_SERVICE_RESTART.value()); agentMgr.send(host.getId(), cmd); } catch (AgentUnavailableException e) { s_logger.warn("Sending ShutdownCommand failed: ", e); From 33e636d65675a0f99684cbf5d6caa67503be68ec Mon Sep 17 00:00:00 2001 From: Rakesh Venkatesh Date: Wed, 2 Sep 2020 14:34:13 +0200 Subject: [PATCH 4/4] Make the setting kvm specific --- .../com/cloud/configuration/ConfigurationManagerImpl.java | 4 ++-- .../hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 31bf32de26f9..2a3873d1aaa3 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -410,7 +410,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public final static ConfigKey IOPS_MAX_WRITE_LENGTH = new ConfigKey(Long.class, "vm.disk.iops.maximum.write.length", "Advanced", "0", "Maximum IOPS write burst duration (seconds). If '0' (zero) then does not check for maximum burst length.", true, ConfigKey.Scope.Global, null); - public static final ConfigKey ADD_HOST_ON_SERVICE_RESTART = new ConfigKey(Boolean.class, "add.host.on.service.restart", "Advanced", "true", + public static final ConfigKey ADD_HOST_ON_SERVICE_RESTART_KVM = new ConfigKey(Boolean.class, "add.host.on.service.restart.kvm", "Advanced", "true", "Indicates whether the host will be added back to cloudstack after restarting agent service on host. If false it wont be added back even after service restart", true, ConfigKey.Scope.Global, null); @@ -6383,6 +6383,6 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] {SystemVMUseLocalStorage, IOPS_MAX_READ_LENGTH, IOPS_MAX_WRITE_LENGTH, - BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART}; + BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART_KVM}; } } diff --git a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java index d031620cab19..eaa5fea183b4 100644 --- a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java +++ b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java @@ -67,7 +67,7 @@ import com.cloud.utils.ssh.SSHCmdHelper; import com.trilead.ssh2.Connection; -import static com.cloud.configuration.ConfigurationManagerImpl.ADD_HOST_ON_SERVICE_RESTART; +import static com.cloud.configuration.ConfigurationManagerImpl.ADD_HOST_ON_SERVICE_RESTART_KVM; public abstract class LibvirtServerDiscoverer extends DiscovererBase implements Discoverer, Listener, ResourceStateAdapter { private static final Logger s_logger = Logger.getLogger(LibvirtServerDiscoverer.class); @@ -477,7 +477,7 @@ public DeleteHostAnswer deleteHost(HostVO host, boolean isForced, boolean isForc _resourceMgr.deleteRoutingHost(host, isForced, isForceDeleteStorage); try { - ShutdownCommand cmd = new ShutdownCommand(ShutdownCommand.DeleteHost, null, !ADD_HOST_ON_SERVICE_RESTART.value()); + ShutdownCommand cmd = new ShutdownCommand(ShutdownCommand.DeleteHost, null, !ADD_HOST_ON_SERVICE_RESTART_KVM.value()); agentMgr.send(host.getId(), cmd); } catch (AgentUnavailableException e) { s_logger.warn("Sending ShutdownCommand failed: ", e);