You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/02/12 07:18:07 UTC
[GitHub] [cloudstack] Pearl1594 opened a new pull request #4503: Specify IP for VR in shared networks
Pearl1594 opened a new pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503
### Description
This PR enables admins to specify IP for a VR in a shared network
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736262317
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736028927
<b>Trillian test result (tid-3287)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34337 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4503-t3287-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 264.94 | test_kubernetes_clusters.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736713622
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531580527
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkCmdByAdmin.java
##########
@@ -42,6 +42,12 @@
@Parameter(name=ApiConstants.HIDE_IP_ADDRESS_USAGE, type=CommandType.BOOLEAN, description="when true ip address usage for the network will not be exported by the listUsageRecords API")
private Boolean hideIpAddressUsage;
+ @Parameter(name = ApiConstants.ROUTER_IP, type = CommandType.STRING, description = "IPV4 address to be assigned to a router in a shared network", since = "4.16")
+ private String routerIp;
+
+ @Parameter(name = ApiConstants.ROUTER_IPV6, type = CommandType.STRING, description = "IPV6 address to be assigned to a router in a shared network", since = "4.16")
+ private String routerIpv6;
Review comment:
> Wouldn't adding such a validator make the field/ parameter mandatory?
I think it is for validating the parameter value when it is specified in the cmd, nothing to do with required - true/false.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-737201068
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2457
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-759962910
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531530078
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -640,6 +640,19 @@ protected NetworkOrchestrator() {
setStateMachine();
}
+ private void updateNetworkDetails(NetworkVO networkPersisted, Network network) {
+ NetworkDetailVO networkDetailVO = null;
+ if (isNotBlank(network.getRouterIp())) {
+ networkDetailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ROUTER_IP, network.getRouterIp().toString(), true);
+ }
+ if (isNotBlank(network.getRouterIpv6())) {
+ networkDetailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ROUTER_IPV6, network.getRouterIpv6().toString(), true);
Review comment:
this will override the above _networkDetailVO_ , as per the code when both are not blank and always the latest one is persisted. I understand, at any given point, IPv4 or IPv6 is used, not both. From code perspective, either keep if-else for IPv4 & IPv6, or move the network details persist to respective if block.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531576198
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -640,6 +640,19 @@ protected NetworkOrchestrator() {
setStateMachine();
}
+ private void updateNetworkDetails(NetworkVO networkPersisted, Network network) {
+ NetworkDetailVO networkDetailVO = null;
+ if (isNotBlank(network.getRouterIp())) {
+ networkDetailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ROUTER_IP, network.getRouterIp().toString(), true);
+ }
+ if (isNotBlank(network.getRouterIpv6())) {
+ networkDetailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ROUTER_IPV6, network.getRouterIpv6().toString(), true);
Review comment:
But based on the code for persisting placeholder record, it seems like one can specify both ipv4 and ipv6 details:
https://github.com/shapeblue/cloudstack-kddi/blob/kddi-release/server/src/com/cloud/network/router/NetworkHelperImpl.java#L701
(and L718)
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkCmdByAdmin.java
##########
@@ -42,6 +42,12 @@
@Parameter(name=ApiConstants.HIDE_IP_ADDRESS_USAGE, type=CommandType.BOOLEAN, description="when true ip address usage for the network will not be exported by the listUsageRecords API")
private Boolean hideIpAddressUsage;
+ @Parameter(name = ApiConstants.ROUTER_IP, type = CommandType.STRING, description = "IPV4 address to be assigned to a router in a shared network", since = "4.16")
+ private String routerIp;
+
+ @Parameter(name = ApiConstants.ROUTER_IPV6, type = CommandType.STRING, description = "IPV6 address to be assigned to a router in a shared network", since = "4.16")
+ private String routerIpv6;
Review comment:
Wouldn't adding such a validator make the field/ parameter mandatory?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-735574065
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2443
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 removed a comment on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 removed a comment on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-779788411
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-779788581
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-779806319
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd merged pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-778025278
@Pearl1594 while this looks ready for merging, let's wait for Travis to pass; I saw some failures. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531528358
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -640,6 +640,19 @@ protected NetworkOrchestrator() {
setStateMachine();
}
+ private void updateNetworkDetails(NetworkVO networkPersisted, Network network) {
Review comment:
may be _updateRouterIpInNetworkDetails(networkId, routerIp, routerIpv6)_ , not required to pass entire network object.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] vladimirpetrov commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
vladimirpetrov commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-764525956
LGTM!
Tested:
- creating shared network with predefined VR IP (using UI and API), then deploying a VM using the same network
- deploying a VM on a shared network with predefined VR IP using the same IP as a VM's - expected error message is shown
- creating shared network with a network offering without services - the field for VR IP is hidden as expected
- restarting a shared network with predefined VR IP (with and without cleanup) does not affect the predefined IP address of the router
- attempting to create a non-shared network with predefined VR IP using the API fails with a proper error message
- creating a shared network without defining VR IP address is possible - the field is not mandatory
- VR IP field does not allow incorrect values
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-733549498
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] vladimirpetrov commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
vladimirpetrov commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-764525956
LGTM!
Tested:
- creating shared network with predefined VR IP (using UI and API), then deploying a VM using the same network
- deploying a VM on a shared network with predefined VR IP using the same IP as a VM's - expected error message is shown
- creating shared network with a network offering without services - the field for VR IP is hidden as expected
- restarting a shared network with predefined VR IP (with and without cleanup) does not affect the predefined IP address of the router
- attempting to create a non-shared network with predefined VR IP using the API fails with a proper error message
- creating a shared network without defining VR IP address is possible - the field is not mandatory
- VR IP field does not allow incorrect values
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-734668745
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2439
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-760815305
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-733598996
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2432
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-734669277
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-779805729
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-759963362
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd closed pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736288544
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-780189034
<b>Trillian test result (tid-3553)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33853 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4503-t3553-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_templates.py
Smoke tests completed. 86 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] shwstppr commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-760643738
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd closed pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-733578162
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-781146290
Tests LGTM both marvin tests and from @vladimirpetrov - I'll merge this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-781925036
> Yes @weizhouapache - I'm working on fixing it. Thanks!
@Pearl1594 good.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-734668919
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736262756
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736966772
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-733577595
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736732604
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2453
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-761200458
<b>Trillian test result (tid-3385)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36818 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4503-t3385-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 770.45 | test_kubernetes_clusters.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-781914536
@rhtyd @Pearl1594 @DaanHoogland
it looks recent travis test failed due to this pr.
see
https://travis-ci.com/github/apache/cloudstack/jobs/484387022
https://travis-ci.com/github/apache/cloudstack/jobs/484391002
error logs below
```
Currently running test: smoke/test_network
/home/travis/.local/lib/python2.7/site-packages/paramiko/transport.py:33: CryptographyDeprecationWarning: Python 2 is no longer supported by the Python core team. Support for it is now deprecated in cryptography, and will be removed in the next release.
from cryptography.hazmat.backends import default_backend
Port Forwarding Rule is deleted
NATRule is deleted
Router is deleted
Deleting Public IP : c582d08b-5207-4e9f-8281-8edce8c76d71
Port Forwarding Rule is deleted
Port Forwarding Rule is deleted
====Trying SSH Connection: Host:192.168.2.6 User:root Port:22 RetryCnt:2===
SshClient: Exception under createConnection: ['Traceback (most recent call last):\n', ' File "/home/travis/.local/lib/python2.7/site-packages/marvin/sshClient.py", line 122, in createConnection\n allow_agent=False)\n', ' File "/home/travis/.local/lib/python2.7/site-packages/paramiko/client.py", line 349, in connect\n retry_on_signal(lambda: sock.connect(addr))\n', ' File "/home/travis/.local/lib/python2.7/site-packages/paramiko/util.py", line 283, in retry_on_signal\n return function()\n', ' File "/home/travis/.local/lib/python2.7/site-packages/paramiko/client.py", line 349, in <lambda>\n retry_on_signal(lambda: sock.connect(addr))\n', ' File "/usr/lib/python2.7/socket.py", line 228, in meth\n return getattr(self._sock,name)(*args)\n', 'timeout: timed out\n']
Traceback (most recent call last):
File "/home/travis/.local/lib/python2.7/site-packages/marvin/sshClient.py", line 122, in createConnection
allow_agent=False)
File "/home/travis/.local/lib/python2.7/site-packages/paramiko/client.py", line 349, in connect
retry_on_signal(lambda: sock.connect(addr))
File "/home/travis/.local/lib/python2.7/site-packages/paramiko/util.py", line 283, in retry_on_signal
return function()
File "/home/travis/.local/lib/python2.7/site-packages/paramiko/client.py", line 349, in <lambda>
retry_on_signal(lambda: sock.connect(addr))
File "/usr/lib/python2.7/socket.py", line 228, in meth
return getattr(self._sock,name)(*args)
timeout: timed out
====Trying SSH Connection: Host:192.168.2.6 User:root Port:22 RetryCnt:1===
SshClient: Exception under createConnection: ['Traceback (most recent call last):\n', ' File "/home/travis/.local/lib/python2.7/site-packages/marvin/sshClient.py", line 122, in createConnection\n allow_agent=False)\n', ' File "/home/travis/.local/lib/python2.7/site-packages/paramiko/client.py", line 349, in connect\n retry_on_signal(lambda: sock.connect(addr))\n', ' File "/home/travis/.local/lib/python2.7/site-packages/paramiko/util.py", line 283, in retry_on_signal\n return function()\n', ' File "/home/travis/.local/lib/python2.7/site-packages/paramiko/client.py", line 349, in <lambda>\n retry_on_signal(lambda: sock.connect(addr))\n', ' File "/usr/lib/python2.7/socket.py", line 228, in meth\n return getattr(self._sock,name)(*args)\n', 'timeout: timed out\n']
Traceback (most recent call last):
File "/home/travis/.local/lib/python2.7/site-packages/marvin/sshClient.py", line 122, in createConnection
allow_agent=False)
File "/home/travis/.local/lib/python2.7/site-packages/paramiko/client.py", line 349, in connect
retry_on_signal(lambda: sock.connect(addr))
File "/home/travis/.local/lib/python2.7/site-packages/paramiko/util.py", line 283, in retry_on_signal
return function()
File "/home/travis/.local/lib/python2.7/site-packages/paramiko/client.py", line 349, in <lambda>
retry_on_signal(lambda: sock.connect(addr))
File "/usr/lib/python2.7/socket.py", line 228, in meth
return getattr(self._sock,name)(*args)
timeout: timed out
====Trying SSH Connection: Host:192.168.2.6 User:root Port:22 RetryCnt:0===
Executing command '{'endip': '172.16.179.21', 'acltype': 'domain', 'routerip': '172.16.179.15', 'name': 'MySharedNetwork - Test', 'networkofferingid': u'f160b481-ef28-4afd-8999-a94b180a74b3', 'physicalnetworkid': u'3a52e342-1cd1-4d5e-b2f4-d9a2efa253a6', 'startip': '172.16.179.2', 'vlan': 212, 'netmask': '255.255.255.0', 'displaytext': 'MySharedNetwork', 'scope': 'all', 'gateway': '172.16.179.1'}'
```
and
```
Printing marvin logs /tmp//MarvinLogs/test_network_AUK59S/failed_plus_exceptions.txt :
2021-02-18 23:08:13,330 - CRITICAL - EXCEPTION: test_01_deployVMInSharedNetwork: ['Traceback (most recent call last):\n', ' File "/usr/lib/python2.7/unittest/case.py", line 329, in run\n testMethod()\n', ' File "/home/travis/build/apache/cloudstack/test/integration/smoke/test_network.py", line 1988, in test_01_deployVMInSharedNetwork\n host = self.get_router_host(router)\n', ' File "/home/travis/build/apache/cloudstack/test/integration/smoke/test_network.py", line 1944, in get_router_host\n host.user, host.password = get_host_credentials(self.config, host.ipaddress)\n', ' File "/home/travis/.local/lib/python2.7/site-packages/marvin/lib/utils.py", line 231, in get_host_credentials\n raise KeyError("Please provide the marvin configuration file with credentials to your hosts")\n', "KeyError: 'Please provide the marvin configuration file with credentials to your hosts'\n"]
2021-02-18 23:08:18,539 - CRITICAL - EXCEPTION: test_02_verifyRouterIpAfterNetworkRestart: ['Traceback (most recent call last):\n', ' File "/usr/lib/python2.7/unittest/case.py", line 329, in run\n testMethod()\n', ' File "/home/travis/build/apache/cloudstack/test/integration/smoke/test_network.py", line 2016, in test_02_verifyRouterIpAfterNetworkRestart\n host = self.get_router_host(router)\n', ' File "/home/travis/build/apache/cloudstack/test/integration/smoke/test_network.py", line 1944, in get_router_host\n host.user, host.password = get_host_credentials(self.config, host.ipaddress)\n', ' File "/home/travis/.local/lib/python2.7/site-packages/marvin/lib/utils.py", line 231, in get_host_credentials\n raise KeyError("Please provide the marvin configuration file with credentials to your hosts")\n', "KeyError: 'Please provide the marvin configuration file with credentials to your hosts'\n"]
2021-02-18 23:08:19,602 - CRITICAL - FAILED: test_03_destroySharedNetwork: ['Traceback (most recent call last):\n', ' File "/usr/lib/python2.7/unittest/case.py", line 329, in run\n testMethod()\n', ' File "/home/travis/build/apache/cloudstack/test/integration/smoke/test_network.py", line 2025, in test_03_destroySharedNetwork\n self.fail("Failed to destroy the shared network")\n', ' File "/usr/lib/python2.7/unittest/case.py", line 410, in fail\n raise self.failureException(msg)\n', 'AssertionError: Failed to destroy the shared network\n']
2021-02-18 23:08:19,638 - CRITICAL - EXCEPTION: test_03_destroySharedNetwork: ['Traceback (most recent call last):\n', ' File "/home/travis/.local/lib/python2.7/site-packages/nose/suite.py", line 228, in run\n self.tearDown()\n', ' File "/home/travis/.local/lib/python2.7/site-packages/nose/suite.py", line 351, in tearDown\n self.teardownContext(ancestor)\n', ' File "/home/travis/.local/lib/python2.7/site-packages/nose/suite.py", line 367, in teardownContext\n try_run(context, names)\n', ' File "/home/travis/.local/lib/python2.7/site-packages/nose/util.py", line 471, in try_run\n return func()\n', ' File "/home/travis/build/apache/cloudstack/test/integration/smoke/test_network.py", line 1902, in tearDownClass\n raise Exception("Warning: Exception during cleanup : %s" % e)\n', "Exception: Warning: Exception during cleanup : Execute cmd: deletenetworkoffering failed, due to: errorCode: 431, errorText:Can't delete network offering 22 as its used by 1 networks. To m
ake the network offering unavaiable, disable it\n"]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531525770
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -718,10 +731,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
networks.add(networkPersisted);
if (network.getPvlanType() != null) {
- NetworkDetailVO detailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ISOLATED_PVLAN_TYPE, network.getPvlanType().toString(), true);
+ NetworkDetailVO detailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ISOLATED_PVLAN_TYPE, network.getPvlanType().toString(), true);
Review comment:
extra tab space, may be not required.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-734658515
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 closed pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 closed pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531576198
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -640,6 +640,19 @@ protected NetworkOrchestrator() {
setStateMachine();
}
+ private void updateNetworkDetails(NetworkVO networkPersisted, Network network) {
+ NetworkDetailVO networkDetailVO = null;
+ if (isNotBlank(network.getRouterIp())) {
+ networkDetailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ROUTER_IP, network.getRouterIp().toString(), true);
+ }
+ if (isNotBlank(network.getRouterIpv6())) {
+ networkDetailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ROUTER_IPV6, network.getRouterIpv6().toString(), true);
Review comment:
But based on the code for persisting placeholder record, it seems like one can specify both ipv4 and ipv6 details:
https://github.com/apache/cloudstack/blob/master/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java#L715
(and L732)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 removed a comment on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 removed a comment on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-733564565
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736967080
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r533091339
##########
File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
##########
@@ -1029,6 +1029,26 @@ private void checkSharedNetworkCidrOverlap(Long zoneId, long physicalNetworkId,
}
}
+ private void validateRouterIps(String routerIp, String routerIpv6, String startIp, String endIp, String startIpv6, String endIpv6) {
+ if (isNotBlank(routerIp)) {
+ if (!NetUtils.isValidIp4(routerIp)) {
+ throw new CloudRuntimeException("Router IPv4 IP provided is of incorrect format");
+ }
+ if (!NetUtils.isIpInRange(routerIp, startIp, endIp)) {
+ throw new CloudRuntimeException("Router IPv4 IP provided is not within the specified range: " + startIp + " - " + endIp);
+ }
+ }
+ if (isNotBlank(routerIpv6)) {
+ String ipv6Range = startIpv6 + "-" + endIpv6;
+ if (!NetUtils.isValidIp6(routerIpv6)) {
+ throw new CloudRuntimeException("Router IPv6 IP provided is of incorrect format");
+ }
+ if (!NetUtils.isIp6InRange(routerIpv6, ipv6Range)) {
+ throw new CloudRuntimeException("Router IPv4 IP provided is not within the specified range: " + startIpv6 + " - " + endIpv6);
Review comment:
_IPv4_ => _IPv6_ in the message string
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531612740
##########
File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
##########
@@ -1029,6 +1029,26 @@ private void checkSharedNetworkCidrOverlap(Long zoneId, long physicalNetworkId,
}
}
+ private void validateRouterIps(String routerIp, String routerIpv6, String startIp, String endIp, String startIpv6, String endIpv6) {
+ if (isNotBlank(routerIp)) {
+ if (!NetUtils.isValidIp4(routerIp)) {
+ throw new CloudRuntimeException("Router IPv4 IP provided is of incorrect format");
+ }
+ if (!NetUtils.isIpInRange(routerIp, startIp, endIp)) {
+ throw new CloudRuntimeException("Router IPv4 IP provided is not within the specified range: " + startIp + " - " + endIp);
+ }
+ }
+ if (isNotBlank(routerIpv6)) {
+ String ipv6Range = startIpv6 + "-" + endIpv6;
+ if (!NetUtils.isValidIp6(routerIpv6)) {
+ throw new CloudRuntimeException("Router IPv6 IP provided is of incorrect format");
+ }
+ if (!NetUtils.isIp6InRange(routerIp, ipv6Range)) {
Review comment:
__routerIp_ => routerIpv6_
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-737184147
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-735700301
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-779824363
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2715
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-735552819
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736287656
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2450
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-733564565
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-779844445
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-779844745
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-760644156
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-781915366
Yes @weizhouapache - I'm working on fixing it. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531530078
##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -640,6 +640,19 @@ protected NetworkOrchestrator() {
setStateMachine();
}
+ private void updateNetworkDetails(NetworkVO networkPersisted, Network network) {
+ NetworkDetailVO networkDetailVO = null;
+ if (isNotBlank(network.getRouterIp())) {
+ networkDetailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ROUTER_IP, network.getRouterIp().toString(), true);
+ }
+ if (isNotBlank(network.getRouterIpv6())) {
+ networkDetailVO = new NetworkDetailVO(networkPersisted.getId(), ApiConstants.ROUTER_IPV6, network.getRouterIpv6().toString(), true);
Review comment:
this will override the above _networkDetailVO_ , as per the code when both are not blank and always the latest one is persisted. Either keep if-else for IPv4 & IPv6, or move the network details persist to respective if block.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531613119
##########
File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
##########
@@ -1029,6 +1029,26 @@ private void checkSharedNetworkCidrOverlap(Long zoneId, long physicalNetworkId,
}
}
+ private void validateRouterIps(String routerIp, String routerIpv6, String startIp, String endIp, String startIpv6, String endIpv6) {
+ if (isNotBlank(routerIp)) {
+ if (!NetUtils.isValidIp4(routerIp)) {
+ throw new CloudRuntimeException("Router IPv4 IP provided is of incorrect format");
+ }
+ if (!NetUtils.isIpInRange(routerIp, startIp, endIp)) {
+ throw new CloudRuntimeException("Router IPv4 IP provided is not within the specified range: " + startIp + " - " + endIp);
+ }
+ }
+ if (isNotBlank(routerIpv6)) {
+ String ipv6Range = startIpv6 + "-" + endIpv6;
+ if (!NetUtils.isValidIp6(routerIpv6)) {
+ throw new CloudRuntimeException("Router IPv6 IP provided is of incorrect format");
+ }
+ if (!NetUtils.isIp6InRange(routerIp, ipv6Range)) {
+ throw new CloudRuntimeException("Router IPv4 IP provided is not within the specified range: " + startIp + " - " + endIp);
Review comment:
_IPv4_ => _IPv6_ and same for start/end ip
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-760017747
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531576582
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -3471,7 +3471,7 @@ private NetworkVO createDefaultNetworkForAccount(DataCenter zone, Account owner,
s_logger.debug("Creating network for account " + owner + " from the network offering id=" + requiredOfferings.get(0).getId() + " as a part of deployVM process");
Network newNetwork = _networkMgr.createGuestNetwork(requiredOfferings.get(0).getId(), owner.getAccountName() + "-network", owner.getAccountName() + "-network",
null, null, null, false, null, owner, null, physicalNetwork, zone.getId(), ACLType.Account, null, null, null, null, true, null, null,
- null);
+ null, null, null);
Review comment:
I see most of the calls, routerips are passed _null_ , can you add another public method with routerip params and use it wherever required ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531580527
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkCmdByAdmin.java
##########
@@ -42,6 +42,12 @@
@Parameter(name=ApiConstants.HIDE_IP_ADDRESS_USAGE, type=CommandType.BOOLEAN, description="when true ip address usage for the network will not be exported by the listUsageRecords API")
private Boolean hideIpAddressUsage;
+ @Parameter(name = ApiConstants.ROUTER_IP, type = CommandType.STRING, description = "IPV4 address to be assigned to a router in a shared network", since = "4.16")
+ private String routerIp;
+
+ @Parameter(name = ApiConstants.ROUTER_IPV6, type = CommandType.STRING, description = "IPV6 address to be assigned to a router in a shared network", since = "4.16")
+ private String routerIpv6;
Review comment:
> Wouldn't adding such a validator make the field/ parameter mandatory?
I think it is for validating the parameter value, nothing to do with required - true/false.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-733549798
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-735700935
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736289061
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-734917118
<b>Trillian test result (tid-3272)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34888 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4503-t3272-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Smoke tests completed. 84 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 214.96 | test_kubernetes_clusters.py
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 80.30 | test_routers_network_ops.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736683133
<b>Trillian test result (tid-3293)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31046 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4503-t3293-kvm-centos7.zip
Smoke tests completed. 86 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-736714040
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-733556229
Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2431
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-734658797
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-737183522
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-759982175
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2540
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-760816493
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-737299745
<b>Trillian test result (tid-3298)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40888 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4503-t3298-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_human_readable_logs.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 84 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_enableHumanReadableLogs | `Error` | 0.32 | test_human_readable_logs.py
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 770.90 | test_kubernetes_clusters.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#discussion_r531524009
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkCmdByAdmin.java
##########
@@ -42,6 +42,12 @@
@Parameter(name=ApiConstants.HIDE_IP_ADDRESS_USAGE, type=CommandType.BOOLEAN, description="when true ip address usage for the network will not be exported by the listUsageRecords API")
private Boolean hideIpAddressUsage;
+ @Parameter(name = ApiConstants.ROUTER_IP, type = CommandType.STRING, description = "IPV4 address to be assigned to a router in a shared network", since = "4.16")
+ private String routerIp;
+
+ @Parameter(name = ApiConstants.ROUTER_IPV6, type = CommandType.STRING, description = "IPV6 address to be assigned to a router in a shared network", since = "4.16")
+ private String routerIpv6;
Review comment:
can add validator "_ApiArgValidator.NotNullOrEmpty_" for both args ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-735552329
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-760017070
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Pearl1594 commented on pull request #4503: Specify IP for VR in shared networks
Posted by GitBox <gi...@apache.org>.
Pearl1594 commented on pull request #4503:
URL: https://github.com/apache/cloudstack/pull/4503#issuecomment-779788411
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org