[GitHub] cloudstack pull request: CLOUDSTACK-9244 Fix setting up RFC1918 ro...

2016-01-20 Thread nislim 
Github user nislim commented on the pull request:

https://github.com/apache/cloudstack/pull/1352#issuecomment-173123805
  
Same result with the refactored code:
```
root@v-23-VM:~# route -n
Kernel IP routing table
Destination Gateway Genmask Flags Metric RefUse 
Iface
0.0.0.0 10.0.12.254 0.0.0.0 UG0  00 eth2
10.0.10.0   0.0.0.0 255.255.255.0   U 0  00 eth1
10.0.12.0   0.0.0.0 255.255.255.0   U 0  00 eth2
169.254.0.0 0.0.0.0 255.255.0.0 U 0  00 eth0
```
Code looks good to me



---
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-9244 Fix setting up RFC1918 ro...

2016-01-20 Thread asfgit 
Github user asfgit closed the pull request at:

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


---
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-9244 Fix setting up RFC1918 ro...

2016-01-19 Thread DaanHoogland 
Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1352#issuecomment-172975452
  
If this helps @nislim And is tested it should go in.
I feel uncertain of this functionality. It is called from sestro and 
console proxy setup functions but the code is also present in those functions. 
I think the lines 1195 and 1252 should be removed and the functionality 
abstracted out.


---
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-9244 Fix setting up RFC1918 ro...

2016-01-19 Thread remibergsma 
Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1352#issuecomment-172977865
  
@DaanHoogland I think we agree cloud-early-config needs major refactoring 
as it is one big copy/paste thing. Just don't think this is the time to do it.

All this does is pickup the public ip address and use it, like other parts 
also do.


---
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-9244 Fix setting up RFC1918 ro...

2016-01-19 Thread remibergsma 
Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1352#issuecomment-173005222
  
`cloud-early-config` now logs:

```
Not setting up route of RFC1918 space to 192.168.22.1 befause 192.168.23.2 
is RFC1918..
```

Routes are not set as expected:
```
root@v-85-VM: # route -n
Kernel IP routing table
Destination Gateway Genmask Flags Metric RefUse 
Iface
0.0.0.0 192.168.23.10.0.0.0 UG0  00 eth2
169.254.0.0 0.0.0.0 255.255.0.0 U 0  00 eth0
192.168.22.00.0.0.0 255.255.255.0   U 0  00 eth1
192.168.23.00.0.0.0 255.255.255.0   U 0  00 eth2
```

And when it has a real public ip a route is set to the internal gw. That 
functionality still works and didn't change.

```
Setting up route of RFC1918 space to 192.168.22.1 
```

Routes are setup now:
```
root@v-1357-VM: # route -n
Kernel IP routing table
Destination Gateway Genmask Flags Metric RefUse 
Iface
0.0.0.0 x.y.227.1   0.0.0.0 UG0  00 eth2
10.0.0.0192.168.22.1255.0.0.0   UG0  00 eth1
169.254.0.0 0.0.0.0 255.255.0.0 U 0  00 eth0
172.16.0.0  192.168.22.1255.240.0.0 UG0  00 eth1
192.168.0.0 192.168.22.1255.255.0.0 UG0  00 eth1
192.168.22.00.0.0.0 255.255.255.0   U 0  00 eth1
 x.y.227.0  0.0.0.0 255.255.255.0   U 0  00 eth2
```


---
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-9244 Fix setting up RFC1918 ro...

2016-01-19 Thread remibergsma 
Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1352#issuecomment-173002828
  
@nislim Can you please test the enhanced patch?


---
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-9244 Fix setting up RFC1918 ro...

2016-01-19 Thread remibergsma 
Github user remibergsma commented on the pull request:

https://github.com/apache/cloudstack/pull/1352#issuecomment-173002681
  
Discussed with @DaanHoogland and he sent me a patch ;-)


---
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-9244 Fix setting up RFC1918 ro...

2016-01-19 Thread DaanHoogland 
Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1352#discussion_r50168987
  
--- Diff: systemvm/patches/debian/config/etc/init.d/cloud-early-config ---
@@ -1166,6 +1166,9 @@ setup_storage_network() {
 }
 
 setup_system_rfc1918_internal() {
+  public_ip=$ETH2_IP
+  [ "$ETH2_IP" == "0.0.0.0" ] && public_ip=$ETH1_IP
--- End diff --

shouldn't line 1195 begone, then? I think it does the same


---
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-9244 Fix setting up RFC1918 ro...

2016-01-19 Thread DaanHoogland 
Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/1352#issuecomment-173112205
  
looks good


---
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-9244 Fix setting up RFC1918 ro...

2016-01-19 Thread remibergsma 
GitHub user remibergsma opened a pull request:

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

CLOUDSTACK-9244 Fix setting up RFC1918 routes

Public ip var was empty so routes were always set. Corrected this.

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

$ git pull https://github.com/remibergsma/cloudstack 
cloud-early-config-route-fix

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

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


commit d6015700532c7c4676f4d402611fa4738cd4045d
Author: Remi Bergsma 
Date:   2016-01-19T17:24:49Z

CLOUDSTACK-9244 Fix setting up RFC1918 routes




---
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-9244 Fix setting up RFC1918 ro...

2016-01-19 Thread nislim 
Github user nislim commented on the pull request:

https://github.com/apache/cloudstack/pull/1352#issuecomment-172929285
  
Fixed my problem, my routing tables are now looking as I expected.
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.
---