Re: Review Request 19034: Assigning LB rule to vm nic secondary ip
On March 14, 2014, 7:38 p.m., Chiradeep Vittal wrote: server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java, line 984 https://reviews.apache.org/r/19034/diff/1/?file=516177#file516177line984 where is the test for this? Added test - Jayapal --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19034/#review37257 --- On March 11, 2014, 12:04 p.m., Jayapal Reddy wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19034/ --- (Updated March 11, 2014, 12:04 p.m.) Review request for cloudstack, Abhinandan Prateek, Chiradeep Vittal, Kishan Kavala, and Murali Reddy. Bugs: CLOUDSTACK-2692 https://issues.apache.org/jira/browse/CLOUDSTACK-2692 Repository: cloudstack-git Description --- With this feature load balancing rules assigned/associated to any ip of the vm nic. The vm nic ip can be primary ip or nic secondary ip. Diffs - api/src/com/cloud/network/lb/LoadBalancingRulesService.java 4c87d34 api/src/org/apache/cloudstack/api/ApiConstants.java 0a3fafd api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java 6bd7537 engine/schema/src/com/cloud/network/dao/LoadBalancerVMMapDao.java 79eaf8e engine/schema/src/com/cloud/network/dao/LoadBalancerVMMapDaoImpl.java b0b14fc engine/schema/src/com/cloud/network/dao/LoadBalancerVMMapVO.java 0a67106 server/src/com/cloud/network/NetworkServiceImpl.java ebeb31a server/src/com/cloud/network/as/AutoScaleManagerImpl.java 2fa3821 server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 6f0c1e9 setup/db/db/schema-430to440.sql ee4cf21 Diff: https://reviews.apache.org/r/19034/diff/ Testing --- Tested configuring the LB rules to vm primary ip and vm secondary ip. The haproxy config is created correctly for the selected ip. Thanks, Jayapal Reddy
Re: Review Request 19034: Assigning LB rule to vm nic secondary ip
On March 14, 2014, 7:38 p.m., Chiradeep Vittal wrote: api/src/com/cloud/network/lb/LoadBalancingRulesService.java, line 97 https://reviews.apache.org/r/19034/diff/1/?file=516169#file516169line97 HoW ABOUT DESCRIBING THE ADDITIONAL PARAMETER IN THE COMMENTS updated the comment On March 14, 2014, 7:38 p.m., Chiradeep Vittal wrote: api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java, line 123 https://reviews.apache.org/r/19034/diff/1/?file=516171#file516171line123 Prefer returning empty map instead of null updated to return empty map On March 14, 2014, 7:38 p.m., Chiradeep Vittal wrote: server/src/com/cloud/network/NetworkServiceImpl.java, line 811 https://reviews.apache.org/r/19034/diff/1/?file=516175#file516175line811 Why not put this code in the LB manager. Moved to LB manager On March 14, 2014, 7:38 p.m., Chiradeep Vittal wrote: server/src/com/cloud/network/as/AutoScaleManagerImpl.java, line 1410 https://reviews.apache.org/r/19034/diff/1/?file=516176#file516176line1410 prefer passing empty maps done On March 14, 2014, 7:38 p.m., Chiradeep Vittal wrote: server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java, line 939 https://reviews.apache.org/r/19034/diff/1/?file=516177#file516177line939 You should have a test written for this condition done On March 14, 2014, 7:38 p.m., Chiradeep Vittal wrote: server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java, line 945 https://reviews.apache.org/r/19034/diff/1/?file=516177#file516177line945 if you had passed in an EMPTY map from the API level you wouldn't have to check for nullness updated to pass empty map and removed null check - Jayapal --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19034/#review37257 --- On March 11, 2014, 12:04 p.m., Jayapal Reddy wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19034/ --- (Updated March 11, 2014, 12:04 p.m.) Review request for cloudstack, Abhinandan Prateek, Chiradeep Vittal, Kishan Kavala, and Murali Reddy. Bugs: CLOUDSTACK-2692 https://issues.apache.org/jira/browse/CLOUDSTACK-2692 Repository: cloudstack-git Description --- With this feature load balancing rules assigned/associated to any ip of the vm nic. The vm nic ip can be primary ip or nic secondary ip. Diffs - api/src/com/cloud/network/lb/LoadBalancingRulesService.java 4c87d34 api/src/org/apache/cloudstack/api/ApiConstants.java 0a3fafd api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java 6bd7537 engine/schema/src/com/cloud/network/dao/LoadBalancerVMMapDao.java 79eaf8e engine/schema/src/com/cloud/network/dao/LoadBalancerVMMapDaoImpl.java b0b14fc engine/schema/src/com/cloud/network/dao/LoadBalancerVMMapVO.java 0a67106 server/src/com/cloud/network/NetworkServiceImpl.java ebeb31a server/src/com/cloud/network/as/AutoScaleManagerImpl.java 2fa3821 server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 6f0c1e9 setup/db/db/schema-430to440.sql ee4cf21 Diff: https://reviews.apache.org/r/19034/diff/ Testing --- Tested configuring the LB rules to vm primary ip and vm secondary ip. The haproxy config is created correctly for the selected ip. Thanks, Jayapal Reddy
Review Request 19034: Assigning LB rule to vm nic secondary ip
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19034/ --- Review request for cloudstack, Abhinandan Prateek, Chiradeep Vittal, Kishan Kavala, and Murali Reddy. Bugs: CLOUDSTACK-2692 https://issues.apache.org/jira/browse/CLOUDSTACK-2692 Repository: cloudstack-git Description --- With this feature load balancing rules assigned/associated to any ip of the vm nic. The vm nic ip can be primary ip or nic secondary ip. Diffs - api/src/com/cloud/network/lb/LoadBalancingRulesService.java 4c87d34 api/src/org/apache/cloudstack/api/ApiConstants.java 0a3fafd api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java 6bd7537 engine/schema/src/com/cloud/network/dao/LoadBalancerVMMapDao.java 79eaf8e engine/schema/src/com/cloud/network/dao/LoadBalancerVMMapDaoImpl.java b0b14fc engine/schema/src/com/cloud/network/dao/LoadBalancerVMMapVO.java 0a67106 server/src/com/cloud/network/NetworkServiceImpl.java ebeb31a server/src/com/cloud/network/as/AutoScaleManagerImpl.java 2fa3821 server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 6f0c1e9 setup/db/db/schema-430to440.sql ee4cf21 Diff: https://reviews.apache.org/r/19034/diff/ Testing --- Tested configuring the LB rules to vm primary ip and vm secondary ip. The haproxy config is created correctly for the selected ip. Thanks, Jayapal Reddy
