Return-Path: X-Original-To: apmail-incubator-cloudstack-dev-archive@minotaur.apache.org Delivered-To: apmail-incubator-cloudstack-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 0940AE7EF for ; Fri, 1 Mar 2013 10:30:41 +0000 (UTC) Received: (qmail 59359 invoked by uid 500); 1 Mar 2013 10:30:40 -0000 Delivered-To: apmail-incubator-cloudstack-dev-archive@incubator.apache.org Received: (qmail 59308 invoked by uid 500); 1 Mar 2013 10:30:40 -0000 Mailing-List: contact cloudstack-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: cloudstack-dev@incubator.apache.org Delivered-To: mailing list cloudstack-dev@incubator.apache.org Received: (qmail 59288 invoked by uid 99); 1 Mar 2013 10:30:39 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 01 Mar 2013 10:30:39 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id B16921C7C6B; Fri, 1 Mar 2013 10:30:31 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============5446445953517266826==" MIME-Version: 1.0 Subject: Re: Review Request: AWS Style HealthCheck feature BugID : 664 From: "Vijay Venkatachalam" To: "Vijay Venkatachalam" Cc: "Rajesh Battala" , "cloudstack" Date: Fri, 01 Mar 2013 10:30:31 -0000 Message-ID: <20130301103031.9277.37378@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Vijay Venkatachalam" X-ReviewGroup: cloudstack X-ReviewRequest-URL: https://reviews.apache.org/r/9165/ X-Sender: "Vijay Venkatachalam" References: <20130301090750.17206.8665@reviews.apache.org> In-Reply-To: <20130301090750.17206.8665@reviews.apache.org> Reply-To: "Vijay Venkatachalam" --===============5446445953517266826== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9165/#review17237 ----------------------------------------------------------- api/src/com/cloud/network/lb/LoadBalancingRule.java Can you reuse the other constructor? using this() api/src/org/apache/cloudstack/api/command/user/loadbalancer/CreateLBHealthC= heckPolicyCmd.java Watch out for Threshold typo api/src/org/apache/cloudstack/api/command/user/loadbalancer/CreateLBHealthC= heckPolicyCmd.java Watch out for Threshold typo api/src/org/apache/cloudstack/api/command/user/loadbalancer/CreateLBHealthC= heckPolicyCmd.java Camel Case Please api/src/org/apache/cloudstack/api/command/user/loadbalancer/CreateLBHealthC= heckPolicyCmd.java Watch out it is not LbRule Id, it is actually using health check id. Yo= u might want to correct stickiness policy. api/src/org/apache/cloudstack/api/command/user/loadbalancer/CreateLBHealthC= heckPolicyCmd.java This Exception is never thrown!! api/src/org/apache/cloudstack/api/command/user/loadbalancer/CreateLBHealthC= heckPolicyCmd.java Typo: "Create" api/src/org/apache/cloudstack/api/command/user/loadbalancer/DeleteLBHealthC= heckPolicyCmd.java Wrong Id reference it is not LBStickinessResponse!! api/src/org/apache/cloudstack/api/command/user/loadbalancer/DeleteLBHealthC= heckPolicyCmd.java Please use consistent terms, either LB Rule, seems to have been conveye= d differently in different places = api/src/org/apache/cloudstack/api/command/user/loadbalancer/ListLBHealthChe= ckPoliciesCmd.java check User access need not be done here, it is done again at LBService = Layer, please remove one. api/src/org/apache/cloudstack/api/response/LBHealthCheckPolicyResponse.java Copy Paste Error api/src/org/apache/cloudstack/api/response/LBHealthCheckPolicyResponse.java Copy Paste Error api/src/org/apache/cloudstack/api/response/LBHealthCheckResponse.java Why do we need this wrapper? api/src/org/apache/cloudstack/api/response/LBHealthCheckResponse.java Unused public function? client/tomcatconf/commands.properties.in No command called listLBHealthCheckPolicy - Vijay Venkatachalam On March 1, 2013, 9:07 a.m., Rajesh Battala wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9165/ > ----------------------------------------------------------- > = > (Updated March 1, 2013, 9:07 a.m.) > = > = > Review request for cloudstack and Vijay Venkatachalam. > = > = > 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 healthche= ck polices. = > = > commands will take the lbrule id as mandatory param and execute the actio= n. > 1. createLBHealthCheck : > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > LB ruleid is the mandatory param to the api. Remaining params (pingpa= th, 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 serv= ices present in the LB rule. > NetScaler will take care of monitoring according to the monitor params. > Monitor name will be (Cloud-Mon--) > = > Initially only one monitor is supported for an LB rule. > if createLBHealthCheck returns an error, it will cleanup the entry creat= ed in db. > = > 2. deleteLBHealthCheck: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > 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: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > LB ruleid is the mandatory param to the api. > this command will list LB HealthChecks created on the LB rule. > = > LBHealthCheckManager: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > A new field is introduce in the table load_balancer_vm_map (state of stri= ng 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_v= m_map. > The time interval fo the LBHealthManager can be configured from Global Se= ttings(healthcheck.update.interval). default value is 600 sec. = > possible values UP, DOWN, UNKNOWN, BUSY, OUT OF SERVICE, GOING OUT OF SER= VICE, 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 0087edc = > api/src/com/cloud/network/element/LoadBalancingServiceProvider.java 879= ea0e = > api/src/com/cloud/network/lb/LoadBalancingRule.java fb1d988 = > api/src/com/cloud/network/lb/LoadBalancingRulesService.java 3743aae = > api/src/com/cloud/network/rules/HealthCheckPolicy.java PRE-CREATION = > api/src/org/apache/cloudstack/api/ApiConstants.java a26b468 = > api/src/org/apache/cloudstack/api/ResponseGenerator.java 267238a = > api/src/org/apache/cloudstack/api/command/user/loadbalancer/CreateLBHea= lthCheckPolicyCmd.java PRE-CREATION = > api/src/org/apache/cloudstack/api/command/user/loadbalancer/DeleteLBHea= lthCheckPolicyCmd.java PRE-CREATION = > api/src/org/apache/cloudstack/api/command/user/loadbalancer/ListLBHealt= hCheckPoliciesCmd.java PRE-CREATION = > api/src/org/apache/cloudstack/api/response/LBHealthCheckPolicyResponse.= java PRE-CREATION = > api/src/org/apache/cloudstack/api/response/LBHealthCheckResponse.java P= RE-CREATION = > client/tomcatconf/commands.properties.in f03e8d5 = > client/tomcatconf/components.xml.in 7d86a1c = > plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/ele= ment/ElasticLoadBalancerElement.java abb36c3 = > plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/= ElasticLoadBalancerManagerImpl.java 81039d1 = > plugins/network-elements/f5/src/com/cloud/network/element/F5ExternalLoa= dBalancerElement.java 94c098e = > plugins/network-elements/netscaler/src/com/cloud/network/element/Netsca= lerElement.java 8f902df = > plugins/network-elements/netscaler/src/com/cloud/network/resource/Netsc= alerResource.java abea464 = > server/src/com/cloud/api/ApiResponseHelper.java eafee8a = > server/src/com/cloud/configuration/Config.java 8a75a96 = > server/src/com/cloud/network/ExternalLoadBalancerDeviceManager.java d97= 9f07 = > server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java= bcefccc = > server/src/com/cloud/network/LBHealthCheckPolicyVO.java PRE-CREATION = > server/src/com/cloud/network/NetworkManagerImpl.java f527b73 = > server/src/com/cloud/network/dao/LBHealthCheckPolicyDao.java PRE-CREATI= ON = > server/src/com/cloud/network/dao/LBHealthCheckPolicyDaoImpl.java PRE-CR= EATION = > server/src/com/cloud/network/dao/LoadBalancerVMMapVO.java 8856993 = > server/src/com/cloud/network/element/VirtualRouterElement.java 500d0b6 = > server/src/com/cloud/network/lb/LBHealthCheckManager.java PRE-CREATION = > server/src/com/cloud/network/lb/LBHealthCheckManagerImpl.java PRE-CREAT= ION = > server/src/com/cloud/network/lb/LoadBalancingRulesManager.java 9d7d22f = > server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 531a= 428 = > server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.= java 0e2eb63 = > server/src/com/cloud/server/ManagementServerImpl.java e80d48c = > setup/db/db/schema-410to420.sql 4349bd0 = > = > Diff: https://reviews.apache.org/r/9165/diff/ > = > = > Testing > ------- > = > Testing Done: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > = > 1. create LB rule of TCP protocol and assing instances, create lb healthc= heck on lb rule. A TCP monitor is created and it will binded to all the ser= vices. > 2. create LB rule of HTTP protocol and assing instances, create lb health= check on lb rule. HTTP monitor is created and it will binded to all the ser= vices. > 3. create LB rule of HTTP protocol and assing instances, create lb health= check on lb rule with pingpath and other params, HTTP monitor is created an= d 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 wi= ll be binded to the newly added service. > 5.for an existing LB with an Monitor, delete a service from LB, monitor w= ill be unbinded and service will be removed. > 6.delete an monitor for LB rule, all the service binded to the monitor wi= ll be unbinded and monitor will get removed. > 7.delete LB rule, lb vserver will be deleted and the monitor will be dele= ted. = > 8. list the LB rules giving the lb rule id, healtcheckpolicy created on t= he 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 LB= HealthCheckManager will be updating the service state from the Netscaler. > = > = > Thanks, > = > Rajesh Battala > = > --===============5446445953517266826==--