----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19038/#review37641 -----------------------------------------------------------
core/src/com/cloud/agent/api/GetRouterAlertsAnswer.java <https://reviews.apache.org/r/19038/#comment69265> What's use of routerName field here? Is it the same as NetworkElementCommand.ROUTER_NAME? core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java <https://reviews.apache.org/r/19038/#comment69270> Put it in query command rather than here. It would demand immediate answer with details rather than a simple true/false answer. See GetDomRVersionCommand. core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java <https://reviews.apache.org/r/19038/#comment69268> This comment is wrong. result.isSuccess() would only indicate the execution of scripts in the VR is success, regardless of it provides result or not. The result is still success if it provides output as well. If result is not success, it would cause a error. Though your logic seems still honor the success? But what if command fail due to e.g. communication fail? I didn't see you have a place for failed answer. core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java <https://reviews.apache.org/r/19038/#comment69269> You won't need generateConfig. This command should be categoried as querycommand since it would demand immediately result. We cannot aggregate it. server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java <https://reviews.apache.org/r/19038/#comment69284> I think you can use listRunningBy() and skip the routers which are not managed by this mgmt server? Get both GuestType routers separately seems strange. server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java <https://reviews.apache.org/r/19038/#comment69285> you can just avoid this null by generate the object and putting a timestamp in it, e.g. 1970-01-01 00:00:00 UTC systemvm/patches/debian/config/opt/cloud/bin/getRouterAlerts.sh <https://reviews.apache.org/r/19038/#comment69277> You can use "tac" or alike tools to reverse the reading file, and stop when find old alerts. systemvm/patches/debian/config/opt/cloud/bin/getRouterAlerts.sh <https://reviews.apache.org/r/19038/#comment69279> Just compare the seconds since epoch. "date +%s" systemvm/patches/debian/config/opt/cloud/bin/getRouterAlerts.sh <https://reviews.apache.org/r/19038/#comment69282> It should be fine to always use alerts="${alerts}\n${line}" anyway... You don't need so complex if statement here. - Sheng Yang On March 14, 2014, 9:36 a.m., Harikrishna Patnala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19038/ > ----------------------------------------------------------- > > (Updated March 14, 2014, 9:36 a.m.) > > > Review request for cloudstack, Chiradeep Vittal, Jayapal Reddy, and Murali > Reddy. > > > Bugs: CLOUDSTACK-6090 > https://issues.apache.org/jira/browse/CLOUDSTACK-6090 > > > Repository: cloudstack-git > > > Description > ------- > > CLOUDSTACK-6090: Virtual Router Service Failure Alerting > Currently in CS we can monitor the running services on Virtual Router and > ensure they are running through the lifetime of VR. > Upon failure of any service in VR, monitoring service logs the alerts in VR > logs. > With this feature those alerts will be sent to MS. > > Added marvin tests > > > Diffs > ----- > > core/src/com/cloud/agent/api/GetRouterAlertsAnswer.java PRE-CREATION > core/src/com/cloud/agent/api/routing/GetRouterAlertsCommand.java > PRE-CREATION > > core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java > 3712aba > > engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml > 765d86c > engine/schema/src/com/cloud/network/dao/OpRouterMonitorServiceDao.java > PRE-CREATION > engine/schema/src/com/cloud/network/dao/OpRouterMonitorServiceDaoImpl.java > PRE-CREATION > engine/schema/src/com/cloud/network/dao/OpRouterMonitorServiceVO.java > PRE-CREATION > server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java > eeab91d > setup/db/db/schema-430to440.sql 056c5f8 > systemvm/patches/debian/config/opt/cloud/bin/getRouterAlerts.sh > PRE-CREATION > systemvm/patches/debian/config/root/monitorServices.py 0319ece > test/integration/smoke/test_VirtualRouter_alerts.py PRE-CREATION > > Diff: https://reviews.apache.org/r/19038/diff/ > > > Testing > ------- > > > Thanks, > > Harikrishna Patnala > >
