[ https://issues.apache.org/jira/browse/CLOUDSTACK-10108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16312513#comment-16312513 ]
ASF GitHub Bot commented on CLOUDSTACK-10108: --------------------------------------------- rhtyd closed pull request #2292: CLOUDSTACK-10108:ConfigKey based approach for reading 'ping' configua… URL: https://github.com/apache/cloudstack/pull/2292 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/engine/orchestration/resources/META-INF/cloudstack/core/spring-engine-orchestration-core-context.xml b/engine/orchestration/resources/META-INF/cloudstack/core/spring-engine-orchestration-core-context.xml index df885b22573..3ded395bb66 100644 --- a/engine/orchestration/resources/META-INF/cloudstack/core/spring-engine-orchestration-core-context.xml +++ b/engine/orchestration/resources/META-INF/cloudstack/core/spring-engine-orchestration-core-context.xml @@ -59,6 +59,7 @@ </bean> <bean id="clusteredAgentManagerImpl" class="com.cloud.agent.manager.ClusteredAgentManagerImpl" /> + <bean id="managementServiceConfigurationImpl" class="com.cloud.configuration.ManagementServiceConfigurationImpl" /> <bean id="cloudOrchestrator" class="org.apache.cloudstack.engine.orchestration.CloudOrchestrator" /> diff --git a/engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java index 7815c76a54e..b7357756c4c 100644 --- a/engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java @@ -38,6 +38,7 @@ import javax.naming.ConfigurationException; import org.apache.cloudstack.ca.CAManager; +import com.cloud.configuration.ManagementServiceConfiguration; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; @@ -180,13 +181,12 @@ @Inject ResourceManager _resourceMgr; + @Inject + ManagementServiceConfiguration mgmtServiceConf; + protected final ConfigKey<Integer> Workers = new ConfigKey<Integer>("Advanced", Integer.class, "workers", "5", "Number of worker threads handling remote agent connections.", false); protected final ConfigKey<Integer> Port = new ConfigKey<Integer>("Advanced", Integer.class, "port", "8250", "Port to listen on for remote agent connections.", false); - protected final ConfigKey<Integer> PingInterval = new ConfigKey<Integer>("Advanced", Integer.class, "ping.interval", "60", - "Interval to send application level pings to make sure the connection is still working", false); - protected final ConfigKey<Float> PingTimeout = new ConfigKey<Float>("Advanced", Float.class, "ping.timeout", "2.5", - "Multiplier to ping.interval before announcing an agent has timed out", true); protected final ConfigKey<Integer> AlertWait = new ConfigKey<Integer>("Advanced", Integer.class, "alert.wait", "1800", "Seconds to wait before alerting on a disconnected agent", true); protected final ConfigKey<Integer> DirectAgentLoadSize = new ConfigKey<Integer>("Advanced", Integer.class, "direct.agent.load.size", "16", @@ -206,14 +206,14 @@ @Override public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException { - s_logger.info("Ping Timeout is " + PingTimeout.value()); + s_logger.info("Ping Timeout is " + mgmtServiceConf.getPingTimeout()); final int threads = DirectAgentLoadSize.value(); _nodeId = ManagementServerNode.getManagementServerId(); s_logger.info("Configuring AgentManagerImpl. management server node id(msid): " + _nodeId); - final long lastPing = (System.currentTimeMillis() >> 10) - getTimeout(); + final long lastPing = (System.currentTimeMillis() >> 10) - mgmtServiceConf.getTimeout(); _hostDao.markHostsAsDisconnected(_nodeId, lastPing); registerForHostEvents(new BehindOnPingListener(), true, true, false); @@ -241,13 +241,6 @@ public boolean configure(final String name, final Map<String, Object> params) th return true; } - protected int getPingInterval() { - return PingInterval.value(); - } - - protected long getTimeout() { - return (long) (Math.ceil(PingTimeout.value() * PingInterval.value())); - } @Override public Task create(final Task.Type type, final Link link, final byte[] data) { @@ -623,7 +616,7 @@ public boolean start() { } } - _monitorExecutor.scheduleWithFixedDelay(new MonitorTask(), getPingInterval(), getPingInterval(), TimeUnit.SECONDS); + _monitorExecutor.scheduleWithFixedDelay(new MonitorTask(), mgmtServiceConf.getPingInterval(), mgmtServiceConf.getPingInterval(), TimeUnit.SECONDS); return true; } @@ -1192,7 +1185,7 @@ protected void connectAgent(final Link link, final Command[] cmds, final Request cmd = cmds[i]; if (cmd instanceof StartupRoutingCommand || cmd instanceof StartupProxyCommand || cmd instanceof StartupSecondaryStorageCommand || cmd instanceof StartupStorageCommand) { - answers[i] = new StartupAnswer((StartupCommand) cmds[i], 0, getPingInterval()); + answers[i] = new StartupAnswer((StartupCommand) cmds[i], 0, mgmtServiceConf.getPingInterval()); break; } } @@ -1252,16 +1245,16 @@ protected void processRequest(final Link link, final Request request) { try { if (cmd instanceof StartupRoutingCommand) { final StartupRoutingCommand startup = (StartupRoutingCommand) cmd; - answer = new StartupAnswer(startup, attache.getId(), getPingInterval()); + answer = new StartupAnswer(startup, attache.getId(), mgmtServiceConf.getPingInterval()); } else if (cmd instanceof StartupProxyCommand) { final StartupProxyCommand startup = (StartupProxyCommand) cmd; - answer = new StartupAnswer(startup, attache.getId(), getPingInterval()); + answer = new StartupAnswer(startup, attache.getId(), mgmtServiceConf.getPingInterval()); } else if (cmd instanceof StartupSecondaryStorageCommand) { final StartupSecondaryStorageCommand startup = (StartupSecondaryStorageCommand) cmd; - answer = new StartupAnswer(startup, attache.getId(), getPingInterval()); + answer = new StartupAnswer(startup, attache.getId(), mgmtServiceConf.getPingInterval()); } else if (cmd instanceof StartupStorageCommand) { final StartupStorageCommand startup = (StartupStorageCommand) cmd; - answer = new StartupAnswer(startup, attache.getId(), getPingInterval()); + answer = new StartupAnswer(startup, attache.getId(), mgmtServiceConf.getPingInterval()); } else if (cmd instanceof ShutdownCommand) { final ShutdownCommand shutdown = (ShutdownCommand) cmd; final String reason = shutdown.getReason(); @@ -1515,7 +1508,7 @@ public boolean handleDirectConnectAgent(final Host host, final StartupCommand[] attache = createAttacheForDirectConnect(host, resource); final StartupAnswer[] answers = new StartupAnswer[cmds.length]; for (int i = 0; i < answers.length; i++) { - answers[i] = new StartupAnswer(cmds[i], attache.getId(), getPingInterval()); + answers[i] = new StartupAnswer(cmds[i], attache.getId(), mgmtServiceConf.getPingInterval()); } attache.process(answers); @@ -1625,7 +1618,7 @@ protected void runInContext() { protected List<Long> findAgentsBehindOnPing() { final List<Long> agentsBehind = new ArrayList<Long>(); - final long cutoffTime = InaccurateClock.getTimeInSeconds() - getTimeout(); + final long cutoffTime = InaccurateClock.getTimeInSeconds() - mgmtServiceConf.getTimeout(); for (final Map.Entry<Long, Long> entry : _pingMap.entrySet()) { if (entry.getValue() < cutoffTime) { agentsBehind.add(entry.getKey()); @@ -1714,7 +1707,7 @@ public String getConfigComponentName() { @Override public ConfigKey<?>[] getConfigKeys() { - return new ConfigKey<?>[] { CheckTxnBeforeSending, Workers, Port, PingInterval, PingTimeout, Wait, AlertWait, DirectAgentLoadSize, DirectAgentPoolSize, + return new ConfigKey<?>[] { CheckTxnBeforeSending, Workers, Port, Wait, AlertWait, DirectAgentLoadSize, DirectAgentPoolSize, DirectAgentThreadCap }; } diff --git a/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java b/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java index 0b9899eb9e0..7a9678e6a4a 100644 --- a/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java +++ b/engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java @@ -198,7 +198,7 @@ private void scanDirectAgentToLoad() { } // for agents that are self-managed, threshold to be considered as disconnected after pingtimeout - final long cutSeconds = (System.currentTimeMillis() >> 10) - getTimeout(); + final long cutSeconds = (System.currentTimeMillis() >> 10) - mgmtServiceConf.getTimeout(); final List<HostVO> hosts = _hostDao.findAndUpdateDirectAgentToLoad(cutSeconds, LoadSize.value().longValue(), _nodeId); final List<HostVO> appliances = _hostDao.findAndUpdateApplianceToLoad(cutSeconds, _nodeId); @@ -747,7 +747,7 @@ public void onManagementNodeJoined(final List<? extends ManagementServerHost> no public void onManagementNodeLeft(final List<? extends ManagementServerHost> nodeList, final long selfNodeId) { for (final ManagementServerHost vo : nodeList) { s_logger.info("Marking hosts as disconnected on Management server" + vo.getMsid()); - final long lastPing = (System.currentTimeMillis() >> 10) - getTimeout(); + final long lastPing = (System.currentTimeMillis() >> 10) - mgmtServiceConf.getTimeout(); _hostDao.markHostsAsDisconnected(vo.getMsid(), lastPing); outOfBandManagementDao.expireServerOwnership(vo.getMsid()); haConfigDao.expireServerOwnership(vo.getMsid()); diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachinePowerStateSyncImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachinePowerStateSyncImpl.java index 3b9d6f596de..60c0a994b4d 100644 --- a/engine/orchestration/src/com/cloud/vm/VirtualMachinePowerStateSyncImpl.java +++ b/engine/orchestration/src/com/cloud/vm/VirtualMachinePowerStateSyncImpl.java @@ -24,7 +24,7 @@ import javax.inject.Inject; import org.apache.log4j.Logger; -import org.apache.cloudstack.framework.config.ConfigKey; +import com.cloud.configuration.ManagementServiceConfiguration; import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.framework.messagebus.PublishScope; @@ -39,9 +39,7 @@ @Inject MessageBus _messageBus; @Inject VMInstanceDao _instanceDao; @Inject VirtualMachineManager _vmMgr; - - protected final ConfigKey<Integer> PingInterval = new ConfigKey<Integer>(Integer.class, "ping.interval", "Advanced", "60", - "Interval to send application level pings to make sure the connection is still working", false); + @Inject ManagementServiceConfiguration mgmtServiceConf; public VirtualMachinePowerStateSyncImpl() { } @@ -107,7 +105,7 @@ private void processReport(long hostId, Map<Long, VirtualMachine.PowerState> tra s_logger.debug("Run missing VM report. current time: " + currentTime.getTime()); // 2 times of sync-update interval for graceful period - long milliSecondsGracefullPeriod = PingInterval.value() * 2000L; + long milliSecondsGracefullPeriod = mgmtServiceConf.getPingInterval() * 2000L; for (VMInstanceVO instance : vmsThatAreMissingReport) { diff --git a/engine/schema/src/com/cloud/configuration/ManagementServiceConfiguration.java b/engine/schema/src/com/cloud/configuration/ManagementServiceConfiguration.java new file mode 100644 index 00000000000..51b7f62f56d --- /dev/null +++ b/engine/schema/src/com/cloud/configuration/ManagementServiceConfiguration.java @@ -0,0 +1,30 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.configuration; + +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; + +public interface ManagementServiceConfiguration extends Configurable { + ConfigKey<Integer> PingInterval = new ConfigKey<Integer>("Advanced", Integer.class, "ping.interval", "60", + "Interval to send application level pings to make sure the connection is still working", false); + ConfigKey<Float> PingTimeout = new ConfigKey<Float>("Advanced", Float.class, "ping.timeout", "2.5", + "Multiplier to ping.interval before announcing an agent has timed out", true); + public int getPingInterval(); + public float getPingTimeout(); + public long getTimeout(); +} diff --git a/engine/schema/src/com/cloud/configuration/ManagementServiceConfigurationImpl.java b/engine/schema/src/com/cloud/configuration/ManagementServiceConfigurationImpl.java new file mode 100644 index 00000000000..a827014f859 --- /dev/null +++ b/engine/schema/src/com/cloud/configuration/ManagementServiceConfigurationImpl.java @@ -0,0 +1,46 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.configuration; + +import org.apache.cloudstack.framework.config.ConfigKey; + +public class ManagementServiceConfigurationImpl implements ManagementServiceConfiguration { + @Override + public String getConfigComponentName() { + return ManagementServiceConfiguration.class.getSimpleName(); + } + + @Override + public ConfigKey<?>[] getConfigKeys() { + return new ConfigKey<?>[] {PingInterval, PingTimeout}; + } + + @Override + public int getPingInterval() { + return ManagementServiceConfiguration.PingInterval.value(); + } + + @Override + public float getPingTimeout() { + return ManagementServiceConfiguration.PingTimeout.value(); + } + + @Override + public long getTimeout() { + return (long) (PingTimeout.value() * PingInterval.value()); + } +} diff --git a/engine/schema/src/com/cloud/host/dao/HostDaoImpl.java b/engine/schema/src/com/cloud/host/dao/HostDaoImpl.java index 3335229b3d3..8c8ca8cc749 100644 --- a/engine/schema/src/com/cloud/host/dao/HostDaoImpl.java +++ b/engine/schema/src/com/cloud/host/dao/HostDaoImpl.java @@ -30,8 +30,7 @@ import javax.inject.Inject; import javax.persistence.TableGenerator; -import com.cloud.utils.NumbersUtil; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import com.cloud.configuration.ManagementServiceConfiguration; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -79,6 +78,7 @@ private static final String LIST_CLUSTERID_FOR_HOST_TAG = "select distinct cluster_id from host join host_tags on host.id = host_tags.host_id and host_tags.tag = ?"; + protected SearchBuilder<HostVO> TypePodDcStatusSearch; protected SearchBuilder<HostVO> IdStatusSearch; @@ -145,7 +145,7 @@ @Inject protected ClusterDao _clusterDao; @Inject - private ConfigurationDao _configDao; + ManagementServiceConfiguration mgmtServiceConf; public HostDaoImpl() { super(); @@ -993,9 +993,7 @@ public boolean updateState(Status oldStatus, Event event, Status newStatus, Host } } if (event.equals(Event.ManagementServerDown)) { - Float pingTimeout = NumbersUtil.parseFloat(_configDao.getValue("ping.timeout"), 2.5f); - Integer pingInterval = NumbersUtil.parseInt(_configDao.getValue("ping.interval"), 60); - ub.set(host, _pingTimeAttr, ((System.currentTimeMillis() >> 10) - (long)(pingTimeout * pingInterval))); + ub.set(host, _pingTimeAttr, ((System.currentTimeMillis() >> 10) - mgmtServiceConf.getTimeout())); } int result = update(ub, sc, null); assert result <= 1 : "How can this update " + result + " rows? "; diff --git a/server/test/resources/createNetworkOffering.xml b/server/test/resources/createNetworkOffering.xml index 9d449428943..126e265682b 100644 --- a/server/test/resources/createNetworkOffering.xml +++ b/server/test/resources/createNetworkOffering.xml @@ -41,7 +41,7 @@ <bean id="ConfigurationManager" class="com.cloud.configuration.ConfigurationManagerImpl"> <property name="name" value="ConfigurationManager"/> </bean> - + <bean id="managementServiceConfigurationImpl" class="com.cloud.configuration.ManagementServiceConfigurationImpl" /> <bean class="org.apache.cloudstack.networkoffering.ChildTestConfiguration" /> <bean id="UservmDetailsDaoImpl" class="com.cloud.vm.dao.UserVmDetailsDaoImpl" /> <bean id="hostGpuGroupsDaoImpl" class="com.cloud.gpu.dao.HostGpuGroupsDaoImpl" /> diff --git a/test/integration/component/test_host.py b/test/integration/component/test_host.py index 6a46d2ab035..c2a590a6117 100644 --- a/test/integration/component/test_host.py +++ b/test/integration/component/test_host.py @@ -98,9 +98,6 @@ def tearDown(self): return - - - def checkHostDown(self, fromHostIp, testHostIp): try: ssh = SshClient(fromHostIp, 22, "root", "password") @@ -165,9 +162,9 @@ def RestartServers(self): """ Restart management server and usage server """ sshClient = SshClient(self.mgtSvrDetails["mgtSvrIp"], - 22, - self.mgtSvrDetails["user"], - self.mgtSvrDetails["passwd"] + 22, + self.mgtSvrDetails["user"], + self.mgtSvrDetails["passwd"] ) command = "service cloudstack-management restart" sshClient.execute(command) @@ -197,8 +194,7 @@ def test_01_host_ha_with_nfs_storagepool_with_vm(self): hostToTest = listHost[0] - hostUpInCloudstack = wait_until(10, 10, self.checkHostUp, hostToTest.ipaddress, hostToTest.ipaddress) - #hostUpInCloudstack = wait_until(40, 10, self.checkHostStateInCloudstack, "Up", hostToTest.id) + hostUpInCloudstack = wait_until(40, 10, self.checkHostStateInCloudstack, "Up", hostToTest.id) if not(hostUpInCloudstack): raise self.fail("Host is not up %s, in cloudstack so failing test " % (hostToTest.ipaddress)) ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > ConfigKey based approach for reading 'ping' configuaration for Management > Server > -------------------------------------------------------------------------------- > > Key: CLOUDSTACK-10108 > URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10108 > Project: CloudStack > Issue Type: Improvement > Security Level: Public(Anyone can view this level - this is the > default.) > Reporter: mrunalini > Priority: Minor > > In CLOUDSTACK-9886, we are reading ping.interval and ping.timeout using > configdao which involves direct reading of DB. So, replace it with ConfigKey > based approach -- This message was sent by Atlassian JIRA (v6.4.14#64029)