[ 
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)

Reply via email to