add use of virsh domifaddr to get VM external DHCP IP by rg9975 · Pull Request #10376 · apache/cloudstack

rg9975 · 2025-02-12T15:10:57Z

Description

Adds use of virsh domifaddr command as first choice for finding the external DHCP address of a newly provisioned VM. This command returns address information about all the interfaces when the Qemu OS guest agent is installed within the VM. When this command fails to find the IP, falls back to original behavior of looking at OS disk lease files to find the data.

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

Screenshots (if appropriate):

How Has This Been Tested?

Tested locally with VM running Qemu guest agent.

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

Tested with VM having guest agent and not having agent for expected behaviors.

yadvr · 2025-02-13T07:03:35Z

@blueorangutan package

blueorangutan · 2025-02-13T07:04:03Z

@rohityadavcloud 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 · 2025-02-13T07:11:25Z

Codecov Report

Attention: Patch coverage is 92.70833% with 7 lines in your changes missing coverage. Please review.

Project coverage is 15.16%. Comparing base (ae1d7cc) to head (ebebf42).
Report is 13 commits behind head on 4.19.

Files with missing lines	Patch %	Lines
...e/wrapper/LibvirtGetVmIpAddressCommandWrapper.java	92.70%	1 Missing and 6 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## 4.19 #10376 +/- ##
=========================================
Coverage 15.15% 15.16% - Complexity 11314 11325 +11 
=========================================
Files 5413 5413 Lines 474670 474762 +92 Branches 57890 57900 +10 =========================================
+ Hits 71943 71997 +54 - Misses 394680 394716 +36 - Partials 8047 8049 +2

Flag	Coverage Δ
uitests	`4.29% <ø> (-0.01%)`	⬇️
unittests	`15.88% <92.70%> (+<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.

blueorangutan · 2025-02-13T08:09:07Z

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

DaanHoogland · 2025-02-13T10:37:52Z

@blueorangutan test

blueorangutan · 2025-02-13T10:40:03Z

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

blueorangutan · 2025-02-14T00:36:29Z

[SF] Trillian test result (tid-12390)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 47368 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10376-t12390-kvm-ol8.zip
Smoke tests completed. 132 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_list_cpvm_vm	`Failure`	0.05	test_ssvm.py
test_04_cpvm_internals	`Failure`	0.05	test_ssvm.py

Pearl1594

code LGTM

DaanHoogland

one logic question.
in general I'd like to see the execute method split out further for better readability

…s; added test cases to cover 90%+ scenarios

weizhouapache · 2025-02-26T15:18:30Z

@blueorangutan package

blueorangutan · 2025-02-26T15:20:04Z

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

…s; added test cases to cover 90%+ scenarios

DaanHoogland

clgtm

blueorangutan · 2025-02-26T17:13:08Z

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

DaanHoogland · 2025-02-27T08:46:08Z

@blueorangutan test

blueorangutan · 2025-02-27T08:48:04Z

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

weizhouapache · 2025-02-27T20:34:22Z

This does work in my testing on ubuntu 24.
I will give a suggestion tomorrow

blueorangutan · 2025-02-27T23:18:53Z

[SF] Trillian test result (tid-12503)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 49673 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10376-t12503-kvm-ol8.zip
Smoke tests completed. 132 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_redundant_vpc_site2site_vpn	`Failure`	434.27	test_vpc_vpn.py

yadvr · 2025-02-28T04:44:26Z

Sorry missed your comment @weizhouapache if there are further changes you could review if this needs to be reverted or you want to raise a new PR?

weizhouapache · 2025-02-28T06:14:28Z

Sorry missed your comment @weizhouapache if there are further changes you could review if this needs to be reverted or you want to raise a new PR?

@rohityadavcloud
no worries, I will address it in another PR #10431

btw, actually I missed a word in my previous comment. 🤦‍♂️

This does NOT work in my testing on ubuntu 24.

rg9975 · 2025-02-28T14:44:28Z

Sorry missed your comment @weizhouapache if there are further changes you could review if this needs to be reverted or you want to raise a new PR?
@rohityadavcloud no worries, I will address it in another PR #10431
btw, actually I missed a word in my previous comment. 🤦‍♂️
This does NOT work in my testing on ubuntu 24.

What exactly is the problem you are seeing? you mean Ubuntu on the hypervisor or on the guest OS?

weizhouapache · 2025-02-28T15:15:25Z

What exactly is the problem you are seeing? you mean Ubuntu on the hypervisor or on the guest OS?

@rg9975
never mind
I found I was testing the first commit of the PR.
The issue has been fixed by the 2nd commit in this PR 2 days ago.

weizhouapache · 2025-02-28T16:52:39Z

+ static String virt_cat_path = null;
+ static String tail_path = null;
+
+ static void init() {


@rg9975
the method is never used in LibvirtGetVmIpAddressCommandWrapper.java
which breaks the functionality
I will fix it in #10431

NuxRo · 2025-03-03T10:58:51Z

Thanks @rg9975 and @weizhouapache - I'm watching this closely and will be testing.

Very interested in the feature for both shared and especially L2 networks.

* add use of virsh domifaddr to get VM external DHCP IP * updates to modularize LibvirtGetVmIpAddressCommandWrapper per comments; added test cases to cover 90%+ scenarios * updates to modularize LibvirtGetVmIpAddressCommandWrapper per comments; added test cases to cover 90%+ scenarios * updates to modularize LibvirtGetVmIpAddressCommandWrapper per comments; added test cases to cover 90%+ scenarios

add use of virsh domifaddr to get VM external DHCP IP
76b34fb

boring-cyborgBot added the component:kvm label Feb 12, 2025

DaanHoogland added this to the 4.19.3 milestone Feb 13, 2025

JoaoJandre approved these changes Feb 13, 2025
View reviewed changes

Pearl1594 requested review from DaanHoogland and weizhouapache February 20, 2025 14:43

Pearl1594 mentioned this pull request Feb 20, 2025
server: fetch IP of VMs on L2 networks #10431
Merged
14 tasks

Pearl1594 approved these changes Feb 20, 2025
View reviewed changes

DaanHoogland reviewed Feb 20, 2025
View reviewed changes

Comment thread...main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java Outdated
Comment thread...main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapper.java Outdated

updates to modularize LibvirtGetVmIpAddressCommandWrapper per comment…
1e3cfbd
…s; added test cases to cover 90%+ scenarios

DaanHoogland reviewed Feb 26, 2025
View reviewed changes

Comment thread.../java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmIpAddressCommandWrapperTest.java Outdated

Glover, Rene (rg9975) added 2 commits February 26, 2025 09:37

updates to modularize LibvirtGetVmIpAddressCommandWrapper per comment…
1136824
…s; added test cases to cover 90%+ scenarios

updates to modularize LibvirtGetVmIpAddressCommandWrapper per comment…
ebebf42
…s; added test cases to cover 90%+ scenarios

DaanHoogland approved these changes Feb 26, 2025
View reviewed changes

yadvr modified the milestones: 4.19.3, 4.20.1 Feb 28, 2025

yadvr merged commit f017985 into apache:4.19Feb 28, 2025

weizhouapache reviewed Feb 28, 2025
View reviewed changes

Pearl1594 added this to ACS 4.20.1 Mar 17, 2025

Pearl1594 moved this to Done in ACS 4.20.1 Mar 17, 2025

rg9975 deleted the kvm-ip-externaldhcp-domifaddr branch June 23, 2025 01:57

Uh oh!

Conversation

rg9975 commented Feb 12, 2025

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!

yadvr commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

codecovBot commented Feb 13, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

DaanHoogland commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 13, 2025

Uh oh!

blueorangutan commented Feb 14, 2025

Uh oh!

Pearl1594 left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

weizhouapache commented Feb 26, 2025

Uh oh!

blueorangutan commented Feb 26, 2025

Uh oh!

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Feb 26, 2025

Uh oh!

DaanHoogland commented Feb 27, 2025

Uh oh!

blueorangutan commented Feb 27, 2025

Uh oh!

weizhouapache commented Feb 27, 2025

Uh oh!

blueorangutan commented Feb 27, 2025

Uh oh!

yadvr commented Feb 28, 2025

Uh oh!

weizhouapache commented Feb 28, 2025

Uh oh!

rg9975 commented Feb 28, 2025

Uh oh!

weizhouapache commented Feb 28, 2025

Uh oh!

weizhouapacheFeb 28, 2025

Choose a reason for hiding this comment

Uh oh!

NuxRo commented Mar 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecovBot commented Feb 13, 2025•
edited
Loading