Skip to content

server: fix global setting system.vm.public.ip.reservation.mode.strictness is not really dynamic#7909

Merged
weizhouapache merged 1 commit intoapache:4.18from
weizhouapache:4.18-fix-system.vm.public.ip.reservation.mode.strictness-dynamic
Aug 25, 2023
Merged

server: fix global setting system.vm.public.ip.reservation.mode.strictness is not really dynamic#7909
weizhouapache merged 1 commit intoapache:4.18from
weizhouapache:4.18-fix-system.vm.public.ip.reservation.mode.strictness-dynamic

Conversation

@weizhouapache
Copy link
Copy Markdown
Member

@weizhouapache weizhouapache commented Aug 24, 2023

Description

If the original value is false, and search build is configured without the condition.
Now change the value to true, it will not get effective due to missing condition.

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)

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?

@weizhouapache
server: fix global setting system.vm.public.ip.reservation.mode.stric…
9a9d176 
…tness is not really dynamic

If the original value is `false`, and search build is configured without the condition. Now change the value to `true`, it will not get effective due to missing condition.
@weizhouapache weizhouapache added this to the 4.18.1.0 milestone Aug 24, 2023
@weizhouapache weizhouapache mentioned this pull request Aug 24, 2023
Closed
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 24, 2023 •
edited
Loading

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 13.06%. Comparing base (8ad1009) to head (9a9d176).
⚠️ Report is 208 commits behind head on 4.18.

Files with missing lines Patch % Lines
...n/java/com/cloud/network/IpAddressManagerImpl.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               4.18    #7909      +/-   ##
============================================
- Coverage     13.06%   13.06%   -0.01%     
+ Complexity     9089     9088       -1     
============================================
  Files          2720     2720              
  Lines        257380   257379       -1     
  Branches      40128    40127       -1     
============================================
- Hits          33622    33620       -2     
- Misses       219535   219537       +2     
+ Partials       4223     4222       -1

☔ 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.

yadvr
yadvr approved these changes Aug 24, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

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

LGTM - not sure if this needs other explicit testing

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Aug 24, 2023

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Aug 24, 2023

@weizhouapache a [SF] 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

blueorangutan commented Aug 24, 2023

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

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Aug 24, 2023

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Aug 24, 2023

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

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Aug 25, 2023

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

Test Result Time (s) Test File
test_02_list_cpvm_vm Failure 0.07 test_ssvm.py
test_04_cpvm_internals Failure 0.03 test_ssvm.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers Failure 434.07 test_vpc_redundant.py
test_05_rvpc_multi_tiers Failure 512.84 test_vpc_redundant.py
test_05_rvpc_multi_tiers Error 512.86 test_vpc_redundant.py

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Aug 25, 2023

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Aug 25, 2023

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

DaanHoogland
DaanHoogland reviewed Aug 25, 2023
View reviewed changes
Comment thread server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
if (SystemVmPublicIpReservationModeStrictness.value()) {
AssignIpAddressSearch.and("forSystemVms", AssignIpAddressSearch.entity().isForSystemVms(), Op.EQ);
}
AssignIpAddressSearch.and("forSystemVms", AssignIpAddressSearch.entity().isForSystemVms(), Op.EQ);
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Aug 25, 2023

Choose a reason for hiding this comment

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

so now, whether the setting is true or false, we allow testing for it in the query!

DaanHoogland
DaanHoogland approved these changes Aug 25, 2023
View reviewed changes
Copy link
Copy Markdown
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.

lgtm

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Aug 25, 2023

@weizhouapache does it make sense to merge this change with #7144 ?

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Aug 25, 2023

@weizhouapache does it make sense to merge this change with #7144 ?

@DaanHoogland
it is already in 🤦
I totally forgot it ...

@weizhouapache weizhouapache merged commit f5a1f41 into apache:4.18 Aug 25, 2023
@DaanHoogland DaanHoogland deleted the 4.18-fix-system.vm.public.ip.reservation.mode.strictness-dynamic branch August 25, 2023 11:47
@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Aug 25, 2023

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

Test Result Time (s) Test File

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yadvr yadvr yadvr approved these changes

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@nvazquez nvazquez Awaiting requested review from nvazquez

Assignees

No one assigned

Labels

component:networking

Projects

None yet

Milestone

4.18.1.0

Development

Successfully merging this pull request may close these issues.

4 participants

@weizhouapache @blueorangutan @DaanHoogland @yadvr