CLOUDSTACK-10108: ConfigKey based approach for reading 'ping' configuaration (#2292)

In CLOUDSTACK-9886, we are reading ping.interval and ping.timeout using configdao which involves direct reading of DB. So, replaced it with ConfigKey based approach.

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" />

engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java

@@ -38,6 +38,7 @@ import javax.inject.Inject;
 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 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
     @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 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
     @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 class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
         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 class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
             }
         }
 
-        _monitorExecutor.scheduleWithFixedDelay(new MonitorTask(), getPingInterval(), getPingInterval(), TimeUnit.SECONDS);
+        _monitorExecutor.scheduleWithFixedDelay(new MonitorTask(), mgmtServiceConf.getPingInterval(), mgmtServiceConf.getPingInterval(), TimeUnit.SECONDS);
 
         return true;
     }
@@ -1192,7 +1185,7 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
             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 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
                 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 class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
         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 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
 
         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 class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
 
     @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 };
     }
 

engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java

@@ -198,7 +198,7 @@ public class ClusteredAgentManagerImpl extends AgentManagerImpl implements Clust
         }
 
         // 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 class ClusteredAgentManagerImpl extends AgentManagerImpl implements Clust
     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());

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

@@ -24,7 +24,7 @@ import java.util.Map;
 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 @@ public class VirtualMachinePowerStateSyncImpl implements VirtualMachinePowerStat
     @Inject MessageBus _messageBus;
     @Inject VMInstanceDao _instanceDao;
     @Inject VirtualMachineManager _vmMgr;
-
+    @Inject ManagementServiceConfiguration mgmtServiceConf;
-    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);
 
     public VirtualMachinePowerStateSyncImpl() {
     }
@@ -107,7 +105,7 @@ public class VirtualMachinePowerStateSyncImpl implements VirtualMachinePowerStat
                 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) {
 

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();
+}

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());
+    }
+}

engine/schema/src/com/cloud/host/dao/HostDaoImpl.java

@@ -30,8 +30,7 @@ import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.persistence.TableGenerator;
 
-import com.cloud.utils.NumbersUtil;
+import com.cloud.configuration.ManagementServiceConfiguration;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
@@ -79,6 +78,7 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
 
     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 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
     @Inject
     protected ClusterDao _clusterDao;
     @Inject
-    private ConfigurationDao _configDao;
+    ManagementServiceConfiguration mgmtServiceConf;
 
     public HostDaoImpl() {
         super();
@@ -993,9 +993,7 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
             }
         }
         if (event.equals(Event.ManagementServerDown)) {
-            Float pingTimeout = NumbersUtil.parseFloat(_configDao.getValue("ping.timeout"), 2.5f);
+            ub.set(host, _pingTimeAttr, ((System.currentTimeMillis() >> 10) - mgmtServiceConf.getTimeout()));
-            Integer pingInterval = NumbersUtil.parseInt(_configDao.getValue("ping.interval"), 60);
-            ub.set(host, _pingTimeAttr, ((System.currentTimeMillis() >> 10) - (long)(pingTimeout * pingInterval)));
         }
         int result = update(ub, sc, null);
         assert result <= 1 : "How can this update " + result + " rows? ";

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" />

test/integration/component/test_host.py

@@ -98,9 +98,6 @@ class TestHostHA(cloudstackTestCase):
 
         return
     
-
-
-    
     def checkHostDown(self, fromHostIp, testHostIp):
         try:
             ssh = SshClient(fromHostIp, 22, "root", "password") 
@@ -165,9 +162,9 @@ class TestHostHA(cloudstackTestCase):
         """ Restart management
         server and usage server """
         sshClient = SshClient(self.mgtSvrDetails["mgtSvrIp"],
-                    22,
+            22,
-                    self.mgtSvrDetails["user"],
+            self.mgtSvrDetails["user"],
-                    self.mgtSvrDetails["passwd"]
+            self.mgtSvrDetails["passwd"]
         )
         command = "service cloudstack-management restart"
         sshClient.execute(command)
@@ -197,8 +194,7 @@ class TestHostHA(cloudstackTestCase):
             
 
         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))