[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[dmabry]

Github user dmabry commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-218765751
  
I tested this in our 4.8 HW Lab and it worked as expected.  I found the 
MASTER VR for a TEST VPC with a private gateway configured.  I started a ping 
to the private gateway from an instance.  Finally I ran virsh destroy against 
r-XX-VM and only dropped a few pings before the private gateway was moved to 
the new VR.

LGTM


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-218743823
  
Can I get one more review on this one?  
@remibergsma are you choosing not to squash because you are not the 
original author of some of the commits?  If that is the case, I understand and 
am fine with it.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-218706540
  
LGTM did a code walk through and I know of a production install this code 
is running in.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1483#discussion_r62985942
  
--- Diff: server/src/com/cloud/network/element/VpcVirtualRouterElement.java 
---
@@ -466,7 +466,7 @@ public boolean deletePrivateGateway(final 
PrivateGateway gateway) throws Concurr
 }
 }
 
-return result > 0 ? true : false;
+return result == routers.size() ? true : false;
 }
--- End diff --

@alexandrelimassantana This code is not very new, please write a little 
pull request to that extend. (also the issue that you quite justly pointed out 
is there in the old code as well and bears no weight on the validity of this 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-218649701
  
I need some code review on this one.  Thanks...


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-217605374
  


### CI RESULTS

```
Tests Run: 85
  Skipped: 0
   Failed: 1
   Errors: 1
 Duration: 10h 26m 38s
```

**Summary of the problem(s):**
```
FAIL: Test destroy(expunge) Virtual Machine
--
Traceback (most recent call last):
  File 
"/data/git/cs1/cloudstack/test/integration/smoke/test_vm_life_cycle.py", line 
646, in test_09_expunge_vm
self.assertEqual(list_vm_response,None,"Check Expunged virtual machine 
is in listVirtualMachines response")
AssertionError: Check Expunged virtual machine is in listVirtualMachines 
response
--
Additional details in: /tmp/MarvinLogs/test_vpc_routers_LJEVIW/results.txt
```

```
ERROR: Test to verify access to loadbalancer haproxy admin stats page
--
Traceback (most recent call last):
  File 
"/data/git/cs1/cloudstack/test/integration/smoke/test_internal_lb.py", line 
854, in tearDown
raise Exception("Cleanup failed with %s" % e)
Exception: Cleanup failed with Job failed: {jobprocstatus : 0, created : 
u'2016-05-07T01:17:22+0200', jobresult : {errorcode : 530, errortext : u'Failed 
to delete network'}, cmd : 
u'org.apache.cloudstack.api.command.user.network.DeleteNetworkCmd', userid : 
u'ecdfa0ce-13af-11e6-929e-5254001daa61', jobstatus : 2, jobid : 
u'7ba837e5-36f2-4467-ab1b-69305fe3c897', jobresultcode : 530, jobresulttype : 
u'object', jobinstancetype : u'Network', accountid : 
u'ecdf8a0d-13af-11e6-929e-5254001daa61'}
--
Additional details in: /tmp/MarvinLogs/test_network_4LL71G/results.txt
```



**Associated Uploads**

**`/tmp/MarvinLogs/DeployDataCenter__May_06_2016_19_30_38_6T9JP6:`**
* 
[dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1483/tmp/MarvinLogs/DeployDataCenter__May_06_2016_19_30_38_6T9JP6/dc_entries.obj)
* 
[failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1483/tmp/MarvinLogs/DeployDataCenter__May_06_2016_19_30_38_6T9JP6/failed_plus_exceptions.txt)
* 
[runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1483/tmp/MarvinLogs/DeployDataCenter__May_06_2016_19_30_38_6T9JP6/runinfo.txt)

**`/tmp/MarvinLogs/test_network_4LL71G:`**
* 
[failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1483/tmp/MarvinLogs/test_network_4LL71G/failed_plus_exceptions.txt)
* 
[results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1483/tmp/MarvinLogs/test_network_4LL71G/results.txt)
* 
[runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1483/tmp/MarvinLogs/test_network_4LL71G/runinfo.txt)

**`/tmp/MarvinLogs/test_vpc_routers_LJEVIW:`**
* 
[failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1483/tmp/MarvinLogs/test_vpc_routers_LJEVIW/failed_plus_exceptions.txt)
* 
[results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1483/tmp/MarvinLogs/test_vpc_routers_LJEVIW/results.txt)
* 
[runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1483/tmp/MarvinLogs/test_vpc_routers_LJEVIW/runinfo.txt)


Uploads will be available until `2016-07-07 02:00:00 +0200 CEST`

*Comment created by [`upr comment`](https://github.com/cloudops/upr).*



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-216560940
  
Thank you sir...


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[kiwiflyer]

Github user kiwiflyer commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-216517829
  
Well, the open version of it - PR 1483


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[kiwiflyer]

Github user kiwiflyer commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-216517631
  
I'm about to start testing this 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[rhtyd]

Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-216227701
  
@remibergsma please squash changes to a single commit, and push -f

tag:needlove


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-208794235
  
@swill force pushed


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[dsclose]

Github user dsclose commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-208755644
  
@kiwiflyer 

I'm currently working on CLOUDSTACK-9339 to resolve the issues that I 
mentioned in #1413 - I don't think this PR breaks anything that currently works 
so CLOUDSTACK-9339 probably shouldn't hold this PR up.

Dean


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[kiwiflyer]

Github user kiwiflyer commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-208522087
  
@remibergsma 

Is this exactly the same PR as the one Wilder closed last week, or does it 
have some changes in it?

Also, there are some comments from @dsclose added after the old PR was 
closed regarding some potential gotchas. See 
https://github.com/apache/cloudstack/pull/1413#issuecomment-205716220

I agree that this (along with the additional fixes proposed above) need to 
get in to the next release.

- Si


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1483#issuecomment-207988893
  
@alexandrelimassantana please make a pull request to the branch that the PR 
is created from, Remi can include you change in this one by pulling it to his 
branch.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[alexandrelimassantana]

Github user alexandrelimassantana commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1483#discussion_r59121072
  
--- Diff: server/src/com/cloud/network/element/VpcVirtualRouterElement.java 
---
@@ -466,7 +466,7 @@ public boolean deletePrivateGateway(final 
PrivateGateway gateway) throws Concurr
 }
 }
 
-return result > 0 ? true : false;
+return result == routers.size() ? true : false;
 }
--- End diff --

This line is equivallent to 
```Java
return result == routers.size();
```

please take the ternary operator away


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[remibergsma]

GitHub user remibergsma opened a pull request:

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

CLOUDSTACK-9287 - Fix unique mac address per rVPC router

This is work by @wilderrodrigues, see PR #1413 It contains important fixes 
and I think it needs to be included so I send the PR again.

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

$ git pull https://github.com/remibergsma/cloudstack pr1413-wilder-47

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

https://github.com/apache/cloudstack/pull/1483.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 #1483


commit f921ec686bcf339968040c57bf0ae9fceffd8fed
Author: Wilder Rodrigues 
Date:   2016-02-13T11:59:20Z

CLOUDSTACK-9287 - Generate new mac address if router is redundant and nic 
profile exists

commit d93b008deb2ced326b314fefbe11511d798be5e8
Author: Wilder Rodrigues 
Date:   2016-02-13T14:48:30Z

CLOUDSTACK-9287 - Put private gateway interface down on backup router

commit 250be376e879b6c43792fdb5af0f5c49747689da
Author: Wilder Rodrigues 
Date:   2016-02-13T16:29:14Z

CLOUDSTACK-9287 - Add integration test to cover the private gw 
interface/mac address issues

commit 057b54aa3e8a462b3ced9b53d7a55dd5a64a0e4e
Author: Remi Bergsma 
Date:   2016-02-14T13:39:53Z

CLOUDSTACK-9287 - Make sure private gw interface is not used for default gw

commit 6a767732f959186e95c71d31a2ed49bb67a85473
Author: Remi Bergsma 
Date:   2016-02-14T17:09:03Z

CLOUDSTACK-9287 - Bring up the private gw interface on state change to 
master

commit 850fb1a557b8eec04de79124fea5b6f923530d66
Author: Wilder Rodrigues 
Date:   2016-02-17T06:16:23Z

CLOUDSTACK-9287 - Check if the nic profile has already been removed from a 
certain router

   - In case of redundant VPCs, the ACL items are revoked in the first 
iteration. Since the econd iteration
 is needed in order to remove the private network, we have to check if 
the nic profile is gone before trying
 to revoke the ACL items again, which would throw a NPE.
   - Some variable extraction in order to ease debugging.

commit c41edc1fe62cf92ab85c59c6da4dced9e626d961
Author: Wilder Rodrigues 
Date:   2016-02-17T06:31:39Z

CLOUDSTACK-9287 - Refactor the interface state configuration

  - This also refactors the CsAddress in order to offer better readability 
in a couple of methods.

commit 0e91468964b3b256497fa9dac50c63f9235ca102
Author: Wilder Rodrigues 
Date:   2016-02-17T06:33:48Z

CLOUDSTACK-9287 - Add integration test to cover the private gateway related 
changes

commit 78bbd498e7c7cb58570025b85d02c8ff921065d5
Author: Wilder Rodrigues 
Date:   2016-02-17T15:07:34Z

CLOUDSTACK-9287 - Fix RVR public interface

commit ffd8fdb45debe1ddcdf7c6706725cd26d4742cf2
Author: Wilder Rodrigues 
Date:   2016-02-17T15:43:16Z

CLOUDSTACK-9287 - Improve test by checking if pvt gw is removed and fix 
typos




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[dsclose]

Github user dsclose commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-205831656
  
Thank you. I've raised CLOUDSTACK-9339 on Jira about the non-VPC RvR 
issues. As my company needs those issues solved in the short term I'll begin 
working on a fix from the 4.7 branch.

If that fix is later useful upstream then I shall create a PR. If I need to 
re-work things from a different branch then I'll be happy to do so subsequently


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[kiwiflyer]

Github user kiwiflyer commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-205820963
  
@dsclose This bug exists as far back as 4.6 I believe, as that was when RvR 
for VPC was introduced. So you might want base your patch on 4.7 and then it 
can be pulled into later releases.

Gentlemen,

@wilderrodrigues  @remibergsma  @ustcweizhou @DaanHoogland @swill 

How do you guys want to handle this. Should Dean base his PR on top of 
Wilders rebase, or are you guys going to collaborate on a solution?

- Si


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

RE: [GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[Dean Close]

Thank you, I'm going to raise a separate bug report about the issues I've found 
with RvR setups. If I work on a patch I'll fork from the master branch unless I 
hear otherwise.

Dean

-Original Message-
From: kiwiflyer [mailto:g...@git.apache.org] 
Sent: 05 April 2016 14:28
To: dev@cloudstack.apache.org
Subject: [GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac 
address ...

Github user kiwiflyer commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-205804779
  
@dsclose  - just FYI. As far as I'm aware, this PR has not been committed 
yet. @ustcweizhou has requested a new PR/rebase.

I just pulled these into our build tree last night, so I was going to start 
testing 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 infrastruct...@apache.org or file a JIRA ticket with 
INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[kiwiflyer]

Github user kiwiflyer commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-205804779
  
@dsclose  - just FYI. As far as I'm aware, this PR has not been committed 
yet. @ustcweizhou has requested a new PR/rebase.

I just pulled these into our build tree last night, so I was going to start 
testing 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[dsclose]

Github user dsclose commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-205716220
  
@wilderrodrigues @ustcweizhou Sorry to arrive late to the party but this 
appears to break/ignore some RvR functionality. 4 things in particular stand 
out:

1. All public interfaces should be down in both redundanant virtual 
routers. Non-public interfaces should be up. The master.py script will bring up 
public interfaces on master routers. I had been discussing this with 
@ustcweizhou on the Cloudstack mailing list, Wei had provided a solution based 
upon the check_is_up method of the CsIP class (in CsAddress.py) but the commit 
at 8bbea5eeb6598b213b78a324c184841dbba69280 in #1413 appears to contradict what 
we discussed.
2. Redundant virtual routers can have multiple public interfaces. 
Presumably, so can VPC routers - although I've not experimented with that. The 
source-NAT IP will be assigned to eth2 - as will any public IP on the same 
subnet. Public IPs on different subnets will be assigned to eth3, eth4, eth5 
etc. with a new device being created for each separate subnet. The commit 
11e61f7054234e17343e9a11948f1804d732ac6c in #1413 does not account for this.
3. Because of point 2, we know that a new interface will be created on an 
RvR when an IP on a new subnet is assigned to it. Because of point 1, we know 
that that interface will be down because the master.py script will not have run 
since it was created. Currently a failover must be induced before this 
interface will be brought up - I'm yet to address this issue because of point 
4, next.
4. Currently static-NATs do not work on RvR setup when they are on a 
different subnet to the source-NAT IP. There are two reasons for this; one is 
missing iptables config allowing traffic across interfaces other than eth2. The 
second reason is that connmarks are being assigned to outbound traffic from 
eth0 (guest network) which are being used to match traffic to an interface in 
the IP rules. Traffic intended for a public network therefore gets routed out 
of the default gateway interface (eth2), breaking any static-NAT traffic on 
public IPs not assigned to eth2.

I'd like to contribute to points 3 and 4 but issue #1413 appears to have 
been marked as closed. To remedy these points I might need to overwrite code 
committed as part of this issue, so it seems best to start a conversation about 
it before rolling the sleeves up and getting stuck in.

Are these issues being considered else where?




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[ustcweizhou]

Github user ustcweizhou commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-203884447
  
@wilderrodrigues will you create a new PR for this ? This PR is not merged.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[wilderrodrigues]

Github user wilderrodrigues closed the pull request at:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[wilderrodrigues]

Github user wilderrodrigues commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1413#discussion_r55236035
  
--- Diff: server/src/com/cloud/network/element/VpcVirtualRouterElement.java 
---
@@ -559,9 +559,16 @@ public boolean applyACLItemsToPrivateGw(final 
PrivateGateway gateway, final List
 final DataCenterVO dcVO = 
_dcDao.findById(network.getDataCenterId());
 final NetworkTopology networkTopology = 
networkTopologyContext.retrieveNetworkTopology(dcVO);
 
+final Network privateNetwork = 
_networkModel.getNetwork(gateway.getNetworkId());
+
 boolean result = true;
 for (final DomainRouterVO domainRouterVO : routers) {
-result = result && networkTopology.applyNetworkACLs(network, 
rules, domainRouterVO, isPrivateGateway);
+final NicProfile nicProfile = 
_networkModel.getNicProfile(domainRouterVO, privateNetwork.getId(), null);
+if (nicProfile != null) {
+result = result && 
networkTopology.applyNetworkACLs(network, rules, domainRouterVO, 
isPrivateGateway);
--- End diff --

No worries, @GabrielBrascher 

:)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[GabrielBrascher]

Github user GabrielBrascher commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1413#discussion_r55127726
  
--- Diff: server/src/com/cloud/network/element/VpcVirtualRouterElement.java 
---
@@ -559,9 +559,16 @@ public boolean applyACLItemsToPrivateGw(final 
PrivateGateway gateway, final List
 final DataCenterVO dcVO = 
_dcDao.findById(network.getDataCenterId());
 final NetworkTopology networkTopology = 
networkTopologyContext.retrieveNetworkTopology(dcVO);
 
+final Network privateNetwork = 
_networkModel.getNetwork(gateway.getNetworkId());
+
 boolean result = true;
 for (final DomainRouterVO domainRouterVO : routers) {
-result = result && networkTopology.applyNetworkACLs(network, 
rules, domainRouterVO, isPrivateGateway);
+final NicProfile nicProfile = 
_networkModel.getNicProfile(domainRouterVO, privateNetwork.getId(), null);
+if (nicProfile != null) {
+result = result && 
networkTopology.applyNetworkACLs(network, rules, domainRouterVO, 
isPrivateGateway);
--- End diff --

Sorry @wilderrodrigues, the "for" there makes sense to the result variable.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[GabrielBrascher]

Github user GabrielBrascher commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1413#discussion_r55127619
  
--- Diff: server/src/com/cloud/network/element/VpcVirtualRouterElement.java 
---
@@ -559,9 +559,16 @@ public boolean applyACLItemsToPrivateGw(final 
PrivateGateway gateway, final List
 final DataCenterVO dcVO = 
_dcDao.findById(network.getDataCenterId());
 final NetworkTopology networkTopology = 
networkTopologyContext.retrieveNetworkTopology(dcVO);
 
+final Network privateNetwork = 
_networkModel.getNetwork(gateway.getNetworkId());
+
 boolean result = true;
 for (final DomainRouterVO domainRouterVO : routers) {
-result = result && networkTopology.applyNetworkACLs(network, 
rules, domainRouterVO, isPrivateGateway);
+final NicProfile nicProfile = 
_networkModel.getNicProfile(domainRouterVO, privateNetwork.getId(), null);
+if (nicProfile != null) {
+result = result && 
networkTopology.applyNetworkACLs(network, rules, domainRouterVO, 
isPrivateGateway);
--- End diff --

@wilderrodrigues I think it might be more readable if changed to:
`return networkTopology.applyNetworkACLs(network, rules, domainRouterVO, 
isPrivateGateway);`
Thanks.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[remibergsma]

Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-185635109
  
LGTM, based on the tests below. This runs in prod at SBP.

Test results:
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -a 
tags=advanced,required_hardware=true \
component/test_vpc_redundant.py \
component/test_routers_iptables_default_policy.py \
component/test_routers_network_ops.py \
component/test_vpc_router_nics.py \
smoke/test_loadbalance.py \
smoke/test_internal_lb.py \
smoke/test_ssvm.py \
smoke/test_network.py

```

Result:

```
Check the password file in the Router VM ... === TestName: 
test_isolate_network_password_server | Status : SUCCESS ===
ok
Create a redundant VPC with two networks with two VMs in each network ... 
=== TestName: test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Status : 
SUCCESS ===
ok
Create a redundant VPC with two networks with two VMs in each network and 
check default routes ... === TestName: test_02_redundant_VPC_default_routes | 
Status : SUCCESS ===
ok
Create a redundant VPC with two networks with two VMs in each network ... 
=== TestName: 
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | Status : 
SUCCESS ===
ok
Create a redundant VPC with 1 Tier, 1 VM, 1 ACL, 1 PF and test Network GC 
Nics ... === TestName: test_04_rvpc_network_garbage_collector_nics | Status : 
SUCCESS ===
ok
Create a redundant VPC with 1 Tier, 1 VM, 1 ACL, 1 PF and test Network GC 
Nics ... === TestName: test_05_rvpc_multi_tiers | Status : SUCCESS ===
ok
Test iptables default INPUT/FORWARD policy on RouterVM ... === TestName: 
test_02_routervm_iptables_policies | Status : SUCCESS ===
ok
Test iptables default INPUT/FORWARD policies on VPC router ... === 
TestName: test_01_single_VPC_iptables_policies | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: 
test_01_isolate_network_FW_PF_default_routes_egress_true | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: 
test_02_isolate_network_FW_PF_default_routes_egress_false | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: 
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: 
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: 
test_03_RVR_Network_check_router_state | Status : SUCCESS ===
ok
Create a VPC with two networks with one VM in each network and test nics 
after destroy ... === TestName: test_01_VPC_nics_after_destroy | Status : 
SUCCESS ===
ok
Create a VPC with two networks with one VM in each network and test default 
routes ... === TestName: test_02_VPC_default_routes | Status : SUCCESS ===
ok
Check that the /etc/dhcphosts.txt doesn't contain duplicate IPs ... === 
TestName: test_router_dhcphosts | Status : SUCCESS ===
ok
Test to create Load balancing rule with source NAT ... === TestName: 
test_01_create_lb_rule_src_nat | Status : SUCCESS ===
ok
Test to create Load balancing rule with non source NAT ... === TestName: 
test_02_create_lb_rule_non_nat | Status : SUCCESS ===
ok
Test for assign & removing load balancing rule ... === TestName: 
test_assign_and_removal_lb | Status : SUCCESS ===
ok
Test create, assign, remove of an Internal LB with roundrobin http traffic 
to 3 vm's in a Single VPC ... === TestName: 
test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Status : SUCCESS ===
ok
Test create, assign, remove of an Internal LB with roundrobin http traffic 
to 3 vm's in a Redundant VPC ... === TestName: 
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | Status : SUCCESS ===
ok
Test to verify access to loadbalancer haproxy admin stats page ... === 
TestName: test_03_vpc_internallb_haproxy_stats_on_all_interfaces | Status : 
SUCCESS ===
ok
Test to verify access to loadbalancer haproxy admin stats page ... === 
TestName: test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | Status : 
SUCCESS ===
ok
Test SSVM Internals ... === TestName: test_03_ssvm_internals | Status : 
SUCCESS ===
ok
Test CPVM Internals ... === TestName: test_04_cpvm_internals | Status : 
SUCCESS ===
ok
Test stop SSVM ... === TestName: test_05_stop_ssvm | Status : SUCCESS ===
ok
Test stop CPVM ... === TestName: test_06_stop_cpvm | Status : SUCCESS ===
ok
Test reboot SSVM ... === TestName: test_07_reboot_ssvm | Status : SUCCESS 
===
ok
Test reboot CPVM ... === TestName: test_08_reboot_cpvm | Status : SUCCESS 
===
ok
Test destroy SSVM ... === TestName: test_09_destroy_ssvm | Status : SUCCESS 
===
ok
Test destroy CPVM ... === TestName: 

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-185052627
  
Hi,

I squashed a few commits that make sense to be squashed and amended all 
commit messages in order to add the CloudStack issue number. In addition, I 
removed a commit by @borisroman related to another fix. Why? That commit is not 
part of this fix and will follow in a separate PR.

Cheer,
Wilder




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-9287 - Fix unique mac address ...

[wilderrodrigues]

Github user wilderrodrigues commented on the pull request:

https://github.com/apache/cloudstack/pull/1413#issuecomment-184850454
  
Hi,

I fixed the issues mentioned in the commits descriptions and added more 
tests in order to cover them.

I'm running the tests and will add the results once they are done.

Cheers,
Wilder


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---