Skip to content

server: check if redundant router is supported when restart network with makeredundant = true - #10612

Merged
DaanHoogland merged 1 commit into
apache:4.19from
weizhouapache:4.19-check-snat-when-makeredudant
May 12, 2025
Merged

server: check if redundant router is supported when restart network with makeredundant = true#10612
DaanHoogland merged 1 commit into
apache:4.19from
weizhouapache:4.19-check-snat-when-makeredudant

Conversation

@weizhouapache

Copy link
Copy Markdown
Member

Description

This PR fixes#10491#10574

makeredundant=true should not work when restart network if

  • source nat is not supported, or
  • redundantrouter is not supported

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
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

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

@weizhouapache

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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

codecovBot commented Mar 24, 2025

Copy link
Copy Markdown

Codecov Report

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

Project coverage is 15.16%. Comparing base (6c40a7b) to head (a9da793).
Report is 42 commits behind head on 4.19.

Files with missing linesPatch %Lines
...ain/java/com/cloud/network/NetworkServiceImpl.java0.00%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## 4.19 #10612 +/- ##
=========================================
Coverage 15.16% 15.16% - Complexity 11328 11332 +4 
=========================================
Files 5414 5414 Lines 474811 474816 +5 Branches 57911 57912 +1 =========================================
+ Hits 72017 72027 +10 + Misses 394742 394733 -9 - Partials 8052 8056 +4 
FlagCoverage Δ
uitests4.28% <ø> (ø)
unittests15.89% <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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DaanHooglandDaanHoogland linked an issue Mar 24, 2025 that may be closed by this pull request
@blueorangutan

Copy link
Copy Markdown

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

@weizhouapache

Copy link
Copy Markdown
MemberAuthor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

TestResultTime (s)Test File
test_01_add_delete_kubernetes_supported_versionError1802.01test_kubernetes_supported_versions.py
ContextSuite context=TestListIdsParams>:setupError0.00test_list_ids_parameter.py
ContextSuite context=TestPrivateGwACL>:setupError0.00test_privategw_acl.py
ContextSuite context=TestProjectSuspendActivate>:setupError1528.85test_projects.py
test_CRUD_operations_userdataError1521.65test_register_userdata.py
test_deploy_vm_with_registered_userdataError5.76test_register_userdata.py
test_deploy_vm_with_registered_userdata_with_override_policy_allowError5.76test_register_userdata.py
test_deploy_vm_with_registered_userdata_with_override_policy_appendError5.84test_register_userdata.py
test_deploy_vm_with_registered_userdata_with_override_policy_denyError5.88test_register_userdata.py
test_deploy_vm_with_registered_userdata_with_paramsError5.78test_register_userdata.py
test_link_and_unlink_userdata_to_templateError5.80test_register_userdata.py
test_user_userdata_crudError6.00test_register_userdata.py
ContextSuite context=TestResetVmOnReboot>:setupError0.00test_reset_vm_on_reboot.py
test_01_vpc_site2site_vpnFailure286.96test_vpc_vpn.py

@Pearl1594Pearl1594 moved this to In Progress in ACS 4.20.1Mar 27, 2025

@sureshanapartisureshanaparti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@sureshanaparti

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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

Copy link
Copy Markdown

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

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@weizhouapache
weizhouapache marked this pull request as ready for review April 15, 2025 12:42
@blueorangutan

Copy link
Copy Markdown

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

TestResultTime (s)Test File

@DaanHooglandDaanHoogland self-assigned this May 8, 2025
@DaanHoogland

DaanHoogland commented May 8, 2025

Copy link
Copy Markdown
Contributor

@weizhouapache
on a network with an offering with no source nat:

🐱 > restart network id=1b4351d0-1f7c-4cde-a5ed-8fd8ea54f004 makeredundant=true
{
"account": "admin",
"accountid": "ea229261-2b47-11f0-bc4c-1e006900026d",
"cmd": "org.apache.cloudstack.api.command.user.network.RestartNetworkCmd",
"completed": "2025-05-08T08:19:52+0000",
"created": "2025-05-08T08:19:52+0000",
"domainid": "a67d0cd0-2b47-11f0-bc4c-1e006900026d",
"domainpath": "ROOT",
"jobid": "71b63e4c-0d24-493a-97a9-ca2d4904ba6b",
"jobinstancetype": "Network",
"jobprocstatus": 0,
"jobresult": {
"errorcode": 530,
"errortext": "Service SourceNat is not supported by the network offering com.cloud.api.query.vo.NetworkOfferingJoinVO$$EnhancerByCGLIB$$4334c8a8@8b"
},
"jobresultcode": 530,
"jobresulttype": "object",
"jobstatus": 2,
"userid": "ea2473ef-2b47-11f0-bc4c-1e006900026d"
}

, so that’s good.

On disabling offerings supporting redundant routers and restarting teh network, however:

🐱 > restart vpc makeredundant=true id=1e91b16e-9c4b-42ec-914b-851c744da064 {
"success": true
}

I tried these scenarios with isolated, vpc and shared networks. All the same results.
cc @sureshanaparti

I think we can merge this, and consider the redundant scenario valid for shared networks.

@DaanHoogland
DaanHoogland merged commit bcd23eb into apache:4.19May 12, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in ACS 4.20.1May 12, 2025
@DaanHoogland
DaanHoogland deleted the 4.19-check-snat-when-makeredudant branch May 12, 2025 06:58
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jun 19, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

VR Mising

6 participants

@weizhouapache@blueorangutan@sureshanaparti@DaanHoogland@yadvr@Pearl1594