> On Jan. 30, 2013, 11:24 p.m., Chiradeep Vittal wrote: > > Can you > > a. ensure there are no tabs > > b. ensure that you do not use printStackTrace() any where > > c. Document the service interface > > d. avoid putting code into NetworkManagerImpl.java > > e. Remove unneeded imports > > f. fix indentations > > g. Avoid implementing the service and manager interface in the same class > > file.
Thanks alot for the review comments. Rebasing master after javlen merge, Am facing some issues. Am working on them. Will update the patch very soon. - Rajesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9165/#review15898 ----------------------------------------------------------- On Jan. 30, 2013, 8:17 p.m., Rajesh Battala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9165/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2013, 8:17 p.m.) > > > Review request for cloudstack. > > > Description > ------- > > Code Review for AWS Style Health Check Feature : > Added API commands : > 1. createLBHealthCheck > 2. deleteLBHealthCheck > 3. listLBHealthCheck > > load_balancer_healthcheck_policies table will hold the data for healthcheck > polices. > > commands will take the lbrule id as mandatory param and execute the action. > 1. createLBHealthCheck : > ======================== > LB ruleid is the mandatory param to the api. Remaining params (pingpath, > responstime, request Healthcheck_interval, Healthy_thresshold, > Unhealth_thresshold) have default values. if not specified in the command. > It will create monitor (tcp/http) depending upon the LB protocol. > after creating the LB Monitor, it will bind the monitor to all the services > present in the LB rule. > NetScaler will take care of monitoring according to the monitor params. > Monitor name will be (Cloud-Mon-<LB IP>-<port>) > > Initially only one monitor is supported for an LB rule. > if createLBHealthCheck returns an error, it will cleanup the entry created > in db. > > 2. deleteLBHealthCheck: > ======================== > LB ruleid is the mandatory param to the api. > the command will first unbind all the services attached to it and then the > monitor will be deleted. > DB entry in load_balancer_healthcheck_policies will be deleted. > > 3. listLBHealthChecks: > ====================== > LB ruleid is the mandatory param to the api. > this command will list LB HealthChecks created on the LB rule. > > LBHealthCheckManager: > ===================== > A new field is introduce in the table load_balancer_vm_map (state of string > type) > > The task of the LBHealthCheckManager is at every period of time, it will > fetch the service status of LB rules and update them in the > load_balancer_vm_map. > The time interval fo the LBHealthManager can be configured from Global > Settings(healthcheck.update.interval). default value is 600 sec. > possible values UP, DOWN, UNKNOWN, BUSY, OUT OF SERVICE, GOING OUT OF > SERVICE, DOWN WHEN GOING OUT OF SERVICE > > > This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-664. > > > Diffs > ----- > > api/src/com/cloud/agent/api/routing/HealthCheckLBConfigAnswer.java > PRE-CREATION > api/src/com/cloud/agent/api/routing/HealthCheckLBConfigCommand.java > PRE-CREATION > api/src/com/cloud/agent/api/to/LoadBalancerTO.java 2d166ea > api/src/com/cloud/event/EventTypes.java 63b7cd0 > api/src/com/cloud/network/element/LoadBalancingServiceProvider.java 879ea0e > api/src/com/cloud/network/lb/LBRulesHealthMonitorService.java PRE-CREATION > api/src/com/cloud/network/lb/LoadBalancingRule.java b68b9cb > api/src/com/cloud/network/lb/LoadBalancingRulesService.java 4081f6e > api/src/com/cloud/network/rules/HealthCheckPolicy.java PRE-CREATION > api/src/org/apache/cloudstack/api/ApiConstants.java d084271 > api/src/org/apache/cloudstack/api/ResponseGenerator.java 63df4dc > > api/src/org/apache/cloudstack/api/command/user/loadbalancer/CreateLBHealthCheckPolicyCmd.java > PRE-CREATION > > api/src/org/apache/cloudstack/api/command/user/loadbalancer/DeleteLBHealthCheckPolicyCmd.java > PRE-CREATION > > api/src/org/apache/cloudstack/api/command/user/loadbalancer/ListLBHealthCheckPoliciesCmd.java > PRE-CREATION > api/src/org/apache/cloudstack/api/response/LBHealthCheckPolicyResponse.java > PRE-CREATION > api/src/org/apache/cloudstack/api/response/LBHealthCheckResponse.java > PRE-CREATION > client/tomcatconf/commands.properties.in 3740fb0 > client/tomcatconf/components-nonoss.xml.in fbfc5cc > client/tomcatconf/components.xml.in c41d4f4 > > plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/element/ElasticLoadBalancerElement.java > 201b397 > > plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java > 82c6120 > > plugins/network-elements/f5/src/com/cloud/network/element/F5ExternalLoadBalancerElement.java > 2e6f6e7 > > plugins/network-elements/netscaler/src/com/cloud/network/element/NetscalerElement.java > c2dc1e0 > > plugins/network-elements/netscaler/src/com/cloud/network/resource/NetscalerResource.java > ca8c8a7 > server/src/com/cloud/api/ApiResponseHelper.java 1c8849a > server/src/com/cloud/configuration/Config.java 5e4996b > server/src/com/cloud/configuration/DefaultComponentLibrary.java 98da7ad > server/src/com/cloud/network/ExternalLoadBalancerDeviceManager.java 5e7b98a > server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java > 275401c > server/src/com/cloud/network/LBHealthCheckPolicyVO.java PRE-CREATION > server/src/com/cloud/network/LoadBalancerVMMapVO.java 3cc66dc > server/src/com/cloud/network/NetworkManager.java 546f1bf > server/src/com/cloud/network/NetworkManagerImpl.java bb60dcf > server/src/com/cloud/network/dao/LBHealthCheckPolicyDao.java PRE-CREATION > server/src/com/cloud/network/dao/LBHealthCheckPolicyDaoImpl.java > PRE-CREATION > server/src/com/cloud/network/element/VirtualRouterElement.java 2b54ae0 > server/src/com/cloud/network/lb/LBHealthCheckManager.java PRE-CREATION > server/src/com/cloud/network/lb/LBHealthCheckManagerImpl.java PRE-CREATION > server/src/com/cloud/network/lb/LoadBalancingRulesManager.java c9b3f93 > server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java dfd5232 > server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java > e1c78e1 > server/src/com/cloud/server/ConfigurationServerImpl.java b0abd04 > setup/db/create-schema.sql ead98a5 > setup/db/db/schema-40to410.sql ed4946e > > Diff: https://reviews.apache.org/r/9165/diff/ > > > Testing > ------- > > Testing Done: > ============= > > 1. create LB rule of TCP protocol and assing instances, create lb healthcheck > on lb rule. A TCP monitor is created and it will binded to all the services. > 2. create LB rule of HTTP protocol and assing instances, create lb > healthcheck on lb rule. HTTP monitor is created and it will binded to all the > services. > 3. create LB rule of HTTP protocol and assing instances, create lb > healthcheck on lb rule with pingpath and other params, HTTP monitor is > created and it will binded to all the services and its properties pingpath > value will be present. > 4.for an existing LB with an Monitor, add a new service to LB, monitor will > be binded to the newly added service. > 5.for an existing LB with an Monitor, delete a service from LB, monitor will > be unbinded and service will be removed. > 6.delete an monitor for LB rule, all the service binded to the monitor will > be unbinded and monitor will get removed. > 7.delete LB rule, lb vserver will be deleted and the monitor will be deleted. > 8. list the LB rules giving the lb rule id, healtcheckpolicy created on the > LB rule will be returned. if not it will return empty list > 9.Modify the healthcheck.update.interval to 1 min, at every one minute > LBHealthCheckManager will be updating the service state from the Netscaler. > > > Thanks, > > Rajesh Battala > >
