Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-221846391
Rebased against latest 4.7.
---
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 swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-221762730
CI is clean and everything is green. 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
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-219037058
It would be useful for someone knowledgeable in the network internals to
elaborate on @jayapalu's comment. We currently do not use the feature he's
talking
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-218848727
tag:needsreview
---
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 swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-218839655
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 0
Errors: 0
Duration: 8h 34m 12s
```
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-218839846
Can I get some code review on this one? Thx...
---
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 ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-202811882
@cristofolini The idea was that the source NAT IP is always the first IP.
Not sure if the logic is correct. But apparently I need to correct the logic
for the
Github user ProjectMoon commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57697669
--- Diff: server/src/com/cloud/network/router/CommandSetupHelper.java ---
@@ -778,10 +778,11 @@ public int compare(final PublicIpAddress o1, final
Github user cristofolini commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-202152926
@ProjectMoon According to that comment on line 781 in `CommandSetupHelper`
the conditional that follows is there to enable sourceNAT, yet you removed the
Github user jayapalu commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57422486
--- Diff: server/src/com/cloud/network/router/CommandSetupHelper.java ---
@@ -778,10 +778,11 @@ public int compare(final PublicIpAddress o1, final
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57319355
--- Diff: server/test/com/cloud/network/IpAddressManagerTest.java ---
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF)
Github user ProjectMoon commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57317877
--- Diff: server/test/com/cloud/network/IpAddressManagerTest.java ---
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF) under
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57313299
--- Diff: server/test/com/cloud/network/IpAddressManagerTest.java ---
@@ -0,0 +1,71 @@
+// Licensed to the Apache Software Foundation (ASF)
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-200825346
It is always great to see parts of a bigger method being extracted to
smaller ones, and then test cases and java docs being used.
I believe the
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57313534
--- Diff: server/src/com/cloud/network/IpAddressManagerImpl.java ---
@@ -1698,6 +1698,22 @@ public String acquireGuestIpAddress(Network
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1450#issuecomment-200345827
I added a basic test now and moved the method out @rafaelweingartner. Maybe
there are more scenarios that can be tested?
---
If your project is set up for it,
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1450#discussion_r57031859
--- Diff: server/src/com/cloud/network/IpAddressManagerImpl.java ---
@@ -1714,8 +1714,13 @@ public boolean applyStaticNats(List staticNats,
17 matches
Mail list logo
