Skip to content

Conversation

@DaanHoogland
Copy link
Contributor

Description

This PR...

Fixes: #10182

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?

@codecov
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.21%. Comparing base (35fe19f) to head (72a370e).
Report is 55 commits behind head on 4.19.

Additional details and impacted files
@@             Coverage Diff              @@
##               4.19   #10183      +/-   ##
============================================
+ Coverage     15.13%   15.21%   +0.08%     
- Complexity    11272    11454     +182     
============================================
  Files          5408     5409       +1     
  Lines        473958   484517   +10559     
  Branches      57811    62220    +4409     
============================================
+ Hits          71721    73707    +1986     
- Misses       394219   402573    +8354     
- Partials       8018     8237     +219     
Flag Coverage Δ
uitests 4.82% <ø> (+0.52%) ⬆️
unittests 15.90% <ø> (+0.05%) ⬆️

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.

@DaanHoogland DaanHoogland changed the title add workaround from issue reporter cleanup VM IP after expunge in redundant VPC Jan 14, 2025
@weizhouapache
Copy link
Member

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Test Result Time (s) Test File

Copy link
Member

@weizhouapache weizhouapache left a comment

Choose a reason for hiding this comment

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

code lgtm

@DaanHoogland DaanHoogland marked this pull request as ready for review January 21, 2025 08:30
Copy link
Contributor

@Pearl1594 Pearl1594 left a comment

Choose a reason for hiding this comment

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

code LGTM

@DaanHoogland DaanHoogland self-assigned this Jan 31, 2025
@DaanHoogland
Copy link
Contributor Author

@weizhouapache unfortunately this didn't work. the address is removed from the primary VR but not from the backup.

@DaanHoogland
Copy link
Contributor Author

btw, I will test in 4.19-head without this to see what behaviour is there.

@DaanHoogland
Copy link
Contributor Author

also
image
instead of
image
which seems a bit of a regression :(

Copy link
Contributor Author

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

looks good

@DaanHoogland
Copy link
Contributor Author

DaanHoogland commented Feb 1, 2025

@Pearl1594 , you tested right?

@weizhouapache weizhouapache self-requested a review February 1, 2025 07:47
sed -i "s#^OnCalendar=.*#OnCalendar=$LOGROTATE_FREQUENCY#g" /usr/lib/systemd/system/logrotate.timer
sed -i 's#^AccuracySec=.*#AccuracySec=5m#g' /usr/lib/systemd/system/logrotate.timer

sed -i 's/^#\(dhcp-leasefile=\/var\/lib\/misc\/dnsmasq.leases\)/\1/' /etc/dnsmasq.conf
Copy link
Member

Choose a reason for hiding this comment

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

just curious, this setting does not exist in VR of isolated networks as well

root@r-1171-VM:~# grep -r dnsmasq.leases /etc/dnsmasq*
/etc/dnsmasq.conf:#dhcp-leasefile=/var/lib/misc/dnsmasq.leases
/etc/dnsmasq.conf.tmpl:#dhcp-leasefile=/var/lib/misc/dnsmasq.leases

Does the issue exist with redundant VRs of isolated network as well ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point @weizhouapache . Let's test that as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call @weizhouapache in the redundant isolated VR the issue remains. cc @Pearl1594

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added, but now I will have to look at shared networks as well, don't I?

Copy link
Member

Choose a reason for hiding this comment

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

maybe not needed, as redundant VRs are not supported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, but the script is not only for redundant VRs. I'd like to give it a bash anyway.

Copy link
Contributor Author

@DaanHoogland DaanHoogland Feb 4, 2025

Choose a reason for hiding this comment

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

this fails for isolated VRs :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so to specify, it does not fail for shared networks, it fails for redundant isolated networks.
backup router:
image
primary:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that after expunging the second VM both entries disappeared.

@apache apache deleted a comment from blueorangutan Feb 3, 2025
@apache apache deleted a comment from blueorangutan Feb 3, 2025
@apache apache deleted a comment from blueorangutan Feb 3, 2025
@apache apache deleted a comment from blueorangutan Feb 3, 2025
@apache apache deleted a comment from blueorangutan Feb 3, 2025
@apache apache deleted a comment from blueorangutan Feb 3, 2025
@apache apache deleted a comment from blueorangutan Feb 3, 2025
@blueorangutan
Copy link

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

@DaanHoogland
Copy link
Contributor Author

@weizhouapache @Pearl1594 , I think we can merge this as it does improve it for the issue reporter, but we are not done for redundant isolated networks, after this.

@weizhouapache
Copy link
Member

@Pearl1594 @DaanHoogland
Can we configure it in CsDhcp.py ?

@DaanHoogland DaanHoogland removed their assignment Feb 5, 2025
@Pearl1594
Copy link
Contributor

Pearl1594 commented Feb 5, 2025

@Pearl1594 @DaanHoogland Can we configure it in CsDhcp.py ?

@weizhouapache Do you mean update the dnsmasq.conf file content in CsDhcp.py? or restart of the dnsmasq service?

@weizhouapache
Copy link
Member

I tested 4.20 with the CsDhcp.py change, both redundant network and redundant VPC look good

@DaanHoogland @Pearl1594

@Pearl1594
Copy link
Contributor

is it consistent @weizhouapache . I dnoticed that the csdhcp.py change doesnt consistently cleanup the dnsmasq.leases file

@weizhouapache
Copy link
Member

weizhouapache commented Feb 6, 2025

is it consistent @weizhouapache . I dnoticed that the csdhcp.py change doesnt consistently cleanup the dnsmasq.leases file

I tested deletion of 4 vms, all are removed from /var/lib/misc/dnsmasq.leases

@DaanHoogland
Copy link
Contributor Author

so we don´t leave the changes in the router setup scripts in, @weizhouapache ? cc @Pearl1594

@weizhouapache
Copy link
Member

so we don´t leave the changes in the router setup scripts in, @weizhouapache ? cc @Pearl1594

from my testing result, right, no need to update the sh scripts.
if you have time, please test and share your results
cc @Pearl1594 @DaanHoogland

@DaanHoogland
Copy link
Contributor Author

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

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 6, 2025

@blueorangutan
Copy link

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

@DaanHoogland
Copy link
Contributor Author

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Test Result Time (s) Test File

@DaanHoogland DaanHoogland merged commit d453c63 into 4.19 Feb 9, 2025
49 of 50 checks passed
@DaanHoogland DaanHoogland deleted the 10182-expunge-vm branch February 9, 2025 08:34
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expunge vm not release in-memory dhcp record in vpc redundant virtual route correctly

4 participants