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 2020/06/24 21:08:54 UTC
[GitHub] [cloudstack] GabrielBrascher opened a new pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
GabrielBrascher opened a new pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177
## Description
<!--- Describe your changes in detail -->
If an IPv6 enabled network is created but the Zone hasn't IPv6 DNS1 or DNS2 configured then **dnsmasq** inside the Virtual Router does not start.
This PRs adds validation that allows creating an IPv6 network only if the Zone has at least DNS1 or DNS2 configured.
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
Fixes: #4157
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
----------------------------------------------------------------
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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-717716328
@Pearl1594 you are right. It did not affect this implementation due to all networks that I tested already contained start/end ipv4 address (this is something to change in the future, but so far CloudStack can only deploy networks of IPv4 or IPv4 + IPv6; in the future, it could be nice to allow networks with IPv6 nly.
I added a small commit addressing it, thanks for the heads up.
----------------------------------------------------------------
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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718818717
@GabrielBrascher you can go for 4.14 as well (or just master)
----------------------------------------------------------------
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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-714369177
@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] GabrielBrascher commented on a change in pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#discussion_r446148602
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -3781,7 +3781,7 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
ipv4 = true;
}
- if (startIPv6 != null) {
+ if (vlanIp6Cidr != null) {
Review comment:
CloudStack uses SLAAC for managing IPv6 ranges, therefore it is not necessary to have a start/end IPv6 address; on the other hand, IPv6 CIDR is mandatory for IPv6 networks.
That is why I changed this and other pieces of code that had start/end ipv6 address as 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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-700290585
@GabrielBrascher 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] GabrielBrascher edited a comment on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher edited a comment on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-717716328
@Pearl1594 you are right. It did not affect this implementation due to all networks that I tested already contained start/end ipv4 address (this is something to change in the future, but so far CloudStack can only deploy networks of IPv4 or IPv4 + IPv6; in the future, it could be nice to allow networks with IPv6 only.
I added a small commit addressing it, thanks for the heads up.
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718143751
Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2303
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-698208235
@GabrielBrascher 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] GabrielBrascher commented on a change in pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#discussion_r513523504
##########
File path: server/src/main/java/com/cloud/network/NetworkModelImpl.java
##########
@@ -2207,14 +2207,8 @@ public boolean isNetworkInlineMode(Network network) {
@Override
public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gateway, String ip6Cidr) throws InvalidParameterValueException {
- if (!NetUtils.isValidIp6(startIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the startIPv6 parameter");
- }
- if (!NetUtils.isValidIp6(endIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the endIPv6 parameter");
- }
- if (!(ip6Gateway != null && ip6Cidr != null)) {
+ if (StringUtils.isBlank(ip6Gateway) || StringUtils.isBlank(ip6Cidr)) {
throw new InvalidParameterValueException("ip6Gateway and ip6Cidr should be defined when startIPv6/endIPv6 are passed in");
Review comment:
Just pushed a commit addressing your comment. Thanks Suresh.
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-712780923
@DaanHoogland 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] DaanHoogland commented on a change in pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#discussion_r494800869
##########
File path: server/src/main/java/com/cloud/network/NetworkModelImpl.java
##########
@@ -2207,14 +2207,8 @@ public boolean isNetworkInlineMode(Network network) {
@Override
public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gateway, String ip6Cidr) throws InvalidParameterValueException {
- if (!NetUtils.isValidIp6(startIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the startIPv6 parameter");
- }
- if (!NetUtils.isValidIp6(endIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the endIPv6 parameter");
- }
- if (!(ip6Gateway != null && ip6Cidr != null)) {
+ if (org.apache.commons.lang3.StringUtils.isBlank(ip6Gateway) || org.apache.commons.lang3.StringUtils.isBlank(ip6Cidr)) {
Review comment:
can you encapsulate this in our local proxy to the StringUtils, so migration efforts will be located in a single 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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718115582
@GabrielBrascher the issue this PR solves is marked for 4.13.2. do you want to have this merged on master anyway?
----------------------------------------------------------------
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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-717846290
@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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-714369995
@DaanHoogland 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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-698207517
----------------------------------------------------------------
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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-712780331
@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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-713838791
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2256
----------------------------------------------------------------
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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718158740
not sure if the ape still knows 4.13 but i think it should
@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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718001032
@DaanHoogland @rhtyd @Pearl1594 @sureshanaparti looking at the codebase for IPv6 there are still a lot of things to improve.
I will leave some of these parts for future PRs as they require testing and so far I made manual tests specifically for the reported issue https://github.com/apache/cloudstack/issues/4157. This PR is partially improving the IPv6, mostly avoiding some critical deployment failures due to a lack of ipv6 gateway/DNS.
Unfortunately, most of our execution flow regarding IPv6 needs to be refactored, this current PR adds a small contribution on behalf of issue #3569, but mainly focused on fixing https://github.com/apache/cloudstack/issues/4157.
I can re-run a couple of manual tests just for the sake. For the (near) future, I think that we should have some IPv6 Marvin tests. I can look into contributing with it.
----------------------------------------------------------------
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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-717846823
@Pearl1594 are you satisfied with 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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-698207801
@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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718134186
@DaanHoogland sounds good. Rebased/cherry-picked respective commits aiming branch 4.13.
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-717828938
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2290
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-713103476
Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2237
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-698208235
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718139318
@GabrielBrascher 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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718138761
@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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718907679
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2311
----------------------------------------------------------------
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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-717846642
without regression I think this can go in. It's @GabrielBrascher 's neck ;)
----------------------------------------------------------------
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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-717795431
@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] GabrielBrascher commented on a change in pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#discussion_r495618184
##########
File path: server/src/main/java/com/cloud/network/NetworkModelImpl.java
##########
@@ -2207,14 +2207,8 @@ public boolean isNetworkInlineMode(Network network) {
@Override
public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gateway, String ip6Cidr) throws InvalidParameterValueException {
- if (!NetUtils.isValidIp6(startIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the startIPv6 parameter");
- }
- if (!NetUtils.isValidIp6(endIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the endIPv6 parameter");
- }
- if (!(ip6Gateway != null && ip6Cidr != null)) {
+ if (org.apache.commons.lang3.StringUtils.isBlank(ip6Gateway) || org.apache.commons.lang3.StringUtils.isBlank(ip6Cidr)) {
Review comment:
Fixed @DaanHoogland, code is now using the `com.cloud.utils.StringUtils.isBlank`. 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] DaanHoogland merged pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-712825988
Packaging result: ✖centos7 ✖centos8 ✔debian. JID-2221
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-713052414
@DaanHoogland 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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-713821986
@DaanHoogland It would be great to have this on 4.15. Despite it being a simple code, this PR can prevent some nasty issues when deploying new Zones with IPv6.
Ping for review: @svenvogel @rhtyd @andrijapanicsb @abhinandanprateek @ravening @sureshanaparti @slavkap @kiwiflyer @weizhouapache @RodrigoDLopez @Pearl1594 and **others**.
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-717795763
@DaanHoogland 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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718747043
@DaanHoogland do you think that it would be better to aim this to 4.15 so we can run 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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-713822818
@GabrielBrascher 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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-700290057
@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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-714722531
<b>Trillian test result (tid-3044)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34503 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4177-t3044-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 85 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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718881143
@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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-719166200
<b>Trillian test result (tid-3107)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34725 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4177-t3107-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.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` | 249.52 | 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] blueorangutan commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-698225527
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2081
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718881976
@GabrielBrascher 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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718159284
I am not sure as well. Travis seems to not work. Maybe due to the migration of java 8 to java 11
----------------------------------------------------------------
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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-698207517
Fixed conflict with the master.
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718937559
@DaanHoogland 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] GabrielBrascher commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-713822447
@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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-713356568
@GabrielBrascher do you still want this before 4.15?
----------------------------------------------------------------
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] DaanHoogland commented on a change in pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#discussion_r494800869
##########
File path: server/src/main/java/com/cloud/network/NetworkModelImpl.java
##########
@@ -2207,14 +2207,8 @@ public boolean isNetworkInlineMode(Network network) {
@Override
public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gateway, String ip6Cidr) throws InvalidParameterValueException {
- if (!NetUtils.isValidIp6(startIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the startIPv6 parameter");
- }
- if (!NetUtils.isValidIp6(endIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the endIPv6 parameter");
- }
- if (!(ip6Gateway != null && ip6Cidr != null)) {
+ if (org.apache.commons.lang3.StringUtils.isBlank(ip6Gateway) || org.apache.commons.lang3.StringUtils.isBlank(ip6Cidr)) {
Review comment:
can you encapsulate this in our local proxy to the StringUtils, so migration efforts will be located in a single 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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-700306823
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2094
----------------------------------------------------------------
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 #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#discussion_r513351980
##########
File path: server/src/main/java/com/cloud/network/NetworkModelImpl.java
##########
@@ -2207,14 +2207,8 @@ public boolean isNetworkInlineMode(Network network) {
@Override
public void checkIp6Parameters(String startIPv6, String endIPv6, String ip6Gateway, String ip6Cidr) throws InvalidParameterValueException {
- if (!NetUtils.isValidIp6(startIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the startIPv6 parameter");
- }
- if (!NetUtils.isValidIp6(endIPv6)) {
- throw new InvalidParameterValueException("Invalid format for the endIPv6 parameter");
- }
- if (!(ip6Gateway != null && ip6Cidr != null)) {
+ if (StringUtils.isBlank(ip6Gateway) || StringUtils.isBlank(ip6Cidr)) {
throw new InvalidParameterValueException("ip6Gateway and ip6Cidr should be defined when startIPv6/endIPv6 are passed in");
Review comment:
@GabrielBrascher may be this exception msg has to be updated, as it is not sure here that startIPv6/endIPv6 are passed or not, as the checks for startIPv6/endIPv6 are moved after 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] GabrielBrascher commented on a change in pull request #4177: [WIP] Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#discussion_r446150677
##########
File path: server/src/test/java/com/cloud/network/NetworkModelTest.java
##########
@@ -90,6 +91,11 @@
private static final long PHYSICAL_NETWORK_1_ID = 1L;
private static final long PHYSICAL_NETWORK_2_ID = 2L;
+ private static final String IPV6_CIDR = "fd59:16ba:559b:243d::/64";
Review comment:
Nowadays IPv6 networking support only `/64` CIDR. Added Unit tests covering this.
##########
File path: server/src/main/java/com/cloud/network/NetworkServiceImpl.java
##########
@@ -1195,7 +1195,7 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac
if (startIP != null) {
ipv4 = true;
}
- if (startIPv6 != null) {
+ if (isNotBlank(ip6Cidr) && isNotBlank(ip6Gateway)) {
Review comment:
Due to SLAAC implementation, an IPv6 network needs CIDR and Gateway. With the refactored code a network is "marked" as ipv6 if IPv6 CIDR and Gateway are not null.
##########
File path: server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -3781,7 +3781,7 @@ public Vlan createVlanAndPublicIpRange(final long zoneId, final long networkId,
ipv4 = true;
}
- if (startIPv6 != null) {
+ if (vlanIp6Cidr != null) {
Review comment:
CloudStack uses SLAAC for managing IPv6 ranges, therefore it is not necessary to have an start/end IPv6 address; on the other hand, IPv6 CIDR is mandatory for IPv6 networks.
That is why I changed this and other pieces of code that had start/end ipv6 address as 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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-713051974
@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] DaanHoogland commented on pull request #4177: Prevent deploying IPv6 network if Zone has no IPv6 DNS configured
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4177:
URL: https://github.com/apache/cloudstack/pull/4177#issuecomment-718936685
@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