server: throw exception if fail to cleanup IP resources when release a public IP by weizhouapache · Pull Request #9059 · apache/cloudstack

weizhouapache · 2024-05-08T11:40:14Z

Description

When reproduced the issue #8967, I got the following error

2024-05-03T14:24:16,885 WARN  [c.c.n.IpAddressManagerImpl] (API-Job-Executor-44:[ctx-6da31d57, job-15369, ctx-eb716451]) (logid:d43b402b) Unable to revoke all the firewall rules for ip id=2 as a part of ip release

2024-05-03T14:24:29,282 DEBUG [c.c.n.IpAddressManagerImpl] (API-Job-Executor-44:[ctx-6da31d57, job-15369, ctx-eb716451]) (logid:d43b402b) Releasing ip id=2; sourceNat = false

2024-05-03T14:24:29,271 WARN  [c.c.n.IpAddressManagerImpl] (API-Job-Executor-44:[ctx-6da31d57, job-15369, ctx-eb716451]) (logid:d43b402b) Failed to release resources for ip address id=2

2024-05-03T14:24:36,266 WARN  [c.c.n.NetworkServiceImpl] (API-Job-Executor-44:[ctx-6da31d57, job-15369, ctx-eb716451]) (logid:d43b402b) Failed to release public ip address id=2

The errors are ignored, public IP is released successfully in cloudstack, but the IP is still associated to a VR. When associate the IP to another network, it caused an issue similar as #8967.
However, the reporter of #8967 could not find any error like "Failed to release" or "Unable to revoke" in their logs, so the root cause of #8967 could be different.

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)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

…a public IP

weizhouapache · 2024-05-08T11:40:42Z

@blueorangutan package

blueorangutan · 2024-05-08T11:42:03Z

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

codecov-commenter · 2024-05-08T11:49:14Z

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 14.95%. Comparing base (ea9a0f4) to head (8d0aab5).
Report is 199 commits behind head on 4.19.

Files	Patch %	Lines
...n/java/com/cloud/network/IpAddressManagerImpl.java	0.00%	5 Missing ⚠️
...n/java/com/cloud/network/dao/IPAddressDaoImpl.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.19    #9059      +/-   ##
============================================
- Coverage     14.96%   14.95%   -0.01%     
- Complexity    10995    11017      +22     
============================================
  Files          5373     5382       +9     
  Lines        469024   470133    +1109     
  Branches      58818    59924    +1106     
============================================
+ Hits          70197    70320     +123     
- Misses       391056   392024     +968     
- Partials       7771     7789      +18

Flag	Coverage Δ
uitests	`4.28% <ø> (-0.04%)`	⬇️
unittests	`15.66% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sureshanaparti · 2024-05-08T12:01:24Z

-            s_logger.warn("Failed to release resources for ip address id=" + addrId);
+            String msg = String.format("Failed to release resources for ip address id=%s", addrId);
+            s_logger.error(msg);
+            throw new CloudRuntimeException(msg);


does this block any other subsequent calls, & changes behavior?

it does, but I think the process should be stopped here if the error happens.

I think this is very much unreadible code:

in a method named disassociatePublicIpAddress(..)

after a call to cleanupIpResources(..)

folows a call to markIpAsUnavailable(..)
at the very least the last one must be renamed. I ropose markIpForReleasing(..). Looking in the code, that is what it actually does.

I agree that some action other than marking for releasing should follow a failure to clean up. Let's investigate the callers and see what can be done.

Hi @weizhouapache should these be addressed here, or later in separate PR?

@sureshanaparti @DaanHoogland
renamed the last method from markAsUnavailable to markAsReleasing

blueorangutan · 2024-05-08T12:53:10Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9576

weizhouapache · 2024-05-08T14:45:32Z

@blueorangutan test matrix

blueorangutan · 2024-05-08T14:46:03Z

@weizhouapache a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2024-05-09T03:25:54Z

[SF] Trillian test result (tid-10194)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43853 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9059-t10194-kvm-centos7.zip
Smoke tests completed. 130 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_events_resource	`Error`	420.30	test_events_resource.py

blueorangutan · 2024-05-09T04:16:46Z

[SF] Trillian test result (tid-10192)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47085 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9059-t10192-xenserver-71.zip
Smoke tests completed. 130 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_events_resource	`Error`	336.85	test_events_resource.py

blueorangutan · 2024-05-09T05:21:32Z

[SF] Trillian test result (tid-10193)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 50852 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9059-t10193-vmware-67u3.zip
Smoke tests completed. 128 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_events_resource	`Error`	351.57	test_events_resource.py
test_create_pvlan_network	`Error`	0.09	test_pvlan.py
test_02_trigger_shutdown	`Failure`	341.73	test_safe_shutdown.py

sureshanaparti

clgtm

sureshanaparti · 2024-06-24T07:10:37Z

@blueorangutan package

blueorangutan · 2024-06-24T07:12:03Z

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

weizhouapache · 2024-06-24T07:21:48Z

@blueorangutan package

blueorangutan · 2024-06-24T07:22:03Z

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-06-24T08:17:20Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10083

blueorangutan · 2024-06-24T08:26:35Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10085

sureshanaparti · 2024-06-26T08:00:59Z

@blueorangutan test

blueorangutan · 2024-06-26T08:02:03Z

@sureshanaparti a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2024-06-26T20:07:42Z

[SF] Trillian test result (tid-10634)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41982 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9059-t10634-kvm-centos7.zip
Smoke tests completed. 131 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

hsato03 · 2024-06-27T20:14:50Z

@@ -732,8 +732,9 @@ public boolean disassociatePublicIpAddress(long addrId, long userId, Account cal
        PublicIpQuarantine publicIpQuarantine = null;
        // Cleanup all ip address resources - PF/LB/Static nat rules
        if (!cleanupIpResources(addrId, userId, caller)) {


Are the IP resources that are cleaned up by the cleanupIpResources method being rolled back when the exception is thrown?

@weizhouapache can you check this ^^^

sureshanaparti · 2024-06-28T11:53:34Z

@blueorangutan package

blueorangutan · 2024-06-28T11:54:03Z

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-06-28T13:02:16Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10195

github-actions · 2024-06-29T05:27:25Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

DaanHoogland

clgtm, but needs testing

kiranchavala · 2024-08-21T08:54:24Z

@weizhouapache

I am not able to reproduce the behaviour, do you have any other specific steps

I have followed these steps

Create 2 sessions and execute the following API at the same time

Execute disassociate IP address
Execute creates a firewall rule on the same IP

Did not observe the log

egrep 'Failed to release | Unable to revoke ' /var/log/cloudstack/management/management-server.log

The create firewall API returns
Error: (HTTP 431, error code 4350) Unable to create firewall rule for the IP address ID=5 as IP is not associated with any network and no networkId is passed in

Also, I see the IPAddress present in the router even after the disassociate IP address is successfull

cat /etc/cloudstack/ips.json

}root@r-4-VM:~# cat /etc/cloudstack/ips.json
{
  "eth0": [
    {
      "add": true,
      "broadcast": "10.1.1.255",
      "cidr": "10.1.1.1/24",
      "device": "eth0",
      "gateway": "",
      "netmask": "255.255.255.0",
      "network": "10.1.1.0/24",
      "nic_dev_id": "0",
      "nw_type": "guest",
      "one_to_one_nat": false,
      "public_ip": "10.1.1.1",
      "size": "24",
      "source_nat": false
    }
  ],
  "eth1": [
    {
      "add": true,
      "broadcast": "169.254.255.255",
      "cidr": "169.254.216.24/16",
      "device": "eth1",
      "gateway": "",
      "netmask": "255.255.0.0",
      "network": "169.254.0.0/16",
      "nic_dev_id": "1",
      "nw_type": "control",
      "one_to_one_nat": false,
      "public_ip": "169.254.216.24",
      "size": "16",
      "source_nat": false
    }
  ],
  "eth2": [
    {
      "add": true,
      "broadcast": "10.0.63.255",
      "cidr": "10.0.54.123/20",
      "device": "eth2",
      "first_i_p": true,
      "gateway": "10.0.48.1",
      "is_private_gateway": false,
      "mtu": "1500",
      "netmask": "255.255.240.0",
      "network": "10.0.48.0/20",
      "new_nic": false,
      "nic_dev_id": 2,
      "nw_type": "public",
      "one_to_one_nat": false,
      "public_ip": "10.0.54.123",
      "size": "20",
      "source_nat": true,
      "vif_mac_address": "1e:00:89:00:00:03"
    },
    {
      "add": false,
      "broadcast": "10.0.63.255",
      "cidr": "10.0.54.124/20",
      "device": "eth2",
      "first_i_p": false,
      "gateway": "10.0.48.1",
      "is_private_gateway": false,
      "mtu": "1500",
      "netmask": "255.255.240.0",
      "network": "10.0.48.0/20",
      "new_nic": false,
      "nic_dev_id": 2,
      "nw_type": "public",
      "one_to_one_nat": false,
      "public_ip": "10.0.54.124",
      "size": "20",
      "source_nat": false,
      "vif_mac_address": "1e:00:89:00:00:03"
    }
  ],
  "id": "ips"

weizhouapache · 2024-08-21T08:59:18Z

@weizhouapache

I am not able to reproduce the behaviour, do you have any other specific steps

I have followed these steps

Create 2 sessions and execute the following API at the same time

Execute disassociate IP address
Execute creates a firewall rule on the same IP

Did not observe the log

egrep 'Failed to release | Unable to revoke ' /var/log/cloudstack/management/management-server.log

The create firewall API returns
Error: (HTTP 431, error code 4350) Unable to create firewall rule for the IP address ID=5 as IP is not associated with any network and no networkId is passed in

Also, I see the IPAddress present in the router even after the disassociate IP address is successfull

cat /etc/cloudstack/ips.json

}root@r-4-VM:~# cat /etc/cloudstack/ips.json
{
  "eth0": [
    {
      "add": true,
      "broadcast": "10.1.1.255",
      "cidr": "10.1.1.1/24",
      "device": "eth0",
      "gateway": "",
      "netmask": "255.255.255.0",
      "network": "10.1.1.0/24",
      "nic_dev_id": "0",
      "nw_type": "guest",
      "one_to_one_nat": false,
      "public_ip": "10.1.1.1",
      "size": "24",
      "source_nat": false
    }
  ],
  "eth1": [
    {
      "add": true,
      "broadcast": "169.254.255.255",
      "cidr": "169.254.216.24/16",
      "device": "eth1",
      "gateway": "",
      "netmask": "255.255.0.0",
      "network": "169.254.0.0/16",
      "nic_dev_id": "1",
      "nw_type": "control",
      "one_to_one_nat": false,
      "public_ip": "169.254.216.24",
      "size": "16",
      "source_nat": false
    }
  ],
  "eth2": [
    {
      "add": true,
      "broadcast": "10.0.63.255",
      "cidr": "10.0.54.123/20",
      "device": "eth2",
      "first_i_p": true,
      "gateway": "10.0.48.1",
      "is_private_gateway": false,
      "mtu": "1500",
      "netmask": "255.255.240.0",
      "network": "10.0.48.0/20",
      "new_nic": false,
      "nic_dev_id": 2,
      "nw_type": "public",
      "one_to_one_nat": false,
      "public_ip": "10.0.54.123",
      "size": "20",
      "source_nat": true,
      "vif_mac_address": "1e:00:89:00:00:03"
    },
    {
      "add": false,
      "broadcast": "10.0.63.255",
      "cidr": "10.0.54.124/20",
      "device": "eth2",
      "first_i_p": false,
      "gateway": "10.0.48.1",
      "is_private_gateway": false,
      "mtu": "1500",
      "netmask": "255.255.240.0",
      "network": "10.0.48.0/20",
      "new_nic": false,
      "nic_dev_id": 2,
      "nw_type": "public",
      "one_to_one_nat": false,
      "public_ip": "10.0.54.124",
      "size": "20",
      "source_nat": false,
      "vif_mac_address": "1e:00:89:00:00:03"
    }
  ],
  "id": "ips"

@kiranchavala
Thanks
This might has been fixed by #9234

weizhouapache · 2024-08-21T08:59:38Z

Closing
Reopen if needed

server: throw exception if fail to cleanup IP resources when release …

4f0b26b

…a public IP

boring-cyborg Bot added the component:networking label May 8, 2024

sureshanaparti reviewed May 8, 2024

View reviewed changes

weizhouapache mentioned this pull request May 14, 2024

IP address conflicts held by public IPs of virtual routers #8967

Closed

yadvr requested review from DaanHoogland and shwstppr June 21, 2024 10:31

yadvr added this to the 4.19.1.0 milestone Jun 21, 2024

sureshanaparti approved these changes Jun 24, 2024

View reviewed changes

Update 9059: rename markAsUnavailable to markAsReleasing

8d0aab5

sureshanaparti added the status:needs-review label Jun 25, 2024

sureshanaparti requested review from vishesh92 and yadvr June 26, 2024 04:43

sureshanaparti marked this pull request as ready for review June 26, 2024 07:47

hsato03 reviewed Jun 27, 2024

View reviewed changes

sureshanaparti added status:needs-testing and removed status:needs-review labels Jun 28, 2024

sureshanaparti modified the milestones: 4.19.1.0, 4.19.2.0 Jun 28, 2024

github-actions Bot added the status:has-conflicts label Jun 29, 2024

DaanHoogland reviewed Jul 1, 2024

View reviewed changes

kiranchavala self-assigned this Aug 20, 2024

weizhouapache closed this Aug 21, 2024

DaanHoogland removed this from the 4.19.2.0 milestone Sep 6, 2024

DaanHoogland removed the status:needs-testing label Jan 28, 2026

Uh oh!

Conversation

weizhouapache commented May 8, 2024

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

weizhouapache commented May 8, 2024

Uh oh!

blueorangutan commented May 8, 2024

Uh oh!

codecov-commenter commented May 8, 2024 • edited by codecov Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sureshanaparti May 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weizhouapache May 8, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jun 21, 2024

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Jun 24, 2024

Choose a reason for hiding this comment

Uh oh!

weizhouapache Jun 24, 2024

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented May 8, 2024

Uh oh!

weizhouapache commented May 8, 2024

Uh oh!

blueorangutan commented May 8, 2024

Uh oh!

blueorangutan commented May 9, 2024

Uh oh!

blueorangutan commented May 9, 2024

Uh oh!

blueorangutan commented May 9, 2024

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Jun 24, 2024

Uh oh!

blueorangutan commented Jun 24, 2024

Uh oh!

weizhouapache commented Jun 24, 2024

Uh oh!

blueorangutan commented Jun 24, 2024

Uh oh!

blueorangutan commented Jun 24, 2024

Uh oh!

blueorangutan commented Jun 24, 2024

Uh oh!

sureshanaparti commented Jun 26, 2024

Uh oh!

blueorangutan commented Jun 26, 2024

Uh oh!

blueorangutan commented Jun 26, 2024

Uh oh!

hsato03 Jun 27, 2024

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Jun 28, 2024

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Jun 28, 2024

Uh oh!

blueorangutan commented Jun 28, 2024

Uh oh!

blueorangutan commented Jun 28, 2024

codecov-commenter commented May 8, 2024 •

edited by codecov Bot

Loading

sureshanaparti May 8, 2024 •

edited

Loading