[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[jayapalu]

Github user jayapalu commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
@ProjectMoon  Jenkins and CI is passed, Can you please give LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[jayapalu]

Github user jayapalu commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
Jenkins got timed out. I am force pushing again to trigger jenkins.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[ProjectMoon]

Github user ProjectMoon commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
Looks good now. Not sure what's up with Jenkins though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[cloudmonger]

Github user cloudmonger commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
### ACS CI BVT Run
 **Sumarry:**
 Build Number 3
 Hypervisor xenserver
 NetworkType Advanced
 Passed=623
 Failed=290
 Skipped=58

_Link to logs Folder (search by build_no):_ 
https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0


**Failed tests:**
* test_dhcp_dns_offload.py

 * ContextSuite context=TestDeployVMs>:setup Failed

* test_affinity_groups_projects.py

 * test_01_deploy_vm_anti_affinity_group Failed

 * test_02_deploy_vm_anti_affinity_group_fail_on_not_enough_hosts Failed

 * test_01_list_aff_grps_for_vm Failed

 * test_02_list_multiple_aff_grps_for_vm Failed

 * test_01_update_aff_grp_by_ids Failed

* test_ps_domain_limits.py

 * test_04_create_template_snapshot Failed

* test_ss_domain_limits.py

 * test_04_create_template_delete_account Failed

 * test_04_create_template_delete_account Failing since 2 runs

 * test_01_multiple_domains_secondary_storage_limits Failed

 * test_01_multiple_domains_secondary_storage_limits Failing since 2 runs

 * test_02_multiple_domains_secondary_storage_counts Failed

* test_deploy_vm_userdata_multi_nic.py

 * test_deployvm_multinic Failed

* test_vpn_service.py

 * test_01_VPN_service Failed

* test_organization_states.py

 * ContextSuite context=TestOrganizationStates>:setup Failed

* test_escalations_snapshots.py

 * ContextSuite context=TestSnapshots>:setup Failed

* test_dynamic_compute_offering.py

 * test_max_account_memory_scale_VM_1_ADMIN_ACCOUNT Failed

 * test_max_account_memory_scale_VM_2_USER_ACCOUNT Failed

 * test_deploy_VM_with_affinity_group_1_ADMIN_ACCOUNT Failed

 * test_deploy_VM_with_affinity_group_2_USER_ACCOUNT Failed

 * ContextSuite context=TestDynamicServiceOffering>:setup Failed

 * test_change_so_running_vm_dynamic_to_dynamic_1_ADMIN_ACCOUNT Failed

 * test_change_so_running_vm_dynamic_to_dynamic_2_USER_ACCOUNT Failed

 * test_change_so_running_vm_dynamic_to_static_1_ADMIN_ACCOUNT Failed

 * test_change_so_running_vm_dynamic_to_static_2_USER_ACCOUNT Failed

 * test_change_so_running_vm_static_to_dynamic_1_ADMIN_ACCOUNT Failed

 * test_change_so_running_vm_static_to_dynamic_2_USER_ACCOUNT Failed

 * test_change_so_running_vm_static_to_static_1_ADMIN_ACCOUNT Failed

 * test_change_so_running_vm_static_to_static_2_USER_ACCOUNT Failed

 * test_change_so_stopped_vm_dynamic_to_dynamic_1_ADMIN_ACCOUNT Failed

 * test_change_so_stopped_vm_dynamic_to_dynamic_2_USER_ACCOUNT Failed

 * test_change_so_stopped_vm_dynamic_to_static_1_ADMIN_ACCOUNT Failed

 * test_change_so_stopped_vm_dynamic_to_static_2_USER_ACCOUNT Failed

 * test_change_so_stopped_vm_static_to_dynamic_1_ADMIN_ACCOUNT Failed

 * test_change_so_stopped_vm_static_to_dynamic_2_USER_ACCOUNT Failed

 * test_change_so_stopped_vm_static_to_static_1_ADMIN_ACCOUNT Failed

* test_ps_resource_limits_volume.py

 * test_attach_volume_exceeding_primary_limits Failed

* test_vpc.py

 * test_07_restart_network_vm_running Failed

 * test_08_delete_vpc Failed

 * test_11_deploy_vm_wo_network_netdomain Failed

 * test_13_deploy_vm_with_vpc_netdomain Failed

 * test_14_deploy_vm_1 Failed

 * test_15_deploy_vm_2 Failed

 * test_16_deploy_vm_for_user_by_admin Failed

 * test_21_deploy_vm_with_gateway_ip Failed

 * test_22_vpn_customer_gw_with_hostname Failed

* test_non_contiguous_vlan.py

 * test_01_add_non_contiguous_ranges Failed

* test_project_usage.py

 * ContextSuite context=TestLBRuleUsage>:setup Failed

 * ContextSuite context=TestNatRuleUsage>:setup Failed

 * ContextSuite context=TestPublicIPUsage>:setup Failed

 * ContextSuite context=TestSnapshotUsage>:setup Failed

 * ContextSuite context=TestVmUsage>:setup Failed

 * ContextSuite context=TestVolumeUsage>:setup Failed

 * ContextSuite context=TestVpnUsage>:setup Failed

* test_ps_max_limits.py

 * test_01_deploy_vm_domain_limit_reached Failed

 * test_02_deploy_vm_account_limit_reached Failed

 * test_03_deploy_vm_project_limit_reached Failed

* test_project_resources.py

 * test_07_associate_public_ip Failed

* test_vpc_network_staticnatrule.py

 * test_01_VPC_StaticNatRuleCreateStoppedState Failed

 * test_03_VPC_StopCreateMultipleStaticNatRuleStopppedState Failed

 * test_04_VPC_CreateMultipleStaticNatRule Failed

 * 

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[rhtyd]

Github user rhtyd commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
@jayapalu can you change the PR base branch to 4.9?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[jayapalu]

Github user jayapalu commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
@ProjectMoon  Can you please review it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[ProjectMoon]

Github user ProjectMoon commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
Hi, that's great news.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[jayapalu]

Github user jayapalu commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
@ProjectMoon 
I have today this PR after rebasing latest master. I did not observe the 
add:true for the removed ip.
Here is output from the ips.json and ip addr show output.

Here is the output:

4: eth2:  mtu 1500 qdisc pfifo_fast state 
UP qlen 1000
link/ether 06:f6:cc:00:00:0e brd ff:ff:ff:ff:ff:ff
inet 10.147.46.102/24 brd 10.147.46.255 scope global eth2
inet 10.147.46.112/24 brd 10.147.46.255 scope global secondary eth2

"eth2": [
{
"add": true, 
"broadcast": "10.147.46.255", 
"cidr": "10.147.46.102/24", 
"device": "eth2", 
"first_i_p": true, 
"gateway": "10.147.46.1", 
"netmask": "255.255.255.0", 
"network": "10.147.46.0/24", 
"new_nic": false, 
"nic_dev_id": 2, 
"nw_type": "public", 
"one_to_one_nat": false, 
"public_ip": "10.147.46.102", 
"size": "24", 
"source_nat": true, 
"vif_mac_address": "06:4a:54:00:00:0e"
}, 
{
"add": false, 
"broadcast": "10.147.46.255", 
"cidr": "10.147.46.107/24", 
"device": "eth2", 
"first_i_p": true, 
"gateway": "10.147.46.1", 
"netmask": "255.255.255.0", 
"network": "10.147.46.0/24", 
"new_nic": false, 
"nic_dev_id": 2, 
"nw_type": "public", 
"one_to_one_nat": true, 
"public_ip": "10.147.46.107", 
"size": "24", 
"source_nat": true, 
"vif_mac_address": "06:b5:36:00:00:13"
}, 
{
"add": false, 
"broadcast": "10.147.46.255", 
"cidr": "10.147.46.108/24", 
"device": "eth2", 
"first_i_p": true, 
"gateway": "10.147.46.1", 
"netmask": "255.255.255.0", 
"network": "10.147.46.0/24", 
"new_nic": false, 
"nic_dev_id": 2, 
"nw_type": "public", 
"one_to_one_nat": true, 
"public_ip": "10.147.46.108", 
"size": "24", 
"source_nat": true, 
"vif_mac_address": "06:6d:c8:00:00:14"
}, 
{
"add": false, 
"broadcast": "10.147.46.255", 
"cidr": "10.147.46.111/24", 
"device": "eth2", 
"first_i_p": true, 
"gateway": "10.147.46.1", 
"netmask": "255.255.255.0", 
"network": "10.147.46.0/24", 
"new_nic": false, 
"nic_dev_id": 2, 
"nw_type": "public", 
"one_to_one_nat": true, 
"public_ip": "10.147.46.111", 
"size": "24", 
"source_nat": true, 
"vif_mac_address": "06:32:90:00:00:17"
}, 
{
"add": true, 
"broadcast": "10.147.46.255", 
"cidr": "10.147.46.112/24", 
"device": "eth2", 
"first_i_p": true, 
"gateway": "10.147.46.1", 
"netmask": "255.255.255.0", 
"network": "10.147.46.0/24", 
"new_nic": false, 
"nic_dev_id": 2, 
"nw_type": "public", 
"one_to_one_nat": true, 
"public_ip": "10.147.46.112", 
"size": "24", 
"source_nat": true, 
"vif_mac_address": "06:83:68:00:00:18"
}


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[jayapalu]

Github user jayapalu commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
@ProjectMoon  The ip issue can be worked as separate ticket/PR. It is an 
isolated issue, it is not having any dependency with this PR. So we will get 
this PR in and create a separate ticket for the ip issue. What do you say ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[ProjectMoon]

Github user ProjectMoon commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
@jayapalu Quite possible, yes. It could be fixed by adding another 
condition to the check that was added with #1907. It could also be "fixed" by 
checkiing if `configured()` returns `True` in the `arpPing()` method of the 
`CsIp` class. But I'm guessing that it doesn't get properly deleted from the 
DataBag when it's removed from the router. Thus it just gets written to 
`/etc/cloudstack/ips.json` again.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[jayapalu]

Github user jayapalu commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
@ProjectMoon 
I have seen the issue of ips.json having the "add:true" while I am testing 
this PR. I tested this PR by manually removing the ip or setting add to false. 
Also I tested with the cleanup restart. With correct ips.json I did not see any 
issues with this PR.

I think the ips.json problem is independent of this PR that need to be 
fixed.







---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[ProjectMoon]

Github user ProjectMoon commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
We have been testing this PR internally for some time now, and we have seen 
some strange `arping` issues. We have seen that `/etc/cloudstack/ips.json` on 
the router has IPs on them that aren't cleaned up. Since #1908 is now merged, 
I'm worried that the changes in this PR will not play well with #1908.

Specifically, we have seen that IPs left over in the `ips.json` file can be 
stuck on `"add": true` even if the IP is no longer on the account. 

Thus, the code that was added in PR 1907 concerns me:

```
 if not address['add'] and not ip.configured():
logging.info("Skipping %s as the add flag is set to %s " % 
(address['public_ip'], address['add']))
continue
```

The `if` condition fails in a case we've been investigating. Because our IP 
is `"add": true`, but `configured()` returns `False`, the IP gets reconfigured 
anyway. This doesn't add the IP to the router's interface, but it _does_ run an 
`arping` command (see CsAddress.py). Essentially, if this PR is merged as-is, 
we might wind up with a regression on CLOUDSTACK-9500. 

I will do a bit of manual testing combining both #1908 and #1907 to see 
what happens. It may just be that after restarting the network (and thus 
cleaning the router), these issues will vanish.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[ProjectMoon]

Github user ProjectMoon commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
I'm looking at the `DataBag` code, and it seems that it loads information 
from the data located at `/etc/cloudstack/` and therefore 1907 should work fine 
with this, ASSUMING that is smart enough to recognize that the IP should no 
longer be there.

I think this is a scenario that should definitely be tested as it has the 
potential to break the whole PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[ProjectMoon]

Github user ProjectMoon commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
Question: would #1907 be relevant to this? That change relied on sending 
all the IPs to the router, but under this change, we will be sending one at a 
time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1908: CLOUDSTACK-9317: Fixed disable static nat on leaving...

[ProjectMoon]

Github user ProjectMoon commented on the issue:

https://github.com/apache/cloudstack/pull/1908
  
Thanks for adding the missing parts. Here is the PR that was open against 
4.8 for this, which I guess can be closed now: 
https://github.com/apache/cloudstack/pull/1623

I guess the comment from @jburwell still applies to this.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---