You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by SudharmaJain <gi...@git.apache.org> on 2015/12/16 07:31:11 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
GitHub user SudharmaJain opened a pull request:
https://github.com/apache/cloudstack/pull/1246
CLOUDSTACK-9165 unable to use reserved IP range in a network for exte…
…rnal VMs
Repro Steps
------------------
1. Create an isolated network with CIDR 192.168.200.0/24.
2. Edit the CIDR to 192.168.200.0/25.
3. Perform Network restart with cleanup.
4. Network Restart with Cleanup, it was successful.
4. Add external VM to the network with IP 192.168.200.150/24 to the network.
5. After restart Netmask1 displayed CIDR 192.168.200.150/25 and Network CIDR as 192.168.200.150/24.
6. Create an instance from cloudstack.
7. Ping the VM from external VM.
Before Applying the fix
-------------------------------
External Vm was unable to communicate with the cloudstack managed VM.
After applying the fix
--------------------------
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/SudharmaJain/cloudstack cs-9165
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1246.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1246
----
commit 1bd12fce44d79a2825c4c78fbfe1183e04ce496d
Author: SudharmaJain <su...@citrix.com>
Date: 2015-12-16T04:42:00Z
CLOUDSTACK-9165 unable to use reserved IP range in a network for external VMs
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
Posted by bvbharatk <gi...@git.apache.org>.
Github user bvbharatk commented on the pull request:
https://github.com/apache/cloudstack/pull/1246#issuecomment-222328207
### ACS CI BVT Run
**Sumarry:**
Build Number 75
Hypervisor xenserver
NetworkType Advanced
Passed=68
Failed=5
Skipped=3
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_vpc_vpn.py
* ContextSuite context=TestRVPCSite2SiteVpn>:setup Failing since 3 runs
* ContextSuite context=TestVpcRemoteAccessVpn>:setup Failing since 4 runs
* ContextSuite context=TestVpcSite2SiteVpn>:setup Failing since 4 runs
* test_volumes.py
* test_06_download_detached_volume Failed
* test_vm_life_cycle.py
* test_10_attachAndDetach_iso Failed
**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_routers.py
test_reset_vm_on_reboot.py
test_snapshots.py
test_deploy_vms_with_varied_deploymentplanners.py
test_login.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
Posted by nlivens <gi...@git.apache.org>.
Github user nlivens commented on the pull request:
https://github.com/apache/cloudstack/pull/1246#issuecomment-217875751
@SudharmaJain, I've added a few comments
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1246: CLOUDSTACK-9165 unable to use reserved IP range in a...
Posted by cloudmonger <gi...@git.apache.org>.
Github user cloudmonger commented on the issue:
https://github.com/apache/cloudstack/pull/1246
### ACS CI BVT Run
**Sumarry:**
Build Number 419
Hypervisor xenserver
NetworkType Advanced
Passed=103
Failed=2
Skipped=7
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_volumes.py
* test_06_download_detached_volume Failed
* test_routers_network_ops.py
* test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false Failed
**Skipped tests:**
test_01_test_vm_volume_snapshot
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_11_ss_nfs_version_on_ssvm
test_nested_virtualization_vmware
test_3d_gpu_support
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_loadbalance.py
test_routers.py
test_reset_vm_on_reboot.py
test_deploy_vms_with_varied_deploymentplanners.py
test_network.py
test_router_dns.py
test_non_contigiousvlan.py
test_login.py
test_deploy_vm_iso.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_vm_life_cycle.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
Posted by nlivens <gi...@git.apache.org>.
Github user nlivens commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1246#discussion_r62503776
--- Diff: server/src/com/cloud/network/guru/GuestNetworkGuru.java ---
@@ -396,6 +396,16 @@ public NicProfile allocate(final Network network, NicProfile nic, final VirtualM
return nic;
}
+ /**
+ * Return a string representing network Cidr for the specifeid network
+ * @param guestNetwork
+ * @return valid network Cidr for the specified network
+ */
+ protected String getValidNetworkCidr(Network guestNetwork){
--- End diff --
I think you should extract this method to another common class (e.g. NetworkModel) since this method can be reused over the different classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/1246#issuecomment-216208690
@SudharmaJain can you rebase against latest master and share state of your PR, thanks
tag:easypr
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
Posted by alexandrelimassantana <gi...@git.apache.org>.
Github user alexandrelimassantana commented on the pull request:
https://github.com/apache/cloudstack/pull/1246#issuecomment-174398371
Hello, there is also something bugging me with this piece of code which appears on the changed files:
```java
final String gatewayCidr = guestNetwork.getNetworkCidr() == null ? guestNetwork.getCidr() : guestNetwork.getNetworkCidr();
```
First, it is repeated code, which could turn into a function in the superclass, since VirtualNetworkApplianceManagerImpl and NetworkHelpImpl are implementation classes.
Also, you are calling the function getNetworkCidr twice in the event of it's return value not being null. It's both confusing and not efficient. You could try writing a separate method for the purpose of correctly setting this variable, such as:
```java
protected String getValidGatewayCidr(){
String gatewayCidr = guestNetwork.getNetworkCidr();
return gatewayCidr == null ? guestNetwork.getCidr() : null;
}
```
These changes would make the code more understandable, provided that you could write javadocs on these pieces of code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1246: CLOUDSTACK-9165 unable to use reserved IP range in a...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the issue:
https://github.com/apache/cloudstack/pull/1246
@nlivens As suggested I have moved the getValidNetworkCidr to common class NetworkModel.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1246: CLOUDSTACK-9165 unable to use reserved IP range in a...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the issue:
https://github.com/apache/cloudstack/pull/1246
tag:mergeready
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1246: CLOUDSTACK-9165 unable to use reserved IP range in a...
Posted by PranaliM <gi...@git.apache.org>.
Github user PranaliM commented on the issue:
https://github.com/apache/cloudstack/pull/1246
Test LGTM based on manual testing results.
Before fix The VM got a Netmask of 255.255.255.128 and after fix the VM got a Netmask of 255.255.255.0
on a network where the CIDR was /25
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1246: CLOUDSTACK-9165 unable to use reserved IP range in a...
Posted by sudhansu7 <gi...@git.apache.org>.
Github user sudhansu7 commented on the issue:
https://github.com/apache/cloudstack/pull/1246
LGTM for code change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
Posted by nlivens <gi...@git.apache.org>.
Github user nlivens commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1246#discussion_r62503830
--- Diff: server/src/com/cloud/network/router/NetworkHelperImpl.java ---
@@ -755,6 +755,16 @@ protected HypervisorType getClusterToStartDomainRouterForOvm(final long podId) {
return networks;
}
+ /**
+ * Return a string representing network Cidr for the specifeid network
+ * @param guestNetwork
+ * @return valid network Cidr for the specified network
+ */
+ protected String getValidNetworkCidr(Network guestNetwork){
--- End diff --
I think you should extract this method to another common class (e.g. NetworkModel) since this method can be reused over the different classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the pull request:
https://github.com/apache/cloudstack/pull/1246#issuecomment-212258625
@alexandrelimassantana I have made changes with respect to ur last comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1246: CLOUDSTACK-9165 unable to use reserved IP range in a...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the issue:
https://github.com/apache/cloudstack/pull/1246
Moved the getValidNetworkCidr to common class NetworkModel and Rebased with master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
Posted by nlivens <gi...@git.apache.org>.
Github user nlivens commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1246#discussion_r62503852
--- Diff: server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java ---
@@ -1588,6 +1588,16 @@ protected StringBuilder createGuestBootLoadArgs(final NicProfile guestNic, final
return buf;
}
+ /**
+ * Return a string representing network Cidr for the specifeid network
+ * @param guestNetwork
+ * @return valid network Cidr for the specified network
+ */
+ protected String getValidNetworkCidr(Network guestNetwork){
--- End diff --
I think you should extract this method to another common class (e.g. NetworkModel) since this method can be reused over the different classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1246: CLOUDSTACK-9165 unable to use reserved IP range in a...
Posted by cloudmonger <gi...@git.apache.org>.
Github user cloudmonger commented on the issue:
https://github.com/apache/cloudstack/pull/1246
### ACS CI BVT Run
**Sumarry:**
Build Number 328
Hypervisor xenserver
NetworkType Advanced
Passed=103
Failed=1
Skipped=7
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_routers_network_ops.py
* test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false Failed
**Skipped tests:**
test_01_test_vm_volume_snapshot
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_11_ss_nfs_version_on_ssvm
test_nested_virtualization_vmware
test_3d_gpu_support
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_loadbalance.py
test_routers.py
test_reset_vm_on_reboot.py
test_deploy_vms_with_varied_deploymentplanners.py
test_network.py
test_router_dns.py
test_non_contigiousvlan.py
test_login.py
test_deploy_vm_iso.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_volumes.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_vm_life_cycle.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9165 unable to use reserved IP...
Posted by pedro-martins <gi...@git.apache.org>.
Github user pedro-martins commented on the pull request:
https://github.com/apache/cloudstack/pull/1246#issuecomment-172029080
Hi =)
What is the difference between network.getCidr() and network.getNetworkCidr() ?
And, there is a possibility to you create a method with a simple javadoc and simple testcase instead of using a short if?
Ty =)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---