[GitHub] cloudstack pull request: DAO: Hit the cache for entity flagged as ...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1532#issuecomment-218367932
  
I reran the tests that failed and they succeeded the second time, so CI is 
in a good place on this one.  I need one more code review on this one.  
@marcaurele can you please force push this PR to kick of Jenkins again so when 
we get another code review we can merge right away.  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: DAO: Hit the cache for entity flagged as ...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1532#issuecomment-218367555
  


### CI RESULTS

```
Tests Run: 94
  Skipped: 0
   Failed: 2
   Errors: 0
 Duration: 7h 45m 13s
```

**Summary of the problem(s):**
```
FAIL: Create a redundant VPC with 1 Tier, 1 VM, 1 ACL, 1 PF and test 
Network GC Nics
--
Traceback (most recent call last):
  File 
"/data/git/cs2/cloudstack/test/integration/smoke/test_vpc_redundant.py", line 
605, in test_04_rvpc_network_garbage_collector_nics
self.check_routers_state(status_to_check="BACKUP", expected_count=2)
  File 
"/data/git/cs2/cloudstack/test/integration/smoke/test_vpc_redundant.py", line 
353, in check_routers_state
self.fail("Expected '%s' routers at state '%s', but found '%s'!" % 
(expected_count, status_to_check, cnts[vals.index(status_to_check)]))
AssertionError: Expected '2' routers at state 'BACKUP', but found '1'!
--
Additional details in: /tmp/MarvinLogs/test_network_BG7UFE/results.txt
```

```
FAIL: test_04_rvpc_privategw_static_routes 
(integration.smoke.test_privategw_acl.TestPrivateGwACL)
--
Traceback (most recent call last):
  File 
"/data/git/cs2/cloudstack/test/integration/smoke/test_privategw_acl.py", line 
277, in test_04_rvpc_privategw_static_routes
self.performVPCTests(vpc_off)
  File 
"/data/git/cs2/cloudstack/test/integration/smoke/test_privategw_acl.py", line 
324, in performVPCTests
self.check_pvt_gw_connectivity(vm1, public_ip_1, vm2.nic[0].ipaddress)
  File 
"/data/git/cs2/cloudstack/test/integration/smoke/test_privategw_acl.py", line 
559, in check_pvt_gw_connectivity
"Ping to outside world from VM should be successful"
AssertionError: Ping to outside world from VM should be successful
--
Additional details in: /tmp/MarvinLogs/test_network_BG7UFE/results.txt
```



**Associated Uploads**

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

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

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

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


Uploads will be available until `2016-07-11 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-9199: Fixed deployVirtualMachi...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1280#issuecomment-218367282
  
@anshul1886 can you please review and address the comments in this PR?  
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: L10n update master 20160127

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1376#issuecomment-218367016
  
#1515 has now been merged (sorry for the delay).  Once you have a chance to 
fix the merge conflicts, I can get this merged.  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 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-9328]: Fix vlan issues from t...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1455#issuecomment-218366880
  
Bump...  @sanju1010 can you please follow up on @DaanHoogland's comments.  
This one is ready to go otherwise, so it would be great if you could follow up 
with us on this @sanju1010.  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 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-9366: Capacity of one zone-wid...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1516#issuecomment-218366572
  
@sudhansu7 can you force push again to kick off Jenkins and Travis again?  
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 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-9265 cleanup around httpclient...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1385#issuecomment-218366133
  
@DaanHoogland can you force push this one again so we can kick off Jenkins 
again.  It has been stuck in this state for a week.  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 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: When no zone name is available display a ...

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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: When no zone name is available display a ...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1477#issuecomment-218365520
  
I am going to merge this one.  I am confident with the change and I have 
verification from @remibergsma, so I think we are good to go on this one.  


---
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: Taking fast and efficient volume snapshot...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1403#issuecomment-218365435
  
@jburwell can I get your final review on this one?  I think that is the 
only thing we are missing 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 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: Restore iptables at once using iptables-r...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1482#issuecomment-218365323
  
@remibergsma just a reminder to squash and re-push so we can get jenkins 
green.  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 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: CPU socket count reporting correction

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1520#issuecomment-218365187
  
@NuxRo can you close and reopen this PR to force Jenkins to kick off a run. 
 It has been hung in this state for days.  I know you can't easily `amend` and 
force push, so closing and reopening the PR will let you kick it off again 
through the UI.


---
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-8970 Centos 6.{1,2,3,4,5} gues...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/956#issuecomment-218362729
  
Can someone with a VMware environment validate this PR is functional?  I 
don't think running CI is going to give us anything on this one since I only 
have KVM  and this is only a DB change.  Thoughts???


---
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-8970 Centos 6.{1,2,3,4,5} gues...

2016-05-10 Thread SudharmaJain
Github user SudharmaJain commented on the pull request:

https://github.com/apache/cloudstack/pull/956#issuecomment-218362347
  
Rebased and pushed it again.


---
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-9299: Out-of-band Management f...

2016-05-10 Thread rhtyd
Github user rhtyd commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1502#discussion_r62790898
  
--- Diff: 
utils/src/main/java/org/apache/cloudstack/utils/process/ProcessRunner.java ---
@@ -0,0 +1,112 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package org.apache.cloudstack.utils.process;
+
+import com.cloud.utils.concurrency.NamedThreadFactory;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import org.apache.log4j.Logger;
+import org.joda.time.Duration;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+public class ProcessRunner {
+public static final Logger LOG = Logger.getLogger(ProcessRunner.class);
+
+private static final ExecutorService processExecutor = 
Executors.newCachedThreadPool(new NamedThreadFactory("ProcessRunner"));
--- End diff --

@jburwell I'll modify the executeCommands method to accept an executor; the 
ipmitool driver would create and re-use a fixedThreadPool


---
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-9299: Out-of-band Management f...

2016-05-10 Thread rhtyd
Github user rhtyd commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1502#discussion_r62790877
  
--- Diff: 
utils/src/main/java/org/apache/cloudstack/utils/process/ProcessRunner.java ---
@@ -0,0 +1,112 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package org.apache.cloudstack.utils.process;
+
+import com.cloud.utils.concurrency.NamedThreadFactory;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import org.apache.log4j.Logger;
+import org.joda.time.Duration;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+public class ProcessRunner {
+public static final Logger LOG = Logger.getLogger(ProcessRunner.class);
+
+private static final ExecutorService processExecutor = 
Executors.newCachedThreadPool(new NamedThreadFactory("ProcessRunner"));
+
+private static String readStream(final InputStream inputStream) {
+String text = null;
+try {
+final BufferedReader bufferedReader = new BufferedReader(new 
InputStreamReader(inputStream));
+String line;
+while ((line = bufferedReader.readLine()) != null) {
+if (Strings.isNullOrEmpty(text)) {
+text = line;
+} else {
+text = text + "\n" + line;
+}
+}
+} catch (IOException e) {
+if (LOG.isTraceEnabled()) {
+LOG.trace("ProcessRunner::readStream failed due to: " + 
e.getMessage());
+}
+}
+return text;
+}
+
+public static ProcessResult executeCommands(final List 
commands, final Duration timeOut) {
+Preconditions.checkArgument(commands != null && timeOut != null);
+
+int retVal = -2;
+String stdOutput = null;
+String stdError = null;
+
+try {
+final Process process = new 
ProcessBuilder().command(commands).start();
+if (timeOut.getStandardSeconds() > 0) {
--- End diff --

@jburwell ipmitool driver does not execute the command with a timeout, the 
utility needs to be generic which is why an option is provided to no consider 
timeout (0 = no timeout). The users of the utility need to make the decision 
and not this utility. For certain cases, it may be acceptable to execute a 
process without any time boundary therefore the utility should allow for both 
use-cases.


---
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-8562: Dynamic Role-Based API C...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1489#issuecomment-218359137
  
Thank you.  I am really trying to avoid that, but lets see what happens.  :)


---
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-8562: Dynamic Role-Based API C...

2016-05-10 Thread rhtyd
Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/1489#issuecomment-218358868
  
@swill done, though if it fails again we may ignore that as long as travis 
is green


---
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-8562: Dynamic Role-Based API C...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1489#issuecomment-218358102
  
thank you sir.  can you repush right away again.  jenkins already failed 
and I like to make sure everything is green before i merge stuff.  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 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-8562: Dynamic Role-Based API C...

2016-05-10 Thread rhtyd
Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/1489#issuecomment-218357625
  
@swill I've fixed the issues, here are the changes:

[changes.diff.txt](https://github.com/apache/cloudstack/files/258551/changes.diff.txt)

The conflicts were in travis, debian/control and the schema files only. If 
Travis is green, we can merge this without need to re-run the CI.


---
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-8901: PrepareTemplate job thre...

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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-8901: PrepareTemplate job thre...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/880#issuecomment-218356820
  
These connectivity issues are unrelated to this PR.  I will merge this 
now...


---
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-8901: PrepareTemplate job thre...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/880#issuecomment-218356736
  


### CI RESULTS

```
Tests Run: 85
  Skipped: 0
   Failed: 3
   Errors: 0
 Duration: 6h 24m 57s
```

**Summary of the problem(s):**
```
FAIL: Test redundant router internals
--
Traceback (most recent call last):
  File 
"/data/git/cs1/cloudstack/test/integration/smoke/test_routers_network_ops.py", 
line 483, in test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false
"Attempt to retrieve google.com index page should be successful once 
rule is added!"
AssertionError: Attempt to retrieve google.com index page should be 
successful once rule is added!
--
Additional details in: /tmp/MarvinLogs/test_network_GDLRM8/results.txt
```

```
FAIL: test_02_vpc_privategw_static_routes 
(integration.smoke.test_privategw_acl.TestPrivateGwACL)
--
Traceback (most recent call last):
  File 
"/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 
253, in test_02_vpc_privategw_static_routes
self.performVPCTests(vpc_off)
  File 
"/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 
324, in performVPCTests
self.check_pvt_gw_connectivity(vm1, public_ip_1, vm2.nic[0].ipaddress)
  File 
"/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 
559, in check_pvt_gw_connectivity
"Ping to outside world from VM should be successful"
AssertionError: Ping to outside world from VM should be successful
--
Additional details in: /tmp/MarvinLogs/test_network_GDLRM8/results.txt
```

```
FAIL: test_03_vpc_privategw_restart_vpc_cleanup 
(integration.smoke.test_privategw_acl.TestPrivateGwACL)
--
Traceback (most recent call last):
  File 
"/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 
265, in test_03_vpc_privategw_restart_vpc_cleanup
self.performVPCTests(vpc_off, True)
  File 
"/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 
331, in performVPCTests
self.check_pvt_gw_connectivity(vm1, public_ip_1, vm2.nic[0].ipaddress)
  File 
"/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 
559, in check_pvt_gw_connectivity
"Ping to outside world from VM should be successful"
AssertionError: Ping to outside world from VM should be successful
--
Additional details in: /tmp/MarvinLogs/test_network_GDLRM8/results.txt
```



**Associated Uploads**

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

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

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


Uploads will be available until `2016-07-11 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 

[GitHub] cloudstack pull request: Marvin: Replace a timer.sleep(30) with pu...

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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-8562: Dynamic Role-Based API C...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1489#issuecomment-218356331
  
Sorry to do this to you @rhtyd, but I just merged like 10 PRs and we now 
have merge conflicts.  Can you resolve the merge conflicts and re-push.  When 
you resolve the merge conflicts, would you mind not squashing the fixes to the 
merge conflicts so we can more easily review the changes required to resolve 
the merge conflicts.  This will help me know if I need to re-run CI before I 
merge this.  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-9340: General DB Optimization

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on the pull request:

https://github.com/apache/cloudstack/pull/1371#issuecomment-218355774
  
@abhinandanprateek Reading through the code, it seems like there could be 
benefit from the following value objects:

* IpAddress interface whose implementations would wrap InetAddress to 
provide factory methods and operations specific to CloudStack orchestration
* Netmask interface to encapsulate netmask-related operations for IPv4 and 
IPv6

Assuming that these new network primitives should orient us towards 
deprecating ``NetUtils``,  the new methods added to ``NetUtils`` for this 
enhancement should find a place in these new primitives.


---
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: agent: Enable IPv6 connectivity for KVM A...

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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-8818: Use MySQL native connect...

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62788791
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDRFactory.java ---
@@ -0,0 +1,79 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import com.cloud.utils.net.NetUtils;
+
+public class CIDRFactory {
+
+public static CIDR getCIDR(InetAddress baseAddress, int cidrMask) 
throws BadCIDRException {
+if (cidrMask < 0) {
+throw new BadCIDRException("Invalid mask length used: " + 
cidrMask);
+}
+if (baseAddress instanceof Inet4Address) {
+if (cidrMask > 32) {
+throw new BadCIDRException("Invalid mask length used: " + 
cidrMask);
+}
+return new CIDR4((Inet4Address)baseAddress, cidrMask);
+}
+// IPv6.
+if (cidrMask > 128) {
+throw new BadCIDRException("Invalid mask length used: " + 
cidrMask);
+}
+return new CIDR6((Inet6Address)baseAddress, cidrMask);
+}
+
+public static CIDR getCIDR(String cidr) throws BadCIDRException {
+int p = cidr.indexOf('/');
+if (p < 0) {
+throw new BadCIDRException("Invalid CIDR notation used: " + 
cidr);
+}
+String addrString = cidr.substring(0, p);
+String maskString = cidr.substring(p + 1);
+InetAddress addr = addressStringToInet(addrString);
+int mask;
+if (maskString.indexOf('.') < 0) {
+mask = Integer.decode(maskString);
+} else {
+mask = NetUtils.getNetMask(maskString);
+if (addr instanceof Inet6Address) {
+mask += 96;
+}
+}
+if (mask < 0) {
+throw new BadCIDRException("Invalid mask length used: " + 
maskString);
+}
+return getCIDR(addr, mask);
+}
+
+public static InetAddress addressStringToInet(String addr) throws 
BadCIDRException {
+try {
+return InetAddress.getByName(addr);
+} catch (UnknownHostException e) {
+throw new BadCIDRException("The fomat of address string is not 
valid " + addr);
+}
+}
--- End diff --

Consider moving this method to the ``IpAddress`` interface as 
``createFromHostName``.


---
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: Addresses CLOUDSTACK-9300 where the MySQL...

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62788718
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDRFactory.java ---
@@ -0,0 +1,79 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import com.cloud.utils.net.NetUtils;
+
+public class CIDRFactory {
+
+public static CIDR getCIDR(InetAddress baseAddress, int cidrMask) 
throws BadCIDRException {
+if (cidrMask < 0) {
+throw new BadCIDRException("Invalid mask length used: " + 
cidrMask);
+}
+if (baseAddress instanceof Inet4Address) {
+if (cidrMask > 32) {
+throw new BadCIDRException("Invalid mask length used: " + 
cidrMask);
+}
+return new CIDR4((Inet4Address)baseAddress, cidrMask);
+}
+// IPv6.
+if (cidrMask > 128) {
+throw new BadCIDRException("Invalid mask length used: " + 
cidrMask);
+}
+return new CIDR6((Inet6Address)baseAddress, cidrMask);
+}
+
+public static CIDR getCIDR(String cidr) throws BadCIDRException {
+int p = cidr.indexOf('/');
+if (p < 0) {
+throw new BadCIDRException("Invalid CIDR notation used: " + 
cidr);
+}
+String addrString = cidr.substring(0, p);
+String maskString = cidr.substring(p + 1);
+InetAddress addr = addressStringToInet(addrString);
+int mask;
+if (maskString.indexOf('.') < 0) {
+mask = Integer.decode(maskString);
+} else {
+mask = NetUtils.getNetMask(maskString);
+if (addr instanceof Inet6Address) {
+mask += 96;
+}
+}
+if (mask < 0) {
+throw new BadCIDRException("Invalid mask length used: " + 
maskString);
+}
+return getCIDR(addr, mask);
+}
--- End diff --

Consider renaming ``getCIDR`` to ``create`` since the ``get`` verb is 
generally in Java for methods that access attributes.


---
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: Removed Unused Void Class

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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-9351: Add ids parameter to res...

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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-9350: KVM-HA- Fix CheckOnHost ...

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62788636
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR6.java ---
@@ -0,0 +1,95 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet6Address;
+import java.net.InetAddress;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+public class CIDR6 implements CIDR {
--- End diff --

This class seems to be an empty implementation.  Why are we including it at 
this time?


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62788572
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
+cidrMask = mask;
+addressInt = ipv4AddressToInt(newaddr);
+int newmask = ipv4PrefixLengthToMask(mask);
+addressInt &= newmask;
+try {
+baseAddress = intToIPv4Address(addressInt);
+} catch (UnknownHostException e) {
+// this should never happen
+}
+addressEndInt = addressInt + ipv4PrefixLengthToLength(cidrMask) - 
1;
+}
+
+public int compareTo(CIDR arg) {
+if (arg instanceof CIDR6) {
+throw new NotImplementedException("Not implemented for CIDR6");
+}
+CIDR4 o = (CIDR4)arg;
+if (o.addressInt == addressInt && o.cidrMask == cidrMask) {
+return 0;
+}
+if (o.addressInt < addressInt) {
+return 1;
+}
+if (o.addressInt > addressInt) {
+return -1;
+}
+if (o.cidrMask < cidrMask) {
+// greater Mask means less IpAddresses so -1
+return -1;
+}
+return 1;
+}
+
+@Override
+public boolean isNull() {
+return false;
+}
+
+@Override
+public boolean contains(InetAddress inetAddress) {
+if (inetAddress == null) {
+throw new NullPointerException("inetAddress");
+}
+
+if (cidrMask == 0) {
+return true;
+}
+
+int search = ipv4AddressToInt(inetAddress);
+return search >= addressInt && search <= addressEndInt;
+}
+
+private static int ipv4PrefixLengthToLength(int prefixLength) {
+return 1 << 32 - prefixLength;
+}
+
+private static int ipv4PrefixLengthToMask(int prefixLength) {
+return ~((1 << 32 - prefixLength) - 1);
+}
+
+private static InetAddress intToIPv4Address(int addr) throws 
UnknownHostException {
+byte[] a = new byte[4];
+a[0] = (byte)(addr >> 24 & 0xFF);
+a[1] = (byte)(addr >> 16 & 0xFF);
+a[2] = (byte)(addr >> 8 & 0xFF);
+a[3] = (byte)(addr & 0xFF);
+return InetAddress.getByAddress(a);
+}
+
+private static int ipv4AddressToInt(InetAddress addr) {
+byte[] address;
+if (addr instanceof Inet6Address) {
+throw new NotImplementedException("Not implemented for ipv6 
address");
+} else {
+address = addr.getAddress();
+}
+return ipv4AddressToInt(address);
+}
+
+private static int ipv4AddressToInt(byte[] address) {
+int net = 0;
+for (byte addres : address) {
+net <<= 8;
+net |= addres & 0xFF;
+}
+return net;
+}
+
+@Override
+public InetAddress getBaseAddress() {
+return baseAddress;
+}
+
+@Override
+public int getMask() {
+ 

[GitHub] cloudstack pull request: CID-1338387: Deletion of method endPointS...

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62788491
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR.java ---
@@ -0,0 +1,98 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.InetAddress;
+import com.cloud.utils.Nullable;
+
+public interface CIDR extends Comparable, Nullable {
+
+final static CIDR NULL = new NullCIDR();
+final static int MAX_CIDR = 32;
--- End diff --

The ``static`` modifier is unnecessary on a interface declaration because 
all members are ``static`` by definition.  Therefore, the ``static`` modifiers 
can be removed.  Also, is the ``MAX_CIDR`` the same for IPv4 and IPv6?


---
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: Notify listeners when a host has been add...

2016-05-10 Thread swill
Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/816#issuecomment-218354565
  
Can you re-push to try to get Jenkins green.  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 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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62788397
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
+cidrMask = mask;
+addressInt = ipv4AddressToInt(newaddr);
+int newmask = ipv4PrefixLengthToMask(mask);
+addressInt &= newmask;
+try {
+baseAddress = intToIPv4Address(addressInt);
+} catch (UnknownHostException e) {
+// this should never happen
+}
+addressEndInt = addressInt + ipv4PrefixLengthToLength(cidrMask) - 
1;
+}
+
+public int compareTo(CIDR arg) {
+if (arg instanceof CIDR6) {
+throw new NotImplementedException("Not implemented for CIDR6");
+}
+CIDR4 o = (CIDR4)arg;
+if (o.addressInt == addressInt && o.cidrMask == cidrMask) {
+return 0;
+}
+if (o.addressInt < addressInt) {
+return 1;
+}
+if (o.addressInt > addressInt) {
+return -1;
+}
+if (o.cidrMask < cidrMask) {
+// greater Mask means less IpAddresses so -1
+return -1;
+}
+return 1;
+}
+
+@Override
+public boolean isNull() {
+return false;
+}
+
+@Override
+public boolean contains(InetAddress inetAddress) {
+if (inetAddress == null) {
+throw new NullPointerException("inetAddress");
+}
+
+if (cidrMask == 0) {
+return true;
+}
+
+int search = ipv4AddressToInt(inetAddress);
+return search >= addressInt && search <= addressEndInt;
+}
+
+private static int ipv4PrefixLengthToLength(int prefixLength) {
+return 1 << 32 - prefixLength;
+}
+
+private static int ipv4PrefixLengthToMask(int prefixLength) {
+return ~((1 << 32 - prefixLength) - 1);
+}
+
+private static InetAddress intToIPv4Address(int addr) throws 
UnknownHostException {
+byte[] a = new byte[4];
+a[0] = (byte)(addr >> 24 & 0xFF);
+a[1] = (byte)(addr >> 16 & 0xFF);
+a[2] = (byte)(addr >> 8 & 0xFF);
+a[3] = (byte)(addr & 0xFF);
+return InetAddress.getByAddress(a);
+}
+
+private static int ipv4AddressToInt(InetAddress addr) {
+byte[] address;
+if (addr instanceof Inet6Address) {
+throw new NotImplementedException("Not implemented for ipv6 
address");
+} else {
+address = addr.getAddress();
+}
+return ipv4AddressToInt(address);
+}
+
+private static int ipv4AddressToInt(byte[] address) {
+int net = 0;
+for (byte addres : address) {
+net <<= 8;
+net |= addres & 0xFF;
+}
+return net;
+}
+
+@Override
+public InetAddress getBaseAddress() {
+return baseAddress;
+}
+
+@Override
+public int getMask() {
+ 

[GitHub] cloudstack pull request: L10n update 4.8 20160422

2016-05-10 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62788312
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
--- End diff --

Default visibility should be sufficient for this constructor.  Also, add a 
Preconsitions.checkArgument check for ``newAddr`` being ``null``.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62788201
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
+cidrMask = mask;
+addressInt = ipv4AddressToInt(newaddr);
+int newmask = ipv4PrefixLengthToMask(mask);
+addressInt &= newmask;
+try {
+baseAddress = intToIPv4Address(addressInt);
+} catch (UnknownHostException e) {
+// this should never happen
+}
+addressEndInt = addressInt + ipv4PrefixLengthToLength(cidrMask) - 
1;
+}
+
+public int compareTo(CIDR arg) {
+if (arg instanceof CIDR6) {
+throw new NotImplementedException("Not implemented for CIDR6");
+}
+CIDR4 o = (CIDR4)arg;
+if (o.addressInt == addressInt && o.cidrMask == cidrMask) {
+return 0;
+}
+if (o.addressInt < addressInt) {
+return 1;
+}
+if (o.addressInt > addressInt) {
+return -1;
+}
+if (o.cidrMask < cidrMask) {
+// greater Mask means less IpAddresses so -1
+return -1;
+}
+return 1;
+}
+
+@Override
+public boolean isNull() {
+return false;
+}
+
+@Override
+public boolean contains(InetAddress inetAddress) {
+if (inetAddress == null) {
+throw new NullPointerException("inetAddress");
+}
+
+if (cidrMask == 0) {
+return true;
+}
+
+int search = ipv4AddressToInt(inetAddress);
+return search >= addressInt && search <= addressEndInt;
+}
+
+private static int ipv4PrefixLengthToLength(int prefixLength) {
+return 1 << 32 - prefixLength;
+}
+
+private static int ipv4PrefixLengthToMask(int prefixLength) {
+return ~((1 << 32 - prefixLength) - 1);
+}
+
+private static InetAddress intToIPv4Address(int addr) throws 
UnknownHostException {
+byte[] a = new byte[4];
+a[0] = (byte)(addr >> 24 & 0xFF);
+a[1] = (byte)(addr >> 16 & 0xFF);
+a[2] = (byte)(addr >> 8 & 0xFF);
+a[3] = (byte)(addr & 0xFF);
+return InetAddress.getByAddress(a);
+}
+
+private static int ipv4AddressToInt(InetAddress addr) {
+byte[] address;
+if (addr instanceof Inet6Address) {
+throw new NotImplementedException("Not implemented for ipv6 
address");
+} else {
+address = addr.getAddress();
+}
+return ipv4AddressToInt(address);
+}
+
+private static int ipv4AddressToInt(byte[] address) {
+int net = 0;
+for (byte addres : address) {
+net <<= 8;
+net |= addres & 0xFF;
+}
+return net;
+}
+
+@Override
+public InetAddress getBaseAddress() {
+return baseAddress;
+}
+
+@Override
+public int getMask() {
+ 

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62788152
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
+cidrMask = mask;
+addressInt = ipv4AddressToInt(newaddr);
+int newmask = ipv4PrefixLengthToMask(mask);
+addressInt &= newmask;
+try {
+baseAddress = intToIPv4Address(addressInt);
+} catch (UnknownHostException e) {
+// this should never happen
+}
+addressEndInt = addressInt + ipv4PrefixLengthToLength(cidrMask) - 
1;
+}
+
+public int compareTo(CIDR arg) {
+if (arg instanceof CIDR6) {
+throw new NotImplementedException("Not implemented for CIDR6");
+}
+CIDR4 o = (CIDR4)arg;
+if (o.addressInt == addressInt && o.cidrMask == cidrMask) {
+return 0;
+}
+if (o.addressInt < addressInt) {
+return 1;
+}
+if (o.addressInt > addressInt) {
+return -1;
+}
+if (o.cidrMask < cidrMask) {
+// greater Mask means less IpAddresses so -1
+return -1;
+}
+return 1;
+}
+
+@Override
+public boolean isNull() {
+return false;
+}
+
+@Override
+public boolean contains(InetAddress inetAddress) {
+if (inetAddress == null) {
+throw new NullPointerException("inetAddress");
+}
+
+if (cidrMask == 0) {
+return true;
+}
+
+int search = ipv4AddressToInt(inetAddress);
+return search >= addressInt && search <= addressEndInt;
+}
+
+private static int ipv4PrefixLengthToLength(int prefixLength) {
+return 1 << 32 - prefixLength;
+}
+
+private static int ipv4PrefixLengthToMask(int prefixLength) {
+return ~((1 << 32 - prefixLength) - 1);
+}
+
+private static InetAddress intToIPv4Address(int addr) throws 
UnknownHostException {
+byte[] a = new byte[4];
+a[0] = (byte)(addr >> 24 & 0xFF);
+a[1] = (byte)(addr >> 16 & 0xFF);
+a[2] = (byte)(addr >> 8 & 0xFF);
+a[3] = (byte)(addr & 0xFF);
+return InetAddress.getByAddress(a);
+}
+
+private static int ipv4AddressToInt(InetAddress addr) {
+byte[] address;
+if (addr instanceof Inet6Address) {
+throw new NotImplementedException("Not implemented for ipv6 
address");
+} else {
+address = addr.getAddress();
+}
+return ipv4AddressToInt(address);
+}
+
+private static int ipv4AddressToInt(byte[] address) {
+int net = 0;
+for (byte addres : address) {
+net <<= 8;
+net |= addres & 0xFF;
+}
+return net;
+}
+
+@Override
+public InetAddress getBaseAddress() {
+return baseAddress;
+}
+
+@Override
+public int getMask() {
+ 

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787923
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
+cidrMask = mask;
+addressInt = ipv4AddressToInt(newaddr);
+int newmask = ipv4PrefixLengthToMask(mask);
+addressInt &= newmask;
+try {
+baseAddress = intToIPv4Address(addressInt);
+} catch (UnknownHostException e) {
+// this should never happen
+}
+addressEndInt = addressInt + ipv4PrefixLengthToLength(cidrMask) - 
1;
+}
+
+public int compareTo(CIDR arg) {
+if (arg instanceof CIDR6) {
+throw new NotImplementedException("Not implemented for CIDR6");
+}
+CIDR4 o = (CIDR4)arg;
+if (o.addressInt == addressInt && o.cidrMask == cidrMask) {
+return 0;
+}
+if (o.addressInt < addressInt) {
+return 1;
+}
+if (o.addressInt > addressInt) {
+return -1;
+}
+if (o.cidrMask < cidrMask) {
+// greater Mask means less IpAddresses so -1
+return -1;
+}
+return 1;
+}
+
+@Override
+public boolean isNull() {
+return false;
+}
+
+@Override
+public boolean contains(InetAddress inetAddress) {
+if (inetAddress == null) {
+throw new NullPointerException("inetAddress");
+}
+
+if (cidrMask == 0) {
+return true;
+}
+
+int search = ipv4AddressToInt(inetAddress);
+return search >= addressInt && search <= addressEndInt;
+}
+
+private static int ipv4PrefixLengthToLength(int prefixLength) {
+return 1 << 32 - prefixLength;
+}
+
+private static int ipv4PrefixLengthToMask(int prefixLength) {
+return ~((1 << 32 - prefixLength) - 1);
+}
+
+private static InetAddress intToIPv4Address(int addr) throws 
UnknownHostException {
+byte[] a = new byte[4];
+a[0] = (byte)(addr >> 24 & 0xFF);
+a[1] = (byte)(addr >> 16 & 0xFF);
+a[2] = (byte)(addr >> 8 & 0xFF);
+a[3] = (byte)(addr & 0xFF);
+return InetAddress.getByAddress(a);
+}
+
+private static int ipv4AddressToInt(InetAddress addr) {
+byte[] address;
+if (addr instanceof Inet6Address) {
+throw new NotImplementedException("Not implemented for ipv6 
address");
+} else {
+address = addr.getAddress();
+}
+return ipv4AddressToInt(address);
+}
+
+private static int ipv4AddressToInt(byte[] address) {
+int net = 0;
+for (byte addres : address) {
+net <<= 8;
+net |= addres & 0xFF;
+}
+return net;
+}
+
+@Override
+public InetAddress getBaseAddress() {
+return baseAddress;
+}
+
+@Override
+public int getMask() {
+ 

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787904
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
+cidrMask = mask;
+addressInt = ipv4AddressToInt(newaddr);
+int newmask = ipv4PrefixLengthToMask(mask);
+addressInt &= newmask;
+try {
+baseAddress = intToIPv4Address(addressInt);
+} catch (UnknownHostException e) {
+// this should never happen
+}
+addressEndInt = addressInt + ipv4PrefixLengthToLength(cidrMask) - 
1;
+}
+
+public int compareTo(CIDR arg) {
+if (arg instanceof CIDR6) {
+throw new NotImplementedException("Not implemented for CIDR6");
+}
+CIDR4 o = (CIDR4)arg;
+if (o.addressInt == addressInt && o.cidrMask == cidrMask) {
+return 0;
+}
+if (o.addressInt < addressInt) {
+return 1;
+}
+if (o.addressInt > addressInt) {
+return -1;
+}
+if (o.cidrMask < cidrMask) {
+// greater Mask means less IpAddresses so -1
+return -1;
+}
+return 1;
+}
+
+@Override
+public boolean isNull() {
+return false;
+}
+
+@Override
+public boolean contains(InetAddress inetAddress) {
+if (inetAddress == null) {
+throw new NullPointerException("inetAddress");
+}
+
+if (cidrMask == 0) {
+return true;
+}
+
+int search = ipv4AddressToInt(inetAddress);
+return search >= addressInt && search <= addressEndInt;
+}
+
+private static int ipv4PrefixLengthToLength(int prefixLength) {
+return 1 << 32 - prefixLength;
+}
+
+private static int ipv4PrefixLengthToMask(int prefixLength) {
+return ~((1 << 32 - prefixLength) - 1);
+}
+
+private static InetAddress intToIPv4Address(int addr) throws 
UnknownHostException {
+byte[] a = new byte[4];
+a[0] = (byte)(addr >> 24 & 0xFF);
+a[1] = (byte)(addr >> 16 & 0xFF);
+a[2] = (byte)(addr >> 8 & 0xFF);
+a[3] = (byte)(addr & 0xFF);
+return InetAddress.getByAddress(a);
+}
+
+private static int ipv4AddressToInt(InetAddress addr) {
+byte[] address;
+if (addr instanceof Inet6Address) {
+throw new NotImplementedException("Not implemented for ipv6 
address");
+} else {
+address = addr.getAddress();
+}
+return ipv4AddressToInt(address);
+}
+
+private static int ipv4AddressToInt(byte[] address) {
+int net = 0;
+for (byte addres : address) {
+net <<= 8;
+net |= addres & 0xFF;
+}
+return net;
+}
--- End diff --

Would it make sense to move the ``intToIPv4Address``, 
``ipv4AddressToInit``, ``ipv4AddressToInt`` methods to the ``IpAddress`` 
interface?


[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787677
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
+cidrMask = mask;
+addressInt = ipv4AddressToInt(newaddr);
+int newmask = ipv4PrefixLengthToMask(mask);
+addressInt &= newmask;
+try {
+baseAddress = intToIPv4Address(addressInt);
+} catch (UnknownHostException e) {
+// this should never happen
+}
+addressEndInt = addressInt + ipv4PrefixLengthToLength(cidrMask) - 
1;
+}
+
+public int compareTo(CIDR arg) {
+if (arg instanceof CIDR6) {
+throw new NotImplementedException("Not implemented for CIDR6");
+}
+CIDR4 o = (CIDR4)arg;
+if (o.addressInt == addressInt && o.cidrMask == cidrMask) {
+return 0;
+}
+if (o.addressInt < addressInt) {
+return 1;
+}
+if (o.addressInt > addressInt) {
+return -1;
+}
+if (o.cidrMask < cidrMask) {
+// greater Mask means less IpAddresses so -1
+return -1;
+}
+return 1;
+}
+
+@Override
+public boolean isNull() {
+return false;
+}
+
+@Override
+public boolean contains(InetAddress inetAddress) {
+if (inetAddress == null) {
+throw new NullPointerException("inetAddress");
--- End diff --

``NullPointerException`` should only be thrown by VM when a null point is 
dereferenced.  Convert to a ``Preconditions.checkArgument`` check to simplify 
the code and ensure the correct exception type is thrown.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787612
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
+cidrMask = mask;
+addressInt = ipv4AddressToInt(newaddr);
+int newmask = ipv4PrefixLengthToMask(mask);
+addressInt &= newmask;
+try {
+baseAddress = intToIPv4Address(addressInt);
+} catch (UnknownHostException e) {
+// this should never happen
+}
+addressEndInt = addressInt + ipv4PrefixLengthToLength(cidrMask) - 
1;
+}
+
+public int compareTo(CIDR arg) {
+if (arg instanceof CIDR6) {
--- End diff --

Check that ``arg`` is not ``null`` before the ``instanceof`` check to avoid 
an NPE.  Also, to positively ensure that a ``ClassCastException`` does not 
occur on line 59, consider changing the ``instanceof`` check to ``!(arg 
instanceof CIDR4)`` to be explicit about the type.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787530
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
+
+protected CIDR4(Inet4Address newaddr, int mask) {
+cidrMask = mask;
+addressInt = ipv4AddressToInt(newaddr);
+int newmask = ipv4PrefixLengthToMask(mask);
+addressInt &= newmask;
+try {
+baseAddress = intToIPv4Address(addressInt);
+} catch (UnknownHostException e) {
+// this should never happen
--- End diff --

While this should never happen, it still might.  Throw an 
``IllegalStateException`` just in case to fail fast.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787493
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
+private int addressInt;
+private final int addressEndInt;
--- End diff --

Why aren't all attributes declared ``final``?


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787426
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
+protected final static Logger s_logger = Logger.getLogger(CIDR4.class);
+
+protected InetAddress baseAddress;
+protected int cidrMask;
--- End diff --

Why are these attributes declared ``protected`` rather than ``private``?


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787416
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR4.java ---
@@ -0,0 +1,172 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.commons.lang.NotImplementedException;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.net.NetUtils;
+
+/**
+ */
+public class CIDR4 implements CIDR {
--- End diff --

Why not declare this class as ``final``?


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787378
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR.java ---
@@ -0,0 +1,98 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.InetAddress;
+import com.cloud.utils.Nullable;
+
+public interface CIDR extends Comparable, Nullable {
+
+final static CIDR NULL = new NullCIDR();
+final static int MAX_CIDR = 32;
+
+public InetAddress getBaseAddress();
+
+public int getMask();
+
+public boolean contains(InetAddress inetAddress);
+
+public boolean contains(CIDR cidr);
+
+public boolean overlaps(CIDR cidr);
+
+public String toString();
+
+public boolean equals(Object o);
+
+public int hashCode();
+
+class NullCIDR implements CIDR {
+
+private NullCIDR() {
+super();
+}
+
+public boolean isNull() {
+return true;
+}
+
+@Override
+public String toString() {
+return "Null CIDR";
+}
+
+@Override
+public InetAddress getBaseAddress() {
+// TODO Auto-generated method stub
+return null;
--- End diff --

Ideally, a Null Object should not return ``null`` instances.  Any thoughts 
on an ``InetAddress`` instance we could use a logically null reference (e.g. 
would ``InetAddress`` of 0.0.0.0 make sense?  


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787136
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR.java ---
@@ -0,0 +1,98 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.InetAddress;
+import com.cloud.utils.Nullable;
+
+public interface CIDR extends Comparable, Nullable {
+
+final static CIDR NULL = new NullCIDR();
+final static int MAX_CIDR = 32;
+
+public InetAddress getBaseAddress();
+
+public int getMask();
+
+public boolean contains(InetAddress inetAddress);
+
+public boolean contains(CIDR cidr);
+
+public boolean overlaps(CIDR cidr);
+
+public String toString();
+
+public boolean equals(Object o);
+
+public int hashCode();
+
+class NullCIDR implements CIDR {
+
+private NullCIDR() {
+super();
+}
+
+public boolean isNull() {
+return true;
+}
+
+@Override
+public String toString() {
+return "Null CIDR";
+}
+
+@Override
+public InetAddress getBaseAddress() {
+// TODO Auto-generated method stub
+return null;
+}
+
+@Override
+public int getMask() {
+// TODO Auto-generated method stub
+return 0;
+}
+
+@Override
+public boolean contains(CIDR cidr) {
+// TODO Auto-generated method stub
+return false;
+}
+
+@Override
+public boolean overlaps(CIDR cidr) {
+// TODO Auto-generated method stub
--- End diff --

Please address and remove the TODO


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787151
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR.java ---
@@ -0,0 +1,98 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.InetAddress;
+import com.cloud.utils.Nullable;
+
+public interface CIDR extends Comparable, Nullable {
+
+final static CIDR NULL = new NullCIDR();
+final static int MAX_CIDR = 32;
+
+public InetAddress getBaseAddress();
+
+public int getMask();
+
+public boolean contains(InetAddress inetAddress);
+
+public boolean contains(CIDR cidr);
+
+public boolean overlaps(CIDR cidr);
+
+public String toString();
+
+public boolean equals(Object o);
+
+public int hashCode();
+
+class NullCIDR implements CIDR {
+
+private NullCIDR() {
+super();
+}
+
+public boolean isNull() {
+return true;
+}
+
+@Override
+public String toString() {
+return "Null CIDR";
+}
+
+@Override
+public InetAddress getBaseAddress() {
+// TODO Auto-generated method stub
+return null;
+}
+
+@Override
+public int getMask() {
+// TODO Auto-generated method stub
+return 0;
+}
+
+@Override
+public boolean contains(CIDR cidr) {
+// TODO Auto-generated method stub
+return false;
+}
+
+@Override
+public boolean overlaps(CIDR cidr) {
+// TODO Auto-generated method stub
+return false;
+}
+
+@Override
+public boolean contains(InetAddress cidr) {
+// TODO Auto-generated method stub
+return false;
+}
+
+@Override
+public int compareTo(CIDR o) {
+// TODO Auto-generated method stub
--- End diff --

Please address and remove the TODO


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787129
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR.java ---
@@ -0,0 +1,98 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.InetAddress;
+import com.cloud.utils.Nullable;
+
+public interface CIDR extends Comparable, Nullable {
+
+final static CIDR NULL = new NullCIDR();
+final static int MAX_CIDR = 32;
+
+public InetAddress getBaseAddress();
+
+public int getMask();
+
+public boolean contains(InetAddress inetAddress);
+
+public boolean contains(CIDR cidr);
+
+public boolean overlaps(CIDR cidr);
+
+public String toString();
+
+public boolean equals(Object o);
+
+public int hashCode();
+
+class NullCIDR implements CIDR {
+
+private NullCIDR() {
+super();
+}
+
+public boolean isNull() {
+return true;
+}
+
+@Override
+public String toString() {
+return "Null CIDR";
+}
+
+@Override
+public InetAddress getBaseAddress() {
+// TODO Auto-generated method stub
+return null;
+}
+
+@Override
+public int getMask() {
+// TODO Auto-generated method stub
+return 0;
+}
+
+@Override
+public boolean contains(CIDR cidr) {
+// TODO Auto-generated method stub
--- End diff --

Please address and remove the TODO


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787143
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR.java ---
@@ -0,0 +1,98 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.InetAddress;
+import com.cloud.utils.Nullable;
+
+public interface CIDR extends Comparable, Nullable {
+
+final static CIDR NULL = new NullCIDR();
+final static int MAX_CIDR = 32;
+
+public InetAddress getBaseAddress();
+
+public int getMask();
+
+public boolean contains(InetAddress inetAddress);
+
+public boolean contains(CIDR cidr);
+
+public boolean overlaps(CIDR cidr);
+
+public String toString();
+
+public boolean equals(Object o);
+
+public int hashCode();
+
+class NullCIDR implements CIDR {
+
+private NullCIDR() {
+super();
+}
+
+public boolean isNull() {
+return true;
+}
+
+@Override
+public String toString() {
+return "Null CIDR";
+}
+
+@Override
+public InetAddress getBaseAddress() {
+// TODO Auto-generated method stub
+return null;
+}
+
+@Override
+public int getMask() {
+// TODO Auto-generated method stub
+return 0;
+}
+
+@Override
+public boolean contains(CIDR cidr) {
+// TODO Auto-generated method stub
+return false;
+}
+
+@Override
+public boolean overlaps(CIDR cidr) {
+// TODO Auto-generated method stub
+return false;
+}
+
+@Override
+public boolean contains(InetAddress cidr) {
+// TODO Auto-generated method stub
--- End diff --

Please address and remove the TODO


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787123
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR.java ---
@@ -0,0 +1,98 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.InetAddress;
+import com.cloud.utils.Nullable;
+
+public interface CIDR extends Comparable, Nullable {
+
+final static CIDR NULL = new NullCIDR();
+final static int MAX_CIDR = 32;
+
+public InetAddress getBaseAddress();
+
+public int getMask();
+
+public boolean contains(InetAddress inetAddress);
+
+public boolean contains(CIDR cidr);
+
+public boolean overlaps(CIDR cidr);
+
+public String toString();
+
+public boolean equals(Object o);
+
+public int hashCode();
+
+class NullCIDR implements CIDR {
+
+private NullCIDR() {
+super();
+}
+
+public boolean isNull() {
+return true;
+}
+
+@Override
+public String toString() {
+return "Null CIDR";
+}
+
+@Override
+public InetAddress getBaseAddress() {
+// TODO Auto-generated method stub
+return null;
+}
+
+@Override
+public int getMask() {
+// TODO Auto-generated method stub
--- End diff --

Please address and remove the TODO


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787065
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR.java ---
@@ -0,0 +1,98 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.InetAddress;
+import com.cloud.utils.Nullable;
+
+public interface CIDR extends Comparable, Nullable {
--- End diff --

All interface methods on a ``public`` interface are inherently ``public``.  
Therefore, the ``public`` modifiers can be removed.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62787032
  
--- Diff: utils/src/main/java/com/cloud/utils/net/cidr/CIDR.java ---
@@ -0,0 +1,98 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.net.cidr;
+
+import java.net.InetAddress;
+import com.cloud.utils.Nullable;
+
+public interface CIDR extends Comparable, Nullable {
+
+final static CIDR NULL = new NullCIDR();
+final static int MAX_CIDR = 32;
+
+public InetAddress getBaseAddress();
+
+public int getMask();
+
+public boolean contains(InetAddress inetAddress);
+
+public boolean contains(CIDR cidr);
+
+public boolean overlaps(CIDR cidr);
+
+public String toString();
+
+public boolean equals(Object o);
+
+public int hashCode();
--- End diff --

It is unnecessary to declare ``toString``, ``equals``, and ``hashCode`` on 
an interface since all classes are ancestors of ``Object``.  Therefore, these 
method declarations can be removed.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786873
  
--- Diff: 
utils/src/main/java/com/cloud/utils/net/cidr/BadCIDRException.java ---
@@ -0,0 +1,33 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+package com.cloud.utils.net.cidr;
+
+import java.io.IOException;
+
+public class BadCIDRException extends IOException {
+
+private static final long serialVersionUID = -432126076052875403L;
+
+public BadCIDRException(String msg) {
--- End diff --

Consider accepting only the bad CIDR value on the constructor and 
encapsulating the creation of the error message.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786802
  
--- Diff: 
utils/src/main/java/com/cloud/utils/net/cidr/BadCIDRException.java ---
@@ -0,0 +1,33 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+package com.cloud.utils.net.cidr;
+
+import java.io.IOException;
+
+public class BadCIDRException extends IOException {
--- End diff --

Consider adding the value of invalid CIDR as member of this 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786731
  
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -1159,6 +1181,25 @@ public static boolean validateGuestCidr(final String 
cidr) {
 }
 }
 
+public static boolean validateGuestCidr(final CIDR cidr) throws 
BadCIDRException {
+// RFC 1918 - The Internet Assigned Numbers Authority (IANA) has 
reserved the
+// following three blocks of the IP address space for private 
internets:
+// 10.0.0.0 - 10.255.255.255 (10/8 prefix)
+// 172.16.0.0 - 172.31.255.255 (172.16/12 prefix)
+// 192.168.0.0 - 192.168.255.255 (192.168/16 prefix)
+
+final CIDR cidr1 = CIDRFactory.getCIDR("10.0.0.0/8");
+final CIDR cidr2 = CIDRFactory.getCIDR("172.16.0.0/12");
+final CIDR cidr3 = CIDRFactory.getCIDR("192.168.0.0/16");
--- End diff --

Consider moving these values to a ``final static ImmutableSet`` on the IPv4 
CIDR class.  The ``CIDR`` interface could also provide ``getGuestCIDRs`` method.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786638
  
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -1159,6 +1181,25 @@ public static boolean validateGuestCidr(final String 
cidr) {
 }
 }
 
+public static boolean validateGuestCidr(final CIDR cidr) throws 
BadCIDRException {
--- End diff --

Why is this method implemented here rather on ``CIDR`` for IPv4/IPv6 dual 
stack operation?


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786538
  
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -1135,6 +1142,21 @@ public static boolean isSameIpRange(final String 
cidrA, final String cidrB) {
 return false;
 }
 
+public static boolean validateGuestCidrForOSPF(final CIDR cidr, final 
CIDR[] ospfSuperCIDRList) {
--- End diff --

Consider moving the method to the ``Protocol`` enumeration.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786497
  
--- Diff: utils/src/main/java/com/cloud/utils/net/NetUtils.java ---
@@ -903,6 +906,10 @@ static boolean areCidrsNotEmpty(String cidrA, String 
cidrB) {
 
 }
 
+public static Long[] cidrToLong(final CIDR ocidr) {
+return cidrToLong(ocidr.toString());
+}
--- End diff --

Why is this method implemented here rather than on ``CIDR`` 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786345
  
--- Diff: test/integration/smoke/test_ospf_zone_config.py ---
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" Test cases for checking quagga API
+"""
+
+# Import Local Modules
+import marvin
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from marvin.lib.utils import (random_gen)
+from nose.plugins.attrib import attr
+
+# Import System modules
+import time
+
+class TestQuagga(cloudstackTestCase):
+
+@classmethod
+def setUpClass(cls):
+# Create Account
+testClient = super(TestQuagga, cls).getClsTestClient()
+cls.apiclient = testClient.getApiClient()
+cls.services = testClient.getParsedTestDataConfig()
+
+# Get Zone, Domain
+cls.domain = get_domain(cls.apiclient)
+cls.zone = get_zone(cls.apiclient, 
cls.testClient.getZoneForTests())
+
+return
+
+def setUp(self):
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+return
+
+def tearDown(self):
+try:
+# Clean up, terminate the created templates
+cleanup_resources(self.apiclient, self.cleanup)
+except Exception as e:
+raise Exception("Warning: Exception during cleanup : %s" % e)
+return
+
+# Check vpcQuaggaConfigCmd positive test case
+#VPCQuaggaConfigUpdateCmd [zoneid=1, quaggaProtocol=null, 
ospfArea=null, quaggaHelloInterval=null, quaggaDeadInterval=null, 
quaggaRetransmitInterval=null, quaggaTransitDelay=null, 
quaggaAuthentication=null, quaggaPassword=junk, ospfSuperCIDR=null, 
quaggaEnabled=null]
+@attr(tags=["smoke", "advanced"], required_hardware="false")
+def test_01_quaggaConfig(self):
+
+cmd = vpcQuaggaConfig.vpcQuaggaConfigCmd()
+cmd.zoneid = self.zone.id
+response = self.apiclient.vpcQuaggaConfig(cmd)
+self.debug("Response quaggaEnabled: %s" % response.quaggaEnabled)
+self.debug("Response quaggaPassword: %s" % response.quaggaPassword)
+self.debug("Response ospfSuperCIDR: %s" % response.ospfSuperCIDR)
+
+
+cmd = vpcQuaggaConfigUpdate.vpcQuaggaConfigUpdateCmd()
+cmd.zoneid = self.zone.id
+cmd.ospfSuperCIDR = "200.100.0.0/18,216.100.0.0/20"
+cmd.quaggaEnabled = "true"
+cmd.quaggaPassword = 
"6P33lsadEbmiJ7ZN7gK8vylOY2dTza3rAuY8UFboH7I=:Xb1G4VhzBkSXmTa8vDqExQQo8PiIUUSNtpxmdOkX1z8="
+response = self.apiclient.vpcQuaggaConfigUpdate(cmd)
+self.debug("Response quaggaEnabled: %s" % response.quaggaEnabled)
+self.debug("Response quaggaPassword: %s" % response.quaggaPassword)
+self.debug("Response ospfSuperCIDR: %s" % response.ospfSuperCIDR)
+
+
+cmd = vpcQuaggaConfig.vpcQuaggaConfigCmd()
+cmd.zoneid = self.zone.id
+response = self.apiclient.vpcQuaggaConfig(cmd)
+self.debug("Response quaggaEnabled: %s" % response.quaggaEnabled)
+self.debug("Response quaggaPassword: %s" % response.quaggaPassword)
+self.debug("Response ospfSuperCIDR: %s" % response.ospfSuperCIDR)
+
+cmd.zoneid = self.zone.id
+
+
+self.assertEqual(
+ response.quaggaEnabled, "true"
+

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786260
  
--- Diff: test/integration/smoke/test_ospf_zone_config.py ---
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" Test cases for checking quagga API
+"""
+
+# Import Local Modules
+import marvin
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from marvin.lib.utils import (random_gen)
+from nose.plugins.attrib import attr
+
+# Import System modules
+import time
+
+class TestQuagga(cloudstackTestCase):
+
+@classmethod
+def setUpClass(cls):
+# Create Account
+testClient = super(TestQuagga, cls).getClsTestClient()
+cls.apiclient = testClient.getApiClient()
+cls.services = testClient.getParsedTestDataConfig()
+
+# Get Zone, Domain
+cls.domain = get_domain(cls.apiclient)
+cls.zone = get_zone(cls.apiclient, 
cls.testClient.getZoneForTests())
+
+return
+
+def setUp(self):
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+return
+
+def tearDown(self):
+try:
+# Clean up, terminate the created templates
+cleanup_resources(self.apiclient, self.cleanup)
+except Exception as e:
+raise Exception("Warning: Exception during cleanup : %s" % e)
+return
+
+# Check vpcQuaggaConfigCmd positive test case
+#VPCQuaggaConfigUpdateCmd [zoneid=1, quaggaProtocol=null, 
ospfArea=null, quaggaHelloInterval=null, quaggaDeadInterval=null, 
quaggaRetransmitInterval=null, quaggaTransitDelay=null, 
quaggaAuthentication=null, quaggaPassword=junk, ospfSuperCIDR=null, 
quaggaEnabled=null]
+@attr(tags=["smoke", "advanced"], required_hardware="false")
+def test_01_quaggaConfig(self):
+
+cmd = vpcQuaggaConfig.vpcQuaggaConfigCmd()
+cmd.zoneid = self.zone.id
+response = self.apiclient.vpcQuaggaConfig(cmd)
+self.debug("Response quaggaEnabled: %s" % response.quaggaEnabled)
+self.debug("Response quaggaPassword: %s" % response.quaggaPassword)
+self.debug("Response ospfSuperCIDR: %s" % response.ospfSuperCIDR)
+
+
+cmd = vpcQuaggaConfigUpdate.vpcQuaggaConfigUpdateCmd()
+cmd.zoneid = self.zone.id
+cmd.ospfSuperCIDR = "200.100.0.0/18,216.100.0.0/20"
+cmd.quaggaEnabled = "true"
+cmd.quaggaPassword = 
"6P33lsadEbmiJ7ZN7gK8vylOY2dTza3rAuY8UFboH7I=:Xb1G4VhzBkSXmTa8vDqExQQo8PiIUUSNtpxmdOkX1z8="
+response = self.apiclient.vpcQuaggaConfigUpdate(cmd)
+self.debug("Response quaggaEnabled: %s" % response.quaggaEnabled)
+self.debug("Response quaggaPassword: %s" % response.quaggaPassword)
+self.debug("Response ospfSuperCIDR: %s" % response.ospfSuperCIDR)
+
+
+cmd = vpcQuaggaConfig.vpcQuaggaConfigCmd()
+cmd.zoneid = self.zone.id
+response = self.apiclient.vpcQuaggaConfig(cmd)
+self.debug("Response quaggaEnabled: %s" % response.quaggaEnabled)
+self.debug("Response quaggaPassword: %s" % response.quaggaPassword)
+self.debug("Response ospfSuperCIDR: %s" % response.ospfSuperCIDR)
+
+cmd.zoneid = self.zone.id
+
+
+self.assertEqual(
+ response.quaggaEnabled, "true"
+

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786232
  
--- Diff: test/integration/smoke/test_ospf_zone_config.py ---
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" Test cases for checking quagga API
+"""
+
+# Import Local Modules
+import marvin
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from marvin.lib.utils import (random_gen)
+from nose.plugins.attrib import attr
+
+# Import System modules
+import time
+
+class TestQuagga(cloudstackTestCase):
+
+@classmethod
+def setUpClass(cls):
+# Create Account
+testClient = super(TestQuagga, cls).getClsTestClient()
+cls.apiclient = testClient.getApiClient()
+cls.services = testClient.getParsedTestDataConfig()
+
+# Get Zone, Domain
+cls.domain = get_domain(cls.apiclient)
+cls.zone = get_zone(cls.apiclient, 
cls.testClient.getZoneForTests())
+
+return
+
+def setUp(self):
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+return
+
+def tearDown(self):
+try:
+# Clean up, terminate the created templates
+cleanup_resources(self.apiclient, self.cleanup)
+except Exception as e:
+raise Exception("Warning: Exception during cleanup : %s" % e)
+return
+
+# Check vpcQuaggaConfigCmd positive test case
+#VPCQuaggaConfigUpdateCmd [zoneid=1, quaggaProtocol=null, 
ospfArea=null, quaggaHelloInterval=null, quaggaDeadInterval=null, 
quaggaRetransmitInterval=null, quaggaTransitDelay=null, 
quaggaAuthentication=null, quaggaPassword=junk, ospfSuperCIDR=null, 
quaggaEnabled=null]
+@attr(tags=["smoke", "advanced"], required_hardware="false")
+def test_01_quaggaConfig(self):
+
+cmd = vpcQuaggaConfig.vpcQuaggaConfigCmd()
+cmd.zoneid = self.zone.id
+response = self.apiclient.vpcQuaggaConfig(cmd)
+self.debug("Response quaggaEnabled: %s" % response.quaggaEnabled)
+self.debug("Response quaggaPassword: %s" % response.quaggaPassword)
+self.debug("Response ospfSuperCIDR: %s" % response.ospfSuperCIDR)
+
+
+cmd = vpcQuaggaConfigUpdate.vpcQuaggaConfigUpdateCmd()
+cmd.zoneid = self.zone.id
+cmd.ospfSuperCIDR = "200.100.0.0/18,216.100.0.0/20"
+cmd.quaggaEnabled = "true"
+cmd.quaggaPassword = 
"6P33lsadEbmiJ7ZN7gK8vylOY2dTza3rAuY8UFboH7I=:Xb1G4VhzBkSXmTa8vDqExQQo8PiIUUSNtpxmdOkX1z8="
+response = self.apiclient.vpcQuaggaConfigUpdate(cmd)
+self.debug("Response quaggaEnabled: %s" % response.quaggaEnabled)
+self.debug("Response quaggaPassword: %s" % response.quaggaPassword)
+self.debug("Response ospfSuperCIDR: %s" % response.ospfSuperCIDR)
+
+
+cmd = vpcQuaggaConfig.vpcQuaggaConfigCmd()
+cmd.zoneid = self.zone.id
+response = self.apiclient.vpcQuaggaConfig(cmd)
+self.debug("Response quaggaEnabled: %s" % response.quaggaEnabled)
+self.debug("Response quaggaPassword: %s" % response.quaggaPassword)
+self.debug("Response ospfSuperCIDR: %s" % response.ospfSuperCIDR)
+
+cmd.zoneid = self.zone.id
+
+
+self.assertEqual(
+ response.quaggaEnabled, "true"
+

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786148
  
--- Diff: test/integration/smoke/test_ospf_zone_config.py ---
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" Test cases for checking quagga API
+"""
+
+# Import Local Modules
+import marvin
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from marvin.lib.utils import (random_gen)
+from nose.plugins.attrib import attr
+
+# Import System modules
+import time
+
+class TestQuagga(cloudstackTestCase):
+
+@classmethod
+def setUpClass(cls):
+# Create Account
+testClient = super(TestQuagga, cls).getClsTestClient()
+cls.apiclient = testClient.getApiClient()
+cls.services = testClient.getParsedTestDataConfig()
+
+# Get Zone, Domain
+cls.domain = get_domain(cls.apiclient)
+cls.zone = get_zone(cls.apiclient, 
cls.testClient.getZoneForTests())
+
+return
+
+def setUp(self):
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+return
+
+def tearDown(self):
+try:
+# Clean up, terminate the created templates
+cleanup_resources(self.apiclient, self.cleanup)
+except Exception as e:
+raise Exception("Warning: Exception during cleanup : %s" % e)
+return
+
+# Check vpcQuaggaConfigCmd positive test case
+#VPCQuaggaConfigUpdateCmd [zoneid=1, quaggaProtocol=null, 
ospfArea=null, quaggaHelloInterval=null, quaggaDeadInterval=null, 
quaggaRetransmitInterval=null, quaggaTransitDelay=null, 
quaggaAuthentication=null, quaggaPassword=junk, ospfSuperCIDR=null, 
quaggaEnabled=null]
--- End diff --

Please remove commented code to avoid accumulation of cruft.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786093
  
--- Diff: test/integration/component/test_ospf.py ---
@@ -0,0 +1,477 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" BVT tests for Service offerings"""
+
+# Import Local Modules
+from marvin.codes import FAILED
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from nose.plugins.attrib import attr
+from marvin.sshClient import SshClient
+
+
+_multiprocess_shared_ = True
+
+
+class TestCreateDynmaicServiceOffering(cloudstackTestCase):
+
+def setUp(self):
+
+self.logger = logging.getLogger('TestOSPF')
+self.stream_handler = logging.StreamHandler()
+self.logger.setLevel(logging.DEBUG)
+self.logger.addHandler(self.stream_handler)
+
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+print str(self)
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+self.services = { 
+ "batman_vpc": {
+ "name": "Batman VPC",
+ "displaytext": "Marvin Batman VPC",
+ "netmask": "255.255.252.0"
+ },
+ "superman_vpc": {
+ "name": "Superman VPC",
+ "displaytext": "Marvin Superman VPC",
+ "netmask": "255.255.252.0"
+ },
+  "vpc_offering": {
+"name": 'Cosmic VPC off',
+"displaytext": 'Cosmic VPC off',
+"supportedservices": 
'Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat,VPCDynamicRouting',
+},
+  "network_offering": {
+"name": 'Milkyway VPC Network 
offering',
+"displaytext": 'Milkyway VPC Network 
off',
+"guestiptype": 'Isolated',
+"supportedservices": 
'Vpn,Dhcp,Dns,SourceNat,PortForwarding,Lb,UserData,StaticNat,NetworkACL,VPCDynamicRouting',
+"traffictype": 'GUEST',
+"availability": 'Optional',
+"useVpc": 'on',
+"serviceProviderList": {
+"Vpn": 'VpcVirtualRouter',
+"Dhcp": 'VpcVirtualRouter',
+"Dns": 'VpcVirtualRouter',
+"SourceNat": 
'VpcVirtualRouter',
+"PortForwarding": 
'VpcVirtualRouter',
+"Lb": 'VpcVirtualRouter',
+"UserData": 'VpcVirtualRouter',
+"StaticNat": 
'VpcVirtualRouter',
+"VPCDynamicRouting": 
'VpcVirtualRouter',
+"NetworkACL": 
'VpcVirtualRouter'
+},
+},
+   "batman_network_tier1": {
+  

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62786006
  
--- Diff: test/integration/component/test_ospf.py ---
@@ -0,0 +1,477 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" BVT tests for Service offerings"""
+
+# Import Local Modules
+from marvin.codes import FAILED
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from nose.plugins.attrib import attr
+from marvin.sshClient import SshClient
+
+
+_multiprocess_shared_ = True
+
+
+class TestCreateDynmaicServiceOffering(cloudstackTestCase):
+
+def setUp(self):
+
+self.logger = logging.getLogger('TestOSPF')
+self.stream_handler = logging.StreamHandler()
+self.logger.setLevel(logging.DEBUG)
+self.logger.addHandler(self.stream_handler)
+
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+print str(self)
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+self.services = { 
+ "batman_vpc": {
+ "name": "Batman VPC",
+ "displaytext": "Marvin Batman VPC",
+ "netmask": "255.255.252.0"
+ },
+ "superman_vpc": {
+ "name": "Superman VPC",
+ "displaytext": "Marvin Superman VPC",
+ "netmask": "255.255.252.0"
+ },
+  "vpc_offering": {
+"name": 'Cosmic VPC off',
+"displaytext": 'Cosmic VPC off',
+"supportedservices": 
'Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat,VPCDynamicRouting',
+},
+  "network_offering": {
+"name": 'Milkyway VPC Network 
offering',
+"displaytext": 'Milkyway VPC Network 
off',
+"guestiptype": 'Isolated',
+"supportedservices": 
'Vpn,Dhcp,Dns,SourceNat,PortForwarding,Lb,UserData,StaticNat,NetworkACL,VPCDynamicRouting',
+"traffictype": 'GUEST',
+"availability": 'Optional',
+"useVpc": 'on',
+"serviceProviderList": {
+"Vpn": 'VpcVirtualRouter',
+"Dhcp": 'VpcVirtualRouter',
+"Dns": 'VpcVirtualRouter',
+"SourceNat": 
'VpcVirtualRouter',
+"PortForwarding": 
'VpcVirtualRouter',
+"Lb": 'VpcVirtualRouter',
+"UserData": 'VpcVirtualRouter',
+"StaticNat": 
'VpcVirtualRouter',
+"VPCDynamicRouting": 
'VpcVirtualRouter',
+"NetworkACL": 
'VpcVirtualRouter'
+},
+},
+   "batman_network_tier1": {
+  

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62785940
  
--- Diff: test/integration/component/test_ospf.py ---
@@ -0,0 +1,477 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" BVT tests for Service offerings"""
+
+# Import Local Modules
+from marvin.codes import FAILED
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from nose.plugins.attrib import attr
+from marvin.sshClient import SshClient
+
+
+_multiprocess_shared_ = True
+
+
+class TestCreateDynmaicServiceOffering(cloudstackTestCase):
+
+def setUp(self):
+
+self.logger = logging.getLogger('TestOSPF')
+self.stream_handler = logging.StreamHandler()
+self.logger.setLevel(logging.DEBUG)
+self.logger.addHandler(self.stream_handler)
+
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+print str(self)
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+self.services = { 
+ "batman_vpc": {
+ "name": "Batman VPC",
+ "displaytext": "Marvin Batman VPC",
+ "netmask": "255.255.252.0"
+ },
+ "superman_vpc": {
+ "name": "Superman VPC",
+ "displaytext": "Marvin Superman VPC",
+ "netmask": "255.255.252.0"
+ },
+  "vpc_offering": {
+"name": 'Cosmic VPC off',
+"displaytext": 'Cosmic VPC off',
+"supportedservices": 
'Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat,VPCDynamicRouting',
+},
+  "network_offering": {
+"name": 'Milkyway VPC Network 
offering',
+"displaytext": 'Milkyway VPC Network 
off',
+"guestiptype": 'Isolated',
+"supportedservices": 
'Vpn,Dhcp,Dns,SourceNat,PortForwarding,Lb,UserData,StaticNat,NetworkACL,VPCDynamicRouting',
+"traffictype": 'GUEST',
+"availability": 'Optional',
+"useVpc": 'on',
+"serviceProviderList": {
+"Vpn": 'VpcVirtualRouter',
+"Dhcp": 'VpcVirtualRouter',
+"Dns": 'VpcVirtualRouter',
+"SourceNat": 
'VpcVirtualRouter',
+"PortForwarding": 
'VpcVirtualRouter',
+"Lb": 'VpcVirtualRouter',
+"UserData": 'VpcVirtualRouter',
+"StaticNat": 
'VpcVirtualRouter',
+"VPCDynamicRouting": 
'VpcVirtualRouter',
+"NetworkACL": 
'VpcVirtualRouter'
+},
+},
+   "batman_network_tier1": {
+  

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62785864
  
--- Diff: test/integration/component/test_ospf.py ---
@@ -0,0 +1,477 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" BVT tests for Service offerings"""
+
+# Import Local Modules
+from marvin.codes import FAILED
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from nose.plugins.attrib import attr
+from marvin.sshClient import SshClient
+
+
+_multiprocess_shared_ = True
+
+
+class TestCreateDynmaicServiceOffering(cloudstackTestCase):
+
+def setUp(self):
+
+self.logger = logging.getLogger('TestOSPF')
+self.stream_handler = logging.StreamHandler()
+self.logger.setLevel(logging.DEBUG)
+self.logger.addHandler(self.stream_handler)
+
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+print str(self)
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+self.services = { 
+ "batman_vpc": {
+ "name": "Batman VPC",
+ "displaytext": "Marvin Batman VPC",
+ "netmask": "255.255.252.0"
+ },
+ "superman_vpc": {
+ "name": "Superman VPC",
+ "displaytext": "Marvin Superman VPC",
+ "netmask": "255.255.252.0"
+ },
+  "vpc_offering": {
+"name": 'Cosmic VPC off',
+"displaytext": 'Cosmic VPC off',
+"supportedservices": 
'Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat,VPCDynamicRouting',
+},
+  "network_offering": {
+"name": 'Milkyway VPC Network 
offering',
+"displaytext": 'Milkyway VPC Network 
off',
+"guestiptype": 'Isolated',
+"supportedservices": 
'Vpn,Dhcp,Dns,SourceNat,PortForwarding,Lb,UserData,StaticNat,NetworkACL,VPCDynamicRouting',
+"traffictype": 'GUEST',
+"availability": 'Optional',
+"useVpc": 'on',
+"serviceProviderList": {
+"Vpn": 'VpcVirtualRouter',
+"Dhcp": 'VpcVirtualRouter',
+"Dns": 'VpcVirtualRouter',
+"SourceNat": 
'VpcVirtualRouter',
+"PortForwarding": 
'VpcVirtualRouter',
+"Lb": 'VpcVirtualRouter',
+"UserData": 'VpcVirtualRouter',
+"StaticNat": 
'VpcVirtualRouter',
+"VPCDynamicRouting": 
'VpcVirtualRouter',
+"NetworkACL": 
'VpcVirtualRouter'
+},
+},
+   "batman_network_tier1": {
+  

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62785806
  
--- Diff: test/integration/component/test_ospf.py ---
@@ -0,0 +1,477 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" BVT tests for Service offerings"""
+
+# Import Local Modules
+from marvin.codes import FAILED
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from nose.plugins.attrib import attr
+from marvin.sshClient import SshClient
+
+
+_multiprocess_shared_ = True
+
+
+class TestCreateDynmaicServiceOffering(cloudstackTestCase):
+
+def setUp(self):
+
+self.logger = logging.getLogger('TestOSPF')
+self.stream_handler = logging.StreamHandler()
+self.logger.setLevel(logging.DEBUG)
+self.logger.addHandler(self.stream_handler)
+
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+print str(self)
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+self.services = { 
+ "batman_vpc": {
+ "name": "Batman VPC",
+ "displaytext": "Marvin Batman VPC",
+ "netmask": "255.255.252.0"
+ },
+ "superman_vpc": {
+ "name": "Superman VPC",
+ "displaytext": "Marvin Superman VPC",
+ "netmask": "255.255.252.0"
+ },
+  "vpc_offering": {
+"name": 'Cosmic VPC off',
+"displaytext": 'Cosmic VPC off',
+"supportedservices": 
'Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat,VPCDynamicRouting',
+},
+  "network_offering": {
+"name": 'Milkyway VPC Network 
offering',
+"displaytext": 'Milkyway VPC Network 
off',
+"guestiptype": 'Isolated',
+"supportedservices": 
'Vpn,Dhcp,Dns,SourceNat,PortForwarding,Lb,UserData,StaticNat,NetworkACL,VPCDynamicRouting',
+"traffictype": 'GUEST',
+"availability": 'Optional',
+"useVpc": 'on',
+"serviceProviderList": {
+"Vpn": 'VpcVirtualRouter',
+"Dhcp": 'VpcVirtualRouter',
+"Dns": 'VpcVirtualRouter',
+"SourceNat": 
'VpcVirtualRouter',
+"PortForwarding": 
'VpcVirtualRouter',
+"Lb": 'VpcVirtualRouter',
+"UserData": 'VpcVirtualRouter',
+"StaticNat": 
'VpcVirtualRouter',
+"VPCDynamicRouting": 
'VpcVirtualRouter',
+"NetworkACL": 
'VpcVirtualRouter'
+},
+},
+   "batman_network_tier1": {
+  

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62785711
  
--- Diff: test/integration/component/test_ospf.py ---
@@ -0,0 +1,477 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" BVT tests for Service offerings"""
+
+# Import Local Modules
+from marvin.codes import FAILED
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from nose.plugins.attrib import attr
+from marvin.sshClient import SshClient
+
+
+_multiprocess_shared_ = True
+
+
+class TestCreateDynmaicServiceOffering(cloudstackTestCase):
+
+def setUp(self):
+
+self.logger = logging.getLogger('TestOSPF')
+self.stream_handler = logging.StreamHandler()
+self.logger.setLevel(logging.DEBUG)
+self.logger.addHandler(self.stream_handler)
+
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+print str(self)
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+self.services = { 
+ "batman_vpc": {
+ "name": "Batman VPC",
+ "displaytext": "Marvin Batman VPC",
+ "netmask": "255.255.252.0"
+ },
+ "superman_vpc": {
+ "name": "Superman VPC",
+ "displaytext": "Marvin Superman VPC",
+ "netmask": "255.255.252.0"
+ },
+  "vpc_offering": {
+"name": 'Cosmic VPC off',
+"displaytext": 'Cosmic VPC off',
+"supportedservices": 
'Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat,VPCDynamicRouting',
+},
+  "network_offering": {
+"name": 'Milkyway VPC Network 
offering',
+"displaytext": 'Milkyway VPC Network 
off',
+"guestiptype": 'Isolated',
+"supportedservices": 
'Vpn,Dhcp,Dns,SourceNat,PortForwarding,Lb,UserData,StaticNat,NetworkACL,VPCDynamicRouting',
+"traffictype": 'GUEST',
+"availability": 'Optional',
+"useVpc": 'on',
+"serviceProviderList": {
+"Vpn": 'VpcVirtualRouter',
+"Dhcp": 'VpcVirtualRouter',
+"Dns": 'VpcVirtualRouter',
+"SourceNat": 
'VpcVirtualRouter',
+"PortForwarding": 
'VpcVirtualRouter',
+"Lb": 'VpcVirtualRouter',
+"UserData": 'VpcVirtualRouter',
+"StaticNat": 
'VpcVirtualRouter',
+"VPCDynamicRouting": 
'VpcVirtualRouter',
+"NetworkACL": 
'VpcVirtualRouter'
+},
+},
+   "batman_network_tier1": {
+  

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62785662
  
--- Diff: test/integration/component/test_ospf.py ---
@@ -0,0 +1,477 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" BVT tests for Service offerings"""
+
+# Import Local Modules
+from marvin.codes import FAILED
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from nose.plugins.attrib import attr
+from marvin.sshClient import SshClient
+
+
+_multiprocess_shared_ = True
+
+
+class TestCreateDynmaicServiceOffering(cloudstackTestCase):
+
+def setUp(self):
+
+self.logger = logging.getLogger('TestOSPF')
+self.stream_handler = logging.StreamHandler()
+self.logger.setLevel(logging.DEBUG)
+self.logger.addHandler(self.stream_handler)
+
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+print str(self)
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+self.services = { 
+ "batman_vpc": {
+ "name": "Batman VPC",
+ "displaytext": "Marvin Batman VPC",
+ "netmask": "255.255.252.0"
+ },
+ "superman_vpc": {
+ "name": "Superman VPC",
+ "displaytext": "Marvin Superman VPC",
+ "netmask": "255.255.252.0"
+ },
+  "vpc_offering": {
+"name": 'Cosmic VPC off',
+"displaytext": 'Cosmic VPC off',
+"supportedservices": 
'Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat,VPCDynamicRouting',
+},
+  "network_offering": {
+"name": 'Milkyway VPC Network 
offering',
+"displaytext": 'Milkyway VPC Network 
off',
+"guestiptype": 'Isolated',
+"supportedservices": 
'Vpn,Dhcp,Dns,SourceNat,PortForwarding,Lb,UserData,StaticNat,NetworkACL,VPCDynamicRouting',
+"traffictype": 'GUEST',
+"availability": 'Optional',
+"useVpc": 'on',
+"serviceProviderList": {
+"Vpn": 'VpcVirtualRouter',
+"Dhcp": 'VpcVirtualRouter',
+"Dns": 'VpcVirtualRouter',
+"SourceNat": 
'VpcVirtualRouter',
+"PortForwarding": 
'VpcVirtualRouter',
+"Lb": 'VpcVirtualRouter',
+"UserData": 'VpcVirtualRouter',
+"StaticNat": 
'VpcVirtualRouter',
+"VPCDynamicRouting": 
'VpcVirtualRouter',
+"NetworkACL": 
'VpcVirtualRouter'
+},
+},
+   "batman_network_tier1": {
+  

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62785576
  
--- Diff: test/integration/component/test_ospf.py ---
@@ -0,0 +1,477 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+""" BVT tests for Service offerings"""
+
+# Import Local Modules
+from marvin.codes import FAILED
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+from nose.plugins.attrib import attr
+from marvin.sshClient import SshClient
+
+
+_multiprocess_shared_ = True
+
+
+class TestCreateDynmaicServiceOffering(cloudstackTestCase):
+
+def setUp(self):
+
+self.logger = logging.getLogger('TestOSPF')
+self.stream_handler = logging.StreamHandler()
+self.logger.setLevel(logging.DEBUG)
+self.logger.addHandler(self.stream_handler)
+
+self.apiclient = self.testClient.getApiClient()
+self.hypervisor = self.testClient.getHypervisorInfo()
+print str(self)
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, 
self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.cleanup = []
+self.services = { 
+ "batman_vpc": {
+ "name": "Batman VPC",
+ "displaytext": "Marvin Batman VPC",
+ "netmask": "255.255.252.0"
+ },
+ "superman_vpc": {
+ "name": "Superman VPC",
+ "displaytext": "Marvin Superman VPC",
+ "netmask": "255.255.252.0"
+ },
+  "vpc_offering": {
+"name": 'Cosmic VPC off',
+"displaytext": 'Cosmic VPC off',
+"supportedservices": 
'Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat,VPCDynamicRouting',
+},
+  "network_offering": {
+"name": 'Milkyway VPC Network 
offering',
+"displaytext": 'Milkyway VPC Network 
off',
+"guestiptype": 'Isolated',
+"supportedservices": 
'Vpn,Dhcp,Dns,SourceNat,PortForwarding,Lb,UserData,StaticNat,NetworkACL,VPCDynamicRouting',
+"traffictype": 'GUEST',
+"availability": 'Optional',
+"useVpc": 'on',
+"serviceProviderList": {
+"Vpn": 'VpcVirtualRouter',
+"Dhcp": 'VpcVirtualRouter',
+"Dns": 'VpcVirtualRouter',
+"SourceNat": 
'VpcVirtualRouter',
+"PortForwarding": 
'VpcVirtualRouter',
+"Lb": 'VpcVirtualRouter',
+"UserData": 'VpcVirtualRouter',
+"StaticNat": 
'VpcVirtualRouter',
+"VPCDynamicRouting": 
'VpcVirtualRouter',
+"NetworkACL": 
'VpcVirtualRouter'
+},
+},
+   "batman_network_tier1": {
+  

[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62785361
  
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/cs/CsQuagga.py ---
@@ -0,0 +1,85 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import logging
+import os.path
+import re
+from cs.CsDatabag import CsDataBag
+from CsProcess import CsProcess
+from CsFile import CsFile
+import CsHelper
+
+OSPFD_CONF = "/etc/quagga/ospfd.conf"
+OSPFD_CONF_NEW = "/etc/quagga/ospfd.conf.new"
+ZEBRA_CONF = "/etc/quagga/zebra.conf"
+ZEBRA_CONF_NEW = "/etc/quagga/zebra.conf.new"
+
+
+class CsQuagga(CsDataBag):
+""" Manage Load Balancer entries """
+
+def process(self):
+#create quagga config
+logging.debug("CsQuagga: zebra" + str(self.dbag["zebra_config"]))
+logging.debug("CsQuagga: ospfd" + str(self.dbag["ospf_config"]))
+
+zebra_config = self.dbag["zebra_config"].split(',')
+ospf_config = self.dbag["ospf_config"].split(',')
+changed = False
+#process zebra
+zebra_conf_new = CsFile(ZEBRA_CONF_NEW)
+zebra_conf_new.empty()
+for x in zebra_config:
+[zebra_conf_new.append(w, -1) for w in x.split('\n')]
+
+zebra_conf_new.commit()
+zebra_conf = CsFile(ZEBRA_CONF)
+if not zebra_conf.compare(zebra_conf_new):
+CsHelper.copy(ZEBRA_CONF_NEW, ZEBRA_CONF)
+changed = True
+
+#process ospfd
+ospfd_conf_new = CsFile(OSPFD_CONF_NEW)
+ospfd_conf_new.empty()
+for x in ospf_config:
+[ospfd_conf_new.append(w, -1) for w in x.split('\n')]
+
+ospfd_conf_new.commit()
+ospfd_conf = CsFile(OSPFD_CONF)
+if not ospfd_conf.compare(ospfd_conf_new):
+CsHelper.copy(OSPFD_CONF_NEW, OSPFD_CONF)
+changed = True
+
+if changed:
+#reset zebra and quagga
+proc = CsProcess(['/var/run/quagga.pid'])
+logging.debug("CsQuagga: Resetted quagga ")
+if not proc.find():
+logging.debug("CsQuagga:: will restart Quagga!")
+CsHelper.service("quagga", "restart")
+else:
+logging.debug("CsQuagga:: will reload Quagga!")
+CsHelper.service("quagga", "reload")
--- End diff --

Where is zebra restarted?


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62785317
  
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/cs/CsQuagga.py ---
@@ -0,0 +1,85 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import logging
+import os.path
+import re
+from cs.CsDatabag import CsDataBag
+from CsProcess import CsProcess
+from CsFile import CsFile
+import CsHelper
+
+OSPFD_CONF = "/etc/quagga/ospfd.conf"
+OSPFD_CONF_NEW = "/etc/quagga/ospfd.conf.new"
+ZEBRA_CONF = "/etc/quagga/zebra.conf"
+ZEBRA_CONF_NEW = "/etc/quagga/zebra.conf.new"
+
+
+class CsQuagga(CsDataBag):
+""" Manage Load Balancer entries """
+
+def process(self):
+#create quagga config
+logging.debug("CsQuagga: zebra" + str(self.dbag["zebra_config"]))
+logging.debug("CsQuagga: ospfd" + str(self.dbag["ospf_config"]))
+
+zebra_config = self.dbag["zebra_config"].split(',')
+ospf_config = self.dbag["ospf_config"].split(',')
+changed = False
+#process zebra
+zebra_conf_new = CsFile(ZEBRA_CONF_NEW)
+zebra_conf_new.empty()
+for x in zebra_config:
+[zebra_conf_new.append(w, -1) for w in x.split('\n')]
+
+zebra_conf_new.commit()
+zebra_conf = CsFile(ZEBRA_CONF)
+if not zebra_conf.compare(zebra_conf_new):
+CsHelper.copy(ZEBRA_CONF_NEW, ZEBRA_CONF)
+changed = True
+
+#process ospfd
+ospfd_conf_new = CsFile(OSPFD_CONF_NEW)
+ospfd_conf_new.empty()
+for x in ospf_config:
+[ospfd_conf_new.append(w, -1) for w in x.split('\n')]
+
+ospfd_conf_new.commit()
+ospfd_conf = CsFile(OSPFD_CONF)
+if not ospfd_conf.compare(ospfd_conf_new):
+CsHelper.copy(OSPFD_CONF_NEW, OSPFD_CONF)
+changed = True
--- End diff --

Lines 43-52 and 55-64 appear to perform identical logic.  Consider 
extracting a generic, private function such as ``create_conf_file(filename, 
new_config)`` that returns (boolean, CsFile).  Lines 36-40 could also be 
collapsed into this function.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62783518
  
--- Diff: 
server/src/org/apache/cloudstack/network/topology/BasicNetworkVisitor.java ---
@@ -316,4 +317,9 @@ public boolean visit(final StaticRoutesRules 
staticRoutesRules) throws ResourceU
 public boolean visit(final AdvancedVpnRules vpnRules) throws 
ResourceUnavailableException {
 throw new CloudRuntimeException("AdvancedVpnRules not implemented 
in Basic Network Topology.");
 }
+
+@Override
+public boolean visit(final QuaggaRules vpnRules) throws 
ResourceUnavailableException {
+throw new CloudRuntimeException("AdvancedVpnRules not implemented 
in Basic Network Topology.");
--- End diff --

Consider using the ``OperationNotSupported`` exception provided by the Java 
runtime for this purpose.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62783468
  
--- Diff: 
server/src/org/apache/cloudstack/network/topology/AdvancedNetworkVisitor.java 
---
@@ -212,4 +214,20 @@ public boolean visit(final AdvancedVpnRules vpnRules) 
throws ResourceUnavailable
 // results accordingly
 return _networkGeneralHelper.sendCommandsToRouter(router, cmds);
 }
+
+@Override
+public boolean visit(final QuaggaRules quaggaRules) throws 
ResourceUnavailableException {
+final VirtualRouter router = quaggaRules.getRouter();
+
+final Commands cmds = new Commands(Command.OnError.Continue);
+
+try {
+_commandSetupHelper.createQuaggaConfigCommand(router, 
quaggaRules.getVpcId(), cmds);
+} catch (BadCIDRException ex) {
+s_logger.warn("Failed to create Quagga config due to faulty 
cidr ");
--- End diff --

Please log the exception to support operational debugging.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62783380
  
--- Diff: server/src/com/cloud/server/ConfigurationServerImpl.java ---
@@ -1236,18 +1218,34 @@ public void 
doInTransactionWithoutResult(TransactionStatus status) {
 
defaultVpcNetworkOfferingProviders.put(Service.PortForwarding, 
Provider.VPCVirtualRouter);
 defaultVpcNetworkOfferingProviders.put(Service.Vpn, 
Provider.VPCVirtualRouter);
 
-for (Map.Entry entry : 
defaultVpcNetworkOfferingProviders.entrySet()) {
- NetworkOfferingServiceMapVO offService =
-new 
NetworkOfferingServiceMapVO(defaultNetworkOfferingForVpcNetworks.getId(), 
entry.getKey(), entry.getValue());
+for (Map.Entry entry : 
defaultVpcNetworkOfferingProviders.entrySet()) {
+NetworkOfferingServiceMapVO offService = new 
NetworkOfferingServiceMapVO(defaultNetworkOfferingForVpcNetworks.getId(), 
entry.getKey(), entry.getValue());
+_ntwkOfferingServiceMapDao.persist(offService);
+s_logger.trace("Added service for the network 
offering: " + offService);
--- End diff --

Please wrap this log statement in a ``isTraceEnabled()`` check to avoid 
unnecessary string concatenation when ``TRACE`` logging is not enabled.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62783052
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -809,15 +945,22 @@ public Vpc createVpc(final long zoneId, final long 
vpcOffId, final long vpcOwner
 
 @DB
 protected Vpc createVpc(final Boolean displayVpc, final VpcVO vpc) {
-final String cidr = vpc.getCidr();
-// Validate CIDR
-if (!NetUtils.isValidCIDR(cidr)) {
-throw new InvalidParameterValueException("Invalid CIDR 
specified " + cidr);
-}
 
-// cidr has to be RFC 1918 complient
-if (!NetUtils.validateGuestCidr(cidr)) {
-throw new InvalidParameterValueException("Guest Cidr " + cidr 
+ " is not RFC1918 compliant");
+try {
+final CIDR cidr = CIDRFactory.getCIDR(vpc.getCidr());
+
+final CIDR[] superCidr = getZoneSuperCidrList(vpc.zoneId);
+
+if 
(_vpcOffServiceDao.areServicesSupportedByNetworkOffering(vpc.getVpcOfferingId(),
 Service.VPCDynamicRouting)) {
+if (!NetUtils.validateGuestCidrForOSPF(cidr, superCidr)) {
+throw new InvalidParameterValueException("Guest Cidr " 
+ cidr + " is not within Zone superCIDR " + Arrays.toString(superCidr));
+}
+} else if (!NetUtils.validateGuestCidr(cidr)) {
--- End diff --

Can this validation be moved to the ``CIDR`` interface to support IPv4/IPv6 
dual stack operation?


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62783039
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -809,15 +945,22 @@ public Vpc createVpc(final long zoneId, final long 
vpcOffId, final long vpcOwner
 
 @DB
 protected Vpc createVpc(final Boolean displayVpc, final VpcVO vpc) {
-final String cidr = vpc.getCidr();
-// Validate CIDR
-if (!NetUtils.isValidCIDR(cidr)) {
-throw new InvalidParameterValueException("Invalid CIDR 
specified " + cidr);
-}
 
-// cidr has to be RFC 1918 complient
-if (!NetUtils.validateGuestCidr(cidr)) {
-throw new InvalidParameterValueException("Guest Cidr " + cidr 
+ " is not RFC1918 compliant");
+try {
+final CIDR cidr = CIDRFactory.getCIDR(vpc.getCidr());
+
+final CIDR[] superCidr = getZoneSuperCidrList(vpc.zoneId);
+
+if 
(_vpcOffServiceDao.areServicesSupportedByNetworkOffering(vpc.getVpcOfferingId(),
 Service.VPCDynamicRouting)) {
+if (!NetUtils.validateGuestCidrForOSPF(cidr, superCidr)) {
--- End diff --

Can this validation be moved to the ``CIDR`` interface to support IPv4/IPv6 
dual stack operation?


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782933
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -747,6 +810,67 @@ public VpcOffering updateVpcOffering(final long 
vpcOffId, final String vpcOfferi
 }
 
 @Override
+public CIDR[] getZoneSuperCidrList(final long zoneId) throws 
BadCIDRException {
+final String superCIDRList = _dcDao.getDetail(zoneId, 
Params.SUPER_CIDR.name());
+if (superCIDRList != null) {
+String[] str_cidr_list = superCIDRList.split(",");
+CIDR[] cidr_list = NetUtils.convertToCIDR(str_cidr_list);
+if (!NetUtils.cidrListConsistency(cidr_list)) {
+throw new InvalidParameterValueException("The cidr list is 
not consistent " + Arrays.toString(cidr_list));
+}
+return cidr_list;
+} else {
+throw new InvalidParameterValueException("Zone super cidr is 
null check cloud.data_center_details for zone id=" + zoneId);
+}
+}
+
+private List getAllVpcCidrs() throws BadCIDRException {
+// get all the used cidrs
+List usedSubnets = new ArrayList();
+for (Vpc vpc : _vpcDao.listAll()) {
+if (_vpcSrvcDao.areServicesSupportedInVpc(vpc.getId(), 
Service.VPCDynamicRouting)) {
+usedSubnets.add(CIDRFactory.getCIDR(vpc.getCidr()));
+}
+}
+return usedSubnets;
+}
+
+@Override
+@ActionEvent(eventType = EventTypes.EVENT_VPC_CREATE, eventDescription 
= "creating vpc", create = true)
+public synchronized Vpc createDynamicVpc(final long zoneId, final long 
vpcOffId, final long vpcOwnerId, final String vpcName, final String 
displayText, final String netmask,
+String networkDomain, final Boolean displayVpc) throws 
ResourceAllocationException, BadCIDRException {
+if (netmask != null) {
+if (!NetUtils.isValidNetmask(netmask)) {
+throw new InvalidParameterValueException("Invalid netmask 
" + netmask);
+}
+final CIDR[] superCidr = getZoneSuperCidrList(zoneId);
+final List usedSubnets = getAllVpcCidrs();
+//find cidr that does not overlap with the used cidrs
+List allSubnets = NetUtils.getAllSubnets(superCidr, 
netmask);
+//check for overlap with usedSubnets
+CIDR unused_cidr = null;
+for (CIDR subnet : allSubnets) {
+if (NetUtils.isCidrOverlap(subnet, usedSubnets)) {
+continue;
+} else {
+unused_cidr = subnet;
+break;
+}
+}
--- End diff --

Consider moving this for loop to ``CIDR.findUnusedSubnet`` method and 
adding a unit test for 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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782870
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -747,6 +810,67 @@ public VpcOffering updateVpcOffering(final long 
vpcOffId, final String vpcOfferi
 }
 
 @Override
+public CIDR[] getZoneSuperCidrList(final long zoneId) throws 
BadCIDRException {
+final String superCIDRList = _dcDao.getDetail(zoneId, 
Params.SUPER_CIDR.name());
+if (superCIDRList != null) {
+String[] str_cidr_list = superCIDRList.split(",");
+CIDR[] cidr_list = NetUtils.convertToCIDR(str_cidr_list);
+if (!NetUtils.cidrListConsistency(cidr_list)) {
+throw new InvalidParameterValueException("The cidr list is 
not consistent " + Arrays.toString(cidr_list));
+}
+return cidr_list;
+} else {
+throw new InvalidParameterValueException("Zone super cidr is 
null check cloud.data_center_details for zone id=" + zoneId);
+}
+}
+
+private List getAllVpcCidrs() throws BadCIDRException {
+// get all the used cidrs
+List usedSubnets = new ArrayList();
+for (Vpc vpc : _vpcDao.listAll()) {
+if (_vpcSrvcDao.areServicesSupportedInVpc(vpc.getId(), 
Service.VPCDynamicRouting)) {
+usedSubnets.add(CIDRFactory.getCIDR(vpc.getCidr()));
+}
+}
+return usedSubnets;
+}
+
+@Override
+@ActionEvent(eventType = EventTypes.EVENT_VPC_CREATE, eventDescription 
= "creating vpc", create = true)
+public synchronized Vpc createDynamicVpc(final long zoneId, final long 
vpcOffId, final long vpcOwnerId, final String vpcName, final String 
displayText, final String netmask,
+String networkDomain, final Boolean displayVpc) throws 
ResourceAllocationException, BadCIDRException {
+if (netmask != null) {
+if (!NetUtils.isValidNetmask(netmask)) {
+throw new InvalidParameterValueException("Invalid netmask 
" + netmask);
+}
+final CIDR[] superCidr = getZoneSuperCidrList(zoneId);
+final List usedSubnets = getAllVpcCidrs();
+//find cidr that does not overlap with the used cidrs
+List allSubnets = NetUtils.getAllSubnets(superCidr, 
netmask);
+//check for overlap with usedSubnets
+CIDR unused_cidr = null;
+for (CIDR subnet : allSubnets) {
+if (NetUtils.isCidrOverlap(subnet, usedSubnets)) {
--- End diff --

Consider implementing this method on the ``CIDR`` 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782819
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -747,6 +810,67 @@ public VpcOffering updateVpcOffering(final long 
vpcOffId, final String vpcOfferi
 }
 
 @Override
+public CIDR[] getZoneSuperCidrList(final long zoneId) throws 
BadCIDRException {
+final String superCIDRList = _dcDao.getDetail(zoneId, 
Params.SUPER_CIDR.name());
+if (superCIDRList != null) {
+String[] str_cidr_list = superCIDRList.split(",");
+CIDR[] cidr_list = NetUtils.convertToCIDR(str_cidr_list);
+if (!NetUtils.cidrListConsistency(cidr_list)) {
+throw new InvalidParameterValueException("The cidr list is 
not consistent " + Arrays.toString(cidr_list));
+}
+return cidr_list;
+} else {
+throw new InvalidParameterValueException("Zone super cidr is 
null check cloud.data_center_details for zone id=" + zoneId);
+}
+}
+
+private List getAllVpcCidrs() throws BadCIDRException {
+// get all the used cidrs
+List usedSubnets = new ArrayList();
+for (Vpc vpc : _vpcDao.listAll()) {
+if (_vpcSrvcDao.areServicesSupportedInVpc(vpc.getId(), 
Service.VPCDynamicRouting)) {
+usedSubnets.add(CIDRFactory.getCIDR(vpc.getCidr()));
+}
+}
+return usedSubnets;
+}
+
+@Override
+@ActionEvent(eventType = EventTypes.EVENT_VPC_CREATE, eventDescription 
= "creating vpc", create = true)
+public synchronized Vpc createDynamicVpc(final long zoneId, final long 
vpcOffId, final long vpcOwnerId, final String vpcName, final String 
displayText, final String netmask,
+String networkDomain, final Boolean displayVpc) throws 
ResourceAllocationException, BadCIDRException {
+if (netmask != null) {
+if (!NetUtils.isValidNetmask(netmask)) {
+throw new InvalidParameterValueException("Invalid netmask 
" + netmask);
+}
+final CIDR[] superCidr = getZoneSuperCidrList(zoneId);
+final List usedSubnets = getAllVpcCidrs();
+//find cidr that does not overlap with the used cidrs
+List allSubnets = NetUtils.getAllSubnets(superCidr, 
netmask);
--- End diff --

Consider moving this method to the ``CIDR`` 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] cloudstack pull request: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782681
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -747,6 +810,67 @@ public VpcOffering updateVpcOffering(final long 
vpcOffId, final String vpcOfferi
 }
 
 @Override
+public CIDR[] getZoneSuperCidrList(final long zoneId) throws 
BadCIDRException {
+final String superCIDRList = _dcDao.getDetail(zoneId, 
Params.SUPER_CIDR.name());
+if (superCIDRList != null) {
+String[] str_cidr_list = superCIDRList.split(",");
+CIDR[] cidr_list = NetUtils.convertToCIDR(str_cidr_list);
+if (!NetUtils.cidrListConsistency(cidr_list)) {
+throw new InvalidParameterValueException("The cidr list is 
not consistent " + Arrays.toString(cidr_list));
+}
+return cidr_list;
+} else {
+throw new InvalidParameterValueException("Zone super cidr is 
null check cloud.data_center_details for zone id=" + zoneId);
+}
+}
+
+private List getAllVpcCidrs() throws BadCIDRException {
+// get all the used cidrs
+List usedSubnets = new ArrayList();
+for (Vpc vpc : _vpcDao.listAll()) {
+if (_vpcSrvcDao.areServicesSupportedInVpc(vpc.getId(), 
Service.VPCDynamicRouting)) {
+usedSubnets.add(CIDRFactory.getCIDR(vpc.getCidr()));
+}
+}
+return usedSubnets;
+}
+
+@Override
+@ActionEvent(eventType = EventTypes.EVENT_VPC_CREATE, eventDescription 
= "creating vpc", create = true)
+public synchronized Vpc createDynamicVpc(final long zoneId, final long 
vpcOffId, final long vpcOwnerId, final String vpcName, final String 
displayText, final String netmask,
+String networkDomain, final Boolean displayVpc) throws 
ResourceAllocationException, BadCIDRException {
+if (netmask != null) {
+if (!NetUtils.isValidNetmask(netmask)) {
--- End diff --

Please consider implementing this validation on the as a static method on 
the ``Netmask`` interface for IPv4/IPv6 dual stack support.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782579
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -747,6 +810,67 @@ public VpcOffering updateVpcOffering(final long 
vpcOffId, final String vpcOfferi
 }
 
 @Override
+public CIDR[] getZoneSuperCidrList(final long zoneId) throws 
BadCIDRException {
+final String superCIDRList = _dcDao.getDetail(zoneId, 
Params.SUPER_CIDR.name());
+if (superCIDRList != null) {
+String[] str_cidr_list = superCIDRList.split(",");
+CIDR[] cidr_list = NetUtils.convertToCIDR(str_cidr_list);
+if (!NetUtils.cidrListConsistency(cidr_list)) {
+throw new InvalidParameterValueException("The cidr list is 
not consistent " + Arrays.toString(cidr_list));
+}
+return cidr_list;
+} else {
+throw new InvalidParameterValueException("Zone super cidr is 
null check cloud.data_center_details for zone id=" + zoneId);
+}
+}
+
+private List getAllVpcCidrs() throws BadCIDRException {
+// get all the used cidrs
+List usedSubnets = new ArrayList();
+for (Vpc vpc : _vpcDao.listAll()) {
+if (_vpcSrvcDao.areServicesSupportedInVpc(vpc.getId(), 
Service.VPCDynamicRouting)) {
--- End diff --

For performance reasons, calling a query in a loop should be avoided.  Is 
it possible to refactor this ``for`` loop into a join?


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782496
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -747,6 +810,67 @@ public VpcOffering updateVpcOffering(final long 
vpcOffId, final String vpcOfferi
 }
 
 @Override
+public CIDR[] getZoneSuperCidrList(final long zoneId) throws 
BadCIDRException {
+final String superCIDRList = _dcDao.getDetail(zoneId, 
Params.SUPER_CIDR.name());
+if (superCIDRList != null) {
+String[] str_cidr_list = superCIDRList.split(",");
+CIDR[] cidr_list = NetUtils.convertToCIDR(str_cidr_list);
+if (!NetUtils.cidrListConsistency(cidr_list)) {
+throw new InvalidParameterValueException("The cidr list is 
not consistent " + Arrays.toString(cidr_list));
+}
--- End diff --

As noted in earlier comments, please consider a CIDR custom collection type 
and encapsulation of this logic in 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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782438
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -705,6 +744,30 @@ public boolean deleteVpcOffering(final long offId) {
 }
 
 @Override
+public Map updateQuaggaConfig(final Long zoneId, final 
String protocol, final String ospfArea, final Short helloInterval, final Short 
deadInterval,
+final Short retransmitInterval, final Short transitDelay, 
final String authentication, final String quaggaPassword, final String 
superCIDR, final Boolean enabled)
+throws BadCIDRException {
+OSPFZoneConfig qzc = new OSPFZoneConfig();
+DataCenterVO dc = _dcDao.findById(zoneId);
+_dcDao.loadDetails(dc);
+Map details = dc.getDetails();
+qzc.setDefaultValues(details);
+qzc.setValues(protocol, ospfArea, helloInterval, deadInterval, 
retransmitInterval, transitDelay, authentication, quaggaPassword, superCIDR, 
enabled);
+details = qzc.getValues();
+dc.setDetails(details);
+_dcDao.saveDetails(dc);
+s_logger.info("Quagga config saved to datacenter " + zoneId);
--- End diff --

Please include the contents of the config in this log message.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782392
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -705,6 +744,30 @@ public boolean deleteVpcOffering(final long offId) {
 }
 
 @Override
+public Map updateQuaggaConfig(final Long zoneId, final 
String protocol, final String ospfArea, final Short helloInterval, final Short 
deadInterval,
+final Short retransmitInterval, final Short transitDelay, 
final String authentication, final String quaggaPassword, final String 
superCIDR, final Boolean enabled)
+throws BadCIDRException {
+OSPFZoneConfig qzc = new OSPFZoneConfig();
+DataCenterVO dc = _dcDao.findById(zoneId);
+_dcDao.loadDetails(dc);
+Map details = dc.getDetails();
+qzc.setDefaultValues(details);
+qzc.setValues(protocol, ospfArea, helloInterval, deadInterval, 
retransmitInterval, transitDelay, authentication, quaggaPassword, superCIDR, 
enabled);
+details = qzc.getValues();
--- End diff --

Given the number of parameters on ``OSPFZoneConfig.setValues`` with the 
sequence of calls on lines 754-756, please consider extracting the creation of 
``OSPFZoneConfig`` instances to a builder class as a static inner class on 
``OSPFZoneConfig``.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782162
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -529,14 +545,14 @@ private boolean findCapabilityForService(final Map 
serviceCapabilitystList, fina
 if (serviceCapabilitystList != null && 
!serviceCapabilitystList.isEmpty()) {
 final Iterator iter = 
serviceCapabilitystList.values().iterator();
 while (iter.hasNext()) {
-final HashMap currentCapabilityMap = 
(HashMap) iter.next();
+final HashMap currentCapabilityMap = 
(HashMap)iter.next();
--- End diff --

Please type to the ``Map`` interface rather than ``HashMap`` to reduce 
coupling.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782148
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -501,7 +517,7 @@ private void 
validateConnectivtyServiceCapabilities(final Set provider
 final Iterator iter = serviceCapabilityCollection.iterator();
 
 while (iter.hasNext()) {
-final HashMap svcCapabilityMap = 
(HashMap) iter.next();
+final HashMap svcCapabilityMap = 
(HashMap)iter.next();
--- End diff --

Please type to the ``Map`` interface rather than ``HashMap`` to reduce 
coupling.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62782035
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -309,6 +314,19 @@ public void doInTransactionWithoutResult(final 
TransactionStatus status) {
 }
 
createVpcOffering(VpcOffering.redundantVPCOfferingName, 
VpcOffering.redundantVPCOfferingName, svcProviderMap, true, State.Enabled, 
null, false, false, true);
 }
+
+if 
(_vpcOffDao.findByUniqueName(VpcOffering.routedVPCOfferingName) == null) {
+s_logger.debug("Creating default routed VPC offering " 
+ VpcOffering.routedVPCOfferingName);
+
+final Map svcProviderMap = new 
HashMap();
+final Set defaultProviders = new 
HashSet();
+defaultProviders.add(Provider.VPCVirtualRouter);
--- End diff --

Consider replacing lines 321-323 with Guava's ``SetMultiMap``


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62781813
  
--- Diff: server/src/com/cloud/network/vpc/VpcManagerImpl.java ---
@@ -309,6 +314,19 @@ public void doInTransactionWithoutResult(final 
TransactionStatus status) {
 }
 
createVpcOffering(VpcOffering.redundantVPCOfferingName, 
VpcOffering.redundantVPCOfferingName, svcProviderMap, true, State.Enabled, 
null, false, false, true);
 }
+
+if 
(_vpcOffDao.findByUniqueName(VpcOffering.routedVPCOfferingName) == null) {
+s_logger.debug("Creating default routed VPC offering " 
+ VpcOffering.routedVPCOfferingName);
--- End diff --

Please wrap this log statement in a ``isDebugEnabled()`` check to avoid 
unnecessary string concatenation when ``DEBUG`` logging is not enabled.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62781709
  
--- Diff: 
server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java 
---
@@ -455,16 +471,26 @@ protected void finalizeNetworkRulesForNetwork(final 
Commands cmds, final DomainR
 
 super.finalizeNetworkRulesForNetwork(cmds, domainRouterVO, 
provider, guestNetworkId);
 
+final VpcVO vpc = _vpcDao.findById(domainRouterVO.getVpcId());
 if (domainRouterVO.getVpcId() != null) {
-
 if (domainRouterVO.getState() == State.Starting || 
domainRouterVO.getState() == State.Running) {
 if 
(_networkModel.isProviderSupportServiceInNetwork(guestNetworkId, 
Service.NetworkACL, Provider.VPCVirtualRouter)) {
 final List networkACLs = 
_networkACLMgr.listNetworkACLItems(guestNetworkId);
 if (networkACLs != null && !networkACLs.isEmpty()) {
-s_logger.debug("Found " + networkACLs.size() + " 
network ACLs to apply as a part of VPC VR " + domainRouterVO + " start for 
guest network id=" + guestNetworkId);
+s_logger.debug(
+"Found " + networkACLs.size() + " network 
ACLs to apply as a part of VPC VR " + domainRouterVO + " start for guest 
network id=" + guestNetworkId);
 
_commandSetupHelper.createNetworkACLsCommands(networkACLs, domainRouterVO, 
cmds, guestNetworkId, false);
 }
 }
+if 
(_vpcOffServiceDao.areServicesSupportedByNetworkOffering(vpc.getVpcOfferingId(),
 Service.VPCDynamicRouting)
+&& 
_networkModel.areServicesSupportedInNetwork(guestNetworkId, 
Service.VPCDynamicRouting)) {
+try {
+
_commandSetupHelper.createQuaggaConfigCommand(domainRouterVO, vpc.getId(), 
cmds);
+} catch (BadCIDRException ex) {
+s_logger.debug(ex);
+throw new CloudRuntimeException("The cidr for 
dynamic routing is bad " + ex);
--- End diff --

Please add the ``ex`` as the cause to provide a complete stack trace for 
debugging.


---
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: OSPF: adding dynamically routing capabili...

2016-05-10 Thread jburwell
Github user jburwell commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/1371#discussion_r62781651
  
--- Diff: 
server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java 
---
@@ -455,16 +471,26 @@ protected void finalizeNetworkRulesForNetwork(final 
Commands cmds, final DomainR
 
 super.finalizeNetworkRulesForNetwork(cmds, domainRouterVO, 
provider, guestNetworkId);
 
+final VpcVO vpc = _vpcDao.findById(domainRouterVO.getVpcId());
 if (domainRouterVO.getVpcId() != null) {
-
 if (domainRouterVO.getState() == State.Starting || 
domainRouterVO.getState() == State.Running) {
 if 
(_networkModel.isProviderSupportServiceInNetwork(guestNetworkId, 
Service.NetworkACL, Provider.VPCVirtualRouter)) {
 final List networkACLs = 
_networkACLMgr.listNetworkACLItems(guestNetworkId);
 if (networkACLs != null && !networkACLs.isEmpty()) {
-s_logger.debug("Found " + networkACLs.size() + " 
network ACLs to apply as a part of VPC VR " + domainRouterVO + " start for 
guest network id=" + guestNetworkId);
+s_logger.debug(
+"Found " + networkACLs.size() + " network 
ACLs to apply as a part of VPC VR " + domainRouterVO + " start for guest 
network id=" + guestNetworkId);
 
_commandSetupHelper.createNetworkACLsCommands(networkACLs, domainRouterVO, 
cmds, guestNetworkId, false);
 }
 }
+if 
(_vpcOffServiceDao.areServicesSupportedByNetworkOffering(vpc.getVpcOfferingId(),
 Service.VPCDynamicRouting)
+&& 
_networkModel.areServicesSupportedInNetwork(guestNetworkId, 
Service.VPCDynamicRouting)) {
+try {
+
_commandSetupHelper.createQuaggaConfigCommand(domainRouterVO, vpc.getId(), 
cmds);
+} catch (BadCIDRException ex) {
+s_logger.debug(ex);
--- End diff --

Why log to ``DEBUG`` instead of ``ERROR``?


---
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.
---


  1   2   >