You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by niteshsarda <gi...@git.apache.org> on 2017/02/10 09:57:14 UTC

[GitHub] cloudstack pull request #1937: CLOUDSTACK-9779 : Releasing secondary guest I...

GitHub user niteshsarda opened a pull request:

    https://github.com/apache/cloudstack/pull/1937

    CLOUDSTACK-9779 : Releasing secondary guest IP fails with error VM nic Ip x.x.x.x is mapped to load balancing rule

    ISSUE 
    =================
    Releasing secondary guest IP fails with error VM nic Ip x.x.x.x is mapped to load balancing rule
    
    REPRO STEPS
    ==================
    1. Create two isolated guest networks with same CIDR
    2. Deploy VMs on both networks
    3. Acquire secondary IP on NICs of both VMs and make sure they have the same value, user can input the IP address.
    4. Configure Loadbalancing rule on one of the secondary IP address and try releasing the other secondary IP address.
    5. The operation would fail
    
    EXPECTED BEHAVIOR
    ==================
    Secondary IP address should be released if there are no LB rules associated with it.
    
    ACTUAL BEHAVIOR
    ==================
    Releasing secondary IP address even if there are no LB rules associated with it.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Accelerite/cloudstack CS-50136

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1937.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1937
    
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1937: CLOUDSTACK-9779 : Releasing secondary guest I...

Posted by niteshsarda <gi...@git.apache.org>.
Github user niteshsarda commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1937#discussion_r101502146
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -852,7 +852,8 @@ public boolean releaseSecondaryIpFromNic(long ipAddressId) {
                     throw new InvalidParameterValueException("Can' remove the ip " + secondaryIp + "is associate with static NAT rule public IP address id " + publicIpVO.getId());
    --- End diff --
    
    Will take this up in separate ticket. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1937: CLOUDSTACK-9779 : Releasing secondary guest I...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1937#discussion_r101968821
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -852,7 +852,8 @@ public boolean releaseSecondaryIpFromNic(long ipAddressId) {
                     throw new InvalidParameterValueException("Can' remove the ip " + secondaryIp + "is associate with static NAT rule public IP address id " + publicIpVO.getId());
                 }
     
    -            if (_lbService.isLbRuleMappedToVmGuestIp(secondaryIp)) {
    +            List<Long> lbRuleIdList = _firewallDao.listIdByNetworkAndPurposeAndNotRevoked(network.getId(), Purpose.LoadBalancing);
    --- End diff --
    
    @niteshsarda sorry what I said is not clear. You added two methods in Daos: listIdByNetworkAndPurposeAndNotRevoked and listByLoadBalancerIdVmIdAndInstanceIp, I suggest to implement it by one method in LoadBalancerDaoImpl.java


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1937: CLOUDSTACK-9779 : Releasing secondary guest IP fails...

Posted by niteshsarda <gi...@git.apache.org>.
Github user niteshsarda commented on the issue:

    https://github.com/apache/cloudstack/pull/1937
  
    @ustcweizhou : I have created a new PR for this issue https://github.com/apache/cloudstack/pull/1972 .
    
    Please check. Also, I have incorporated all the comments which you have provided in this PR in the new PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1937: CLOUDSTACK-9779 : Releasing secondary guest I...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1937#discussion_r101515754
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -852,7 +852,8 @@ public boolean releaseSecondaryIpFromNic(long ipAddressId) {
                     throw new InvalidParameterValueException("Can' remove the ip " + secondaryIp + "is associate with static NAT rule public IP address id " + publicIpVO.getId());
    --- End diff --
    
    @niteshsarda I've created a PR for it: https://github.com/apache/cloudstack/pull/1947


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1937: CLOUDSTACK-9779 : Releasing secondary guest IP fails...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on the issue:

    https://github.com/apache/cloudstack/pull/1937
  
    files are also changed from 100644 \u2192 100755 which is not necessary


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1937: CLOUDSTACK-9779 : Releasing secondary guest I...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1937#discussion_r100511377
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -852,7 +852,8 @@ public boolean releaseSecondaryIpFromNic(long ipAddressId) {
                     throw new InvalidParameterValueException("Can' remove the ip " + secondaryIp + "is associate with static NAT rule public IP address id " + publicIpVO.getId());
                 }
     
    -            if (_lbService.isLbRuleMappedToVmGuestIp(secondaryIp)) {
    +            List<Long> lbRuleIdList = _firewallDao.listIdByNetworkAndPurposeAndNotRevoked(network.getId(), Purpose.LoadBalancing);
    --- End diff --
    
    this two checks can be done by a join search in LoadBalancerVMMapDao.java


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1937: CLOUDSTACK-9779 : Releasing secondary guest I...

Posted by niteshsarda <gi...@git.apache.org>.
Github user niteshsarda closed the pull request at:

    https://github.com/apache/cloudstack/pull/1937


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1937: CLOUDSTACK-9779 : Releasing secondary guest I...

Posted by ustcweizhou <gi...@git.apache.org>.
Github user ustcweizhou commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1937#discussion_r100509837
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -852,7 +852,8 @@ public boolean releaseSecondaryIpFromNic(long ipAddressId) {
                     throw new InvalidParameterValueException("Can' remove the ip " + secondaryIp + "is associate with static NAT rule public IP address id " + publicIpVO.getId());
    --- End diff --
    
    line 849. The issue also happen on static nat. Could you fix it as well ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1937: CLOUDSTACK-9779 : Releasing secondary guest I...

Posted by niteshsarda <gi...@git.apache.org>.
Github user niteshsarda commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1937#discussion_r101966427
  
    --- Diff: server/src/com/cloud/network/NetworkServiceImpl.java ---
    @@ -852,7 +852,8 @@ public boolean releaseSecondaryIpFromNic(long ipAddressId) {
                     throw new InvalidParameterValueException("Can' remove the ip " + secondaryIp + "is associate with static NAT rule public IP address id " + publicIpVO.getId());
                 }
     
    -            if (_lbService.isLbRuleMappedToVmGuestIp(secondaryIp)) {
    +            List<Long> lbRuleIdList = _firewallDao.listIdByNetworkAndPurposeAndNotRevoked(network.getId(), Purpose.LoadBalancing);
    --- End diff --
    
    @ustcweizhou  : As per your comment, do you expect Null and Empty check for list variable "lbRuleIdList " to be moved in LoadBalancerVMMapDao.java class ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---