avocado-framework / avocado-vt

Avocado VT Plugin
https://avocado-vt.readthedocs.org/
Other
83 stars 241 forks source link

Obtaing the ip address of the guest and update address cache #3935

Closed TasmiyaNalatwad closed 1 month ago

TasmiyaNalatwad commented 2 months ago

Few of the times it is seen that the ip address is not being fetch and the avocado runs were failed with error "ERROR: Failures occurred while postprocess:\n\n: Guest virt-tests-vm1 dmesg verification failed: Login timeout expired (output: 'exceeded 240 s timeout, last failure: No ipv4 DHCP lease for MAC aa:bb:cc:dd:ee:ff') "

To handle this error the patch has been sent. The patch helps in obtaining ip address of the guest using "virsh-net-dhcp-leases default" command. If the guest mac address is found in the command output, the mac ipv4 address is obatined and updated in the address.cache

TasmiyaNalatwad commented 2 months ago

Please find the avocado test run results executed on the patch

12:24:12 WARNING : Overriding user setting and enabling kvm bootstrap as guest tests are requested 12:24:13 INFO : Check for environment 12:24:18 INFO : Creating temporary mux dir 12:24:18 INFO : 12:24:18 INFO : Running Guest Tests Suite lifecycle_bck 12:24:18 INFO : Running: /usr/local/bin/avocado run --vt-type libvirt --vt-config /home/tests/data/avocado-vt/backends/libvirt/cfg/lifecycle_bck.cfg --force-job-id cab81490c1993d1add354f23d2c3e9698a106fdf --job-results-dir /home/tests/results --vt-only-filter "virtio_scsi virtio_net qcow2 Ubuntu.24.04.ppc64le" JOB ID : cab81490c1993d1add354f23d2c3e9698a106fdf JOB LOG : /home/tests/results/job-2024-06-24T12.24-cab8149/job.log (1/1) type_specific.io-github-autotest-libvirt.virsh.destroy.error_test.extra_option: STARTED (1/1) type_specific.io-github-autotest-libvirt.virsh.destroy.error_test.extra_option: PASS (256.51 s) RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 JOB HTML : /home/tests/results/job-2024-06-24T12.24-cab8149/results.html JOB TIME : 263.65 s 12:28:49 INFO : 12:28:49 INFO : Summary of test results can be found below: TestSuite

chloerh commented 1 month ago

Hi @TasmiyaNalatwad About the code check of this pr, I found a very weird thing: only 41 checks were run for this pr which did not include code style check while other prs have 50 checks run for ci-check. I have checked one pr created on June 19 and one on June 25, they both had 50 checks run. I'm totally confused. Do you happen to have any idea what happened here? Thanks in advance.

PraveenPenguin commented 1 month ago

@chloerh it is like GitHub action (which trigger CI) only calls for first time contributor when maintainer's ACK it , that's why it missed via automation engine , for regular contributor it triggers automatically (as restrict random request to trigger CI (which is not authentic) thus to save resources )

chloerh commented 4 weeks ago

@chloerh it is like GitHub action (which trigger CI) only calls for first time contributor when maintainer's ACK it , that's why it missed via automation engine , for regular contributor it triggers automatically (as restrict random request to trigger CI (which is not authentic) thus to save resources )

@PraveenPenguin I see. Thanks for explaining. I was worried about something might be wrong with our ci-check (randomly)

luckyh commented 3 weeks ago

Hello @TasmiyaNalatwad @lmr, this introduced a regression against obtaining the guest's ip addresses properly by the changed routine. That's because:

  1. as virsh was not a mandatory dependency for qemu testing, we should not expect the ip addresses could always be obtained via libvirt tooling (virsh).
  2. since avocado-vt supported user-customized networks, even if virsh was installed, in some cases we still can not obtain ip addresses because we merely queried the dhcp leases from the default network managed by libvirt.
luckyh commented 3 weeks ago

@TasmiyaNalatwad @lmr do you have any plan to fix the issue? otherwise, I will revert this commit for the sake of qemu users.

TasmiyaNalatwad commented 3 weeks ago

@luckyh Thanks for the review and inputs. I have a plan to get the IP address without using virsh commands. How about using arp table inspection that lists all known IP-MAC address mappings. This way we can get the ip address of guest for respective Mac address. arp command run on host : arp -n

@luckyh @lmr Requesting your inputs on this approach.

luckyh commented 2 weeks ago

Requesting your inputs on this approach.

@TasmiyaNalatwad as we also need to deal with ipv6 addresses for this context I'd suggest using ip neigh instead (according to arp's manual page [1]). However, it is still not to be the perfect replacement to the serial login approach, because in some cases we could not get the guest ip addresses by inspecting the arp/neighbor table from the host.

Instead, I'd suggest the following approach if the original issue only happened to libvirt testing (--vt_type=libvirt), that is to overwrite the _get_address method to the subclass of the libvirt_vm module via calling virsh domifaddr $domain_name (which is similiar to the approach you used but this time we merely need to do the inspection on the specified domain), for example:

diff --git a/virttest/libvirt_vm.py b/virttest/libvirt_vm.py
index 825674e2f..7ce748804 100644
--- a/virttest/libvirt_vm.py
+++ b/virttest/libvirt_vm.py
@@ -378,6 +378,14 @@ class VM(virt_vm.BaseVM):
             LOG.error("Failed to backup xml file:\n%s", detail)
             return ""

+    def _get_address(self, index=0, ip_version="ipv4", session=None, timeout=60.0):
+        try:
+            return super()._get_address(index, ip_version, session, timeout)
+        except Exception:
+            output = virsh.domifaddr(self.name)
+            # parse the output and return a valid ip
+            ...
+
     def clone(
         self,
         name=None,

Let me know your opinion.

[1] arp manual page

NOTE
       This program is obsolete. For replacement check ip neigh.
luckyh commented 1 week ago

Hi @TasmiyaNalatwad , finally I sent the PR https://github.com/avocado-framework/avocado-vt/pull/3989 in favor of qemu testing, as I feel that the current virsh approach had to be replaced after all. Feel free to issue a new approach for resolving the original issue, thanks.

TasmiyaNalatwad commented 1 week ago

Hi @TasmiyaNalatwad , finally I sent the PR #3989 in favor of qemu testing, as I feel that the current virsh approach had to be replaced after all. Feel free to issue a new approach for resolving the original issue, thanks.

Hi @luckyh , Ok The approach you shared above _get_address method to the subclass of the libvirt_vm module via calling virsh domifaddr $domain_name Looks good to us, this would fix the issue without affecting qemu users. Will share the patch soon regarding the same. Thank you!