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
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
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
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
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1413#issuecomment-183966121
During the tests we found a couple of issues:
1. Default route was being set to private gateway interface after the
master router went through a
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1413#issuecomment-183715170
Manual Tests:
* Master Router
```
root@r-23-VM:~# ip addr | grep eth3 | grep state | awk '{print $9;}'
UP
root@r-23-VM:
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1413#issuecomment-183682371
I have done manual tests, but please do not merge it before I push the
existing integrations tests that I'm improving now to cover those fin
GitHub user wilderrodrigues opened a pull request:
https://github.com/apache/cloudstack/pull/1413
Fix unique mac address per rVPC router
This PR fixes an issue we faced yesterday in production with Redundant VPCs
and Private Gateways.
In that case, we ended up with 2
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1358#issuecomment-174600362
Thanks @borisroman and @remibergsma .
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email and have your
reply
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1354#issuecomment-173906754
Simple UI fix. LGTM :+1:
---
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
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1356#issuecomment-173902852
@remibergsma already tested and code LGTM :+1:
We have this PR already running in out Beta environment, which is a copy of
our production
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1358#issuecomment-173902040
Problem fixed!
Ping @remibergsma @borisroman
* Delete ACL that contains items via Cloud Monkey
```
(local) ðµ > del
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1359#issuecomment-173869657
Hi @ProjectMoon
Could you please create you PR against Master. If that's a refactor does
not really make sense to create the PR agains
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1358#issuecomment-173865670
Found and issue when destroying the VPC. Will fix it and update the PR.
```
(local) ðµ > delete vpc id=be232d77-495e-4f89-9b89-972db6a2e
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1358#issuecomment-173860410
Ping @remibergsma @borisroman @michaelandersen
Manual tests:
* Deleting attached ACL
Cheers,
Wilder
---
If your project is set up for it, you can reply to this
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/857#issuecomment-173197191
Ping @remibergsma @borisroman
I don;t have a way to test it now, but looked at the code - just like
@DaanHoogland @wido and @runseb did. I trust
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1198#issuecomment-173196826
Ping @remibergsma @borisroman
This one needs an integration test to cover the changes. Worth writing one
for. Who got time left
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1200#issuecomment-173196336
@koushik-das
Is there any integration test that can cover this?
If so, we will run it and get this merged. Otherwise I can help writing
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1323#issuecomment-173196099
Ping @remibergsma @borisroman
This one needs to be tested.
Cheers,
Wilder
---
If your project is set up for it, you can reply to
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1251#issuecomment-173195505
Ping @remibergsma
This one ca have the integrations tests executed against.
@pedro-martins: no need to add javadocs. The code should be
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1346#issuecomment-173169424
Ping @borisroman @remibergsma
PR LGTM :+1:
* Environment
* Hardware required: TRUE
* Management Server + MySQL on CentOS 7.1
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1346#issuecomment-172512058
Started the tests:
```
[root@cs1 integration]# nosetests --with-marvin
--marvin-config=/data/shared/marvin/mct-zone1-kvm1-ISOLATED.cfg -s -a
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1346#issuecomment-172488515
Hi @borisroman
Thanks for the fixes! It will really make the VRs behave better! :)
I now sit next to Boris here at Schuberg Philis and was
Github user wilderrodrigues closed the pull request at:
https://github.com/apache/cloudstack/pull/1277
---
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
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-172362547
This PR has already been merged by @remibergsma . For same weird reason it
remained open here and now says it has conflicts. Closing it.
Thanks
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-172250734
@remibergsma @DaanHoogland
This PR doesn't contain the commit that @koushik-das voted -1. So, it's
good to go.
I just double
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1329#issuecomment-172247143
Cool! Finally we got it!
LGTM!
Please, merge it @remibergsma !
Cheers,
Wilder
---
If your project is set up for it, you can
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1328#issuecomment-172247094
Just did a code review, but I also pointed @borisroman to the test code
when he was working on it.
LGTM... please, proceed with the merge
Github user wilderrodrigues commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/872#discussion_r49930246
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/configure.py ---
@@ -479,13 +479,13 @@ def process(self):
def deletevpn(self
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1301#issuecomment-172181790
@nitin-maharana
Love you PR details, dude! That's how a PR should be created. :)
Ping @remibergsma
LGTM, please proceed
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1317#issuecomment-172181688
Ping @remibergsma
LGTM, please proceed with merge.
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1318#issuecomment-172181655
Sat together with @borisroman when he was working on it. LGTM
Ping @remibergsma
Please proceed with merge.
Cheers,
Wilder
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1299#issuecomment-172181534
Ping @remibergsma
LGTM, please proceed with merge.
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1298#issuecomment-172181476
Ping @remibergsma
LGTM, please proceed with merge.
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1276#issuecomment-172181356
Ping @remibergsma
LGTM, please proceed with merge.
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1291#issuecomment-172181142
Ping @remibergsma
LGTM, please proceed with merge.
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1296#issuecomment-172180650
Ping @remibergsma
LGTM, please proceed with merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear
GitHub user wilderrodrigues reopened a pull request:
https://github.com/apache/cloudstack/pull/1277
[4.7] Critical VPCVR issues fixed: CLOUDSTACK-9154; CLOUDSTACK-9187; and
CLOUDSTACK-9188
This PR applies the same fixes as in the PR #1259, but against branch 4.7.
Please
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-172176953
Ping @remibergsma
If you get a second LGTM, feel free to merge.
Cheers,
Wilder
---
If your project is set up for it, you can reply
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-171572729
If those changes are needed by the community in the futures, please let me
know and I will reopen the PR. One can also feel free to create a PR on top of
Github user wilderrodrigues closed the pull request at:
https://github.com/apache/cloudstack/pull/1277
---
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
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170558660
@DaanHoogland @remibergsma @koushik-das
It worked here without the commit mentioned by @koushik-das .
```
[root@cs1 integration]# less
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170535918
Let's all move on, @koushik-das. If we don't, we will be talking about this
for weeks. We have been saying the same thing all the time. I got y
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170508275
Hi @koushik-das
Your questions were rhetorical and hence not valid because I did not
disagree with you in terms of refactor. I asked you to help
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170340809
* Results
```
[root@cs1 integration]# less
/tmp//MarvinLogs/test_vpc_redundant_DIBS05/results.txt
Create a redundant VPC with two networks
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170242223
Hi @koushik-das
It's not only with specific VPC tests, @koushik-das. That's why I asked you
to do it manually, but unfortunately you do
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170239651
Are you not sure what it should do once real machines are gone and a
network is still present?
The tests do:
1. Deploy a DC
2. Create
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170237820
Hi @koushik-das ,
Do you have real hardware to test?
Well I have 2 tests running agains real hardware. @remibergsma also
tested it and
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170236373
@koushik-das
You will have to run the tests on your environment. How did you check that
the Network GC was running every 10 seconds? After
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170218395
@remibergsma @koushik-das @DaanHoogland
I removed the commit you mentioned. But now we have to retest it. Do you
have a test environment? I need
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-170017549
Try something, @koushik-das:
```
Do not use this code for the steps below, but Master instead.
```
1. Deploy a DC
2. Create a
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-169996476
Do you agree that it is a completely different issue you are trying to
address?
ACS needs refactor in each corner of its code, but we cannot work
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-169939093
@DaanHoogland and @koushik-das
I split the commits and pushed the changes. You now should be able to
review the actual changes I have applied to
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-169928856
Hi @DaanHoogland and @koushik-das
Now I see your point. Thanks for the screenshot, @DaanHoogland. I checked
the Github diff but did not notice
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1311#issuecomment-169600413
The test I missed:
```
[root@cs1 integration]# less
/tmp//MarvinLogs/test_routers_network_ops_6WMMRL/results.txt
Test redundant router
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1311#issuecomment-169592688
Ping @remibergsma @DaanHoogland @borisroman
I'm now running the ```test_routers_network_ops``` because I noticed it was
not on the list afte
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1277#issuecomment-169304485
Ping @remibergsma @DaanHoogland @koushik-das @bhaisaab
Is there anyone caring about getting those fixes in? If not, I will close
the PR
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1311#issuecomment-169303235
Ping @remibergsma @miguelaferreira @michaelandersen
* Environment
- Management Server on CentOS 7.1
- 1 KVM Host on CentOS 7.1
GitHub user wilderrodrigues opened a pull request:
https://github.com/apache/cloudstack/pull/1311
CLOUDSTACK-9213 - As a user I want to be able to use multiple ip's/cidrs in
an ACL
This PR fixes a problem with iptables when creating ACL items using a comma
separated value li
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1276#issuecomment-18278
Did a code review on this PR and also talked to @michaelandersen on Slack
about switching the MASTER router off. With the changes he applied on the Java
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1276#issuecomment-17779
Jenkins error is not related:
My other tests are still running. Will post results here once they are done.
---
If your project is set up for it, you
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1259#issuecomment-166010092
Sorry @DaanHoogland, I don't understand what you meant. :) pr/pull jobs
related to my previous comment? Or is it something preventing you to test th
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1259#issuecomment-166005919
From here I see that the build was successful:
https://builds.apache.org/job/cloudstack-pr-analysis/162/console
```
[INFO
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1259#issuecomment-165997403
Ping @remibergsma @DaanHoogland @borisroman @miguelaferreira @bhaisaab
@karuturi
* Environment
```
Management Server on CentOS 7.1
GitHub user wilderrodrigues opened a pull request:
https://github.com/apache/cloudstack/pull/1259
Critical VPCVR issues fixed: CLOUDSTACK-9154; CLOUDSTACK-9187; and
CLOUDSTACK-9188
This PR fixes:
* CLOUDSTACK-9154: rVPC doesn't recover from cleaning up of network ga
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1224#issuecomment-164783572
@DaanHoogland, there was no strong argument about it. @bhaisaab doesn't as
point that can stop a merge. He simply wants to make his daily tasks e
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164494786
I truly believe we need a meeting to discuss this. I understand the hole
licensing thing, but having a PR open to review is already a prove that the
issue
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164474404
@rafaelweingartner
It's not about throwing stones, it's about the process.
Simply put: 2 LGTM, and depending on the PR at le
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164467579
+1 for reverting it. And I don't even care if the RAT is broken after the
rever. Reasons are obvious, but we have to stick to the 2 LGTM otherwis
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1224#issuecomment-164371097
@DaanHoogland @bhaisaab
The first thing is that using a standard is better than doing the way
nobody else is doing. When one adopts a technology
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1235#issuecomment-164253492
The change LGTM. We should get this in and for ACS 4.8 we do a refactor in
order to have a proper solution in place.
Cheers,
Wilder
---
If
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1231#issuecomment-164253054
Thanks @DaanHoogland @remibergsma @bhaisaab and @borisroman for reacting
very quickly!
---
If your project is set up for it, you can reply to this email
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1231#issuecomment-164161230
Ping @remibergsma @DaanHoogland @borisroman
Could you guys test it before the RC? I just fixed, but have to go to a
concert now.
---
If your
GitHub user wilderrodrigues opened a pull request:
https://github.com/apache/cloudstack/pull/1231
CLOUDSTACK-9151 - As a Developer I want the VRID to be set within the
limits of KeepaliveD
This PR fixes a blocker issue!
- Just like with RVRs, use the VRID 51 instead of
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1224#issuecomment-164157579
Nice one, @borisroman !
LGTM :+1:
Waiting for the test results. :)
---
If your project is set up for it, you can reply to this email and
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1196#issuecomment-164149214
Go for it, @remibergsma! Changes LGTM :+1:
@DaanHoogland: should we wait for the outcome of your build?
Cheers,
Wilder
---
If your
Github user wilderrodrigues commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1196#discussion_r47431578
--- Diff:
plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixHelper.java
---
@@ -236,4 +236,15 @@ public static
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1227#issuecomment-164139213
Cool! Also nice you put the screenshot to show it in action.
LGTM
Cheers,
Wilder
---
If your project is set up for it, you can reply
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1221#issuecomment-164135820
Thanks for the reviw, @borisroman! :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user wilderrodrigues commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1221#discussion_r47429990
--- Diff: test/integration/smoke/test_internal_lb.py ---
@@ -286,9 +302,12 @@ def setUpClass(cls):
%s
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1222#issuecomment-164127045
Awesome!
---
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
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1222#issuecomment-164018739
Ping @DaanHoogland @remibergsma @miguelaferreira @borisroman
@davidamorimfaria
Tests have been executed again and all looks fine
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1222#issuecomment-164008050
Ping @remibergsma @DaanHoogland @miguelaferreira @borisroman
@michaelandersen
One test failed due to a network hiccup. I will run only that test
1 - 100 of 799 matches
Mail list logo
