Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server: apply rules when VR of Domain VPC is recreated #8354

Merged
merged 2 commits into from
Jun 28, 2024

Conversation

GaOrtiga
Copy link
Contributor

@GaOrtiga GaOrtiga commented Dec 13, 2023

Description

After re-creating a VR from a domain VPC, ACS will not apply static nat, port forward and load balancer rules to IPs that are not associated to the account that owns the VPC.
An adjustment was made to make sure that these rules are applied for all IPs when performing the following procedures:

  • VR restart

  • VPC restart

  • VPC restart with cleanup

  • recreate VR when health-check fails

  • recreate VR on failover

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

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

How Has This Been Tested?

In a local lab, I performed the procedures listed in the description and verified that after applying the changes, the IPs owned by accounts different than the VPC owner were having their rules applied accordingly.

@weizhouapache weizhouapache changed the title apply rules when VR is recreated server: apply rules when VR of Domain VPC is recreated Dec 13, 2023
Copy link

codecov bot commented Dec 13, 2023

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 30.75%. Comparing base (a15b706) to head (77aa9b2).
Report is 389 commits behind head on 4.19.

Files Patch % Lines
...ork/router/VirtualNetworkApplianceManagerImpl.java 66.66% 0 Missing and 1 partial ⚠️
.../router/VpcVirtualNetworkApplianceManagerImpl.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               4.19    #8354      +/-   ##
============================================
+ Coverage     22.25%   30.75%   +8.49%     
- Complexity    22439    33065   +10626     
============================================
  Files          5117     5353     +236     
  Lines        346819   374603   +27784     
  Branches      49790    54634    +4844     
============================================
+ Hits          77190   115196   +38006     
+ Misses       258466   244117   -14349     
- Partials      11163    15290    +4127     
Flag Coverage Δ
simulator-marvin-tests 24.59% <50.00%> (+0.76%) ⬆️
uitests 4.38% <ø> (-0.07%) ⬇️
unit-tests 16.44% <0.00%> (?)

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.

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm, looks like a good cleanup

@DaanHoogland
Copy link
Contributor

@blueorangutan package

@DaanHoogland DaanHoogland added this to the 4.19.0.0 milestone Dec 14, 2023
@blueorangutan
Copy link

@DaanHoogland 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

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

@shwstppr shwstppr modified the milestones: 4.19.0.0, 4.19.1.0 Dec 14, 2023
@shwstppr
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Test Result Time (s) Test File
test_08_migrate_vm Error 46.09 test_vm_life_cycle.py
test_01_verify_ipv6_vpc Error 603.86 test_vpc_ipv6.py
test_05_rvpc_multi_tiers Failure 503.24 test_vpc_redundant.py
test_05_rvpc_multi_tiers Error 503.26 test_vpc_redundant.py
test_01_redundant_vpc_site2site_vpn Failure 579.31 test_vpc_vpn.py

Copy link
Contributor

@GutoVeronezi GutoVeronezi left a comment

Choose a reason for hiding this comment

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

CLGTM

Copy link
Contributor

@GutoVeronezi GutoVeronezi left a comment

Choose a reason for hiding this comment

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

Just a minor adjustment.

Co-authored-by: Daniel Augusto Veronezi Salvador <[email protected]>
@DaanHoogland DaanHoogland changed the base branch from main to 4.19 February 16, 2024 10:13
@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@DaanHoogland 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

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

@DaanHoogland
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Test Result Time (s) Test File
test_02_upgrade_kubernetes_cluster Failure 236.44 test_kubernetes_clusters.py

@GaOrtiga
Copy link
Contributor Author

@DaanHoogland @GutoVeronezi @shwstppr @JoaoJandre

Is there any pending concern regarding this or can we proceed with the merge?

@weizhouapache
Copy link
Member

weizhouapache commented Mar 19, 2024

@DaanHoogland @GutoVeronezi @shwstppr @JoaoJandre

Is there any pending concern regarding this or can we proceed with the merge?

@GaOrtiga
can you ask someone to test it, share the test steps and results ?

@sureshanaparti
Copy link
Contributor

@DaanHoogland @GutoVeronezi @shwstppr @JoaoJandre
Is there any pending concern regarding this or can we proceed with the merge?

@GaOrtiga can you ask someone to test it, share the test steps and results ?

Ping @GaOrtiga Any update on the testing and results?

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

clgtm

@sureshanaparti
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@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

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

@weizhouapache
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@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
Copy link

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

@rohityadavcloud
Copy link
Member

Ping @GaOrtiga Any further update?

@GaOrtiga
Copy link
Contributor Author

Ping @GaOrtiga Any further update?

No updates, just needs testing, I will check if someone can test and post results.

@weizhouapache
Copy link
Member

@GaOrtiga
I am trying to test this feature. However, I cannot find any UI/doc change for the domain VPC (#7153 ).
can you please add the UI changes and create a doc PR ?

@weizhouapache
Copy link
Member

tested ok. cc @sureshanaparti @GaOrtiga

@GaOrtiga
Copy link
Contributor Author

@GaOrtiga I am trying to test this feature. However, I cannot find any UI/doc change for the domain VPC (#7153 ). can you please add the UI changes and create a doc PR ?

@weizhouapache Thank you for testing. Yes I will create the doc PR.

@sureshanaparti
Copy link
Contributor

@GaOrtiga I am trying to test this feature. However, I cannot find any UI/doc change for the domain VPC (#7153 ). can you please add the UI changes and create a doc PR ?

@weizhouapache Thank you for testing. Yes I will create the doc PR.

Hi @GaOrtiga Please update the doc PR reference here once it is ready. Thanks.

@sureshanaparti sureshanaparti merged commit a5c8bb3 into apache:4.19 Jun 28, 2024
25 of 26 checks passed
@GaOrtiga
Copy link
Contributor Author

@weizhouapache @sureshanaparti
Link for the doc PR:
apache/cloudstack-documentation#410

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jul 2, 2024
* apply rules when VR is recreated

* Apply suggestions from code review

Co-authored-by: Daniel Augusto Veronezi Salvador <[email protected]>

Co-authored-by: Gabriel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

8 participants