nova.tests.virt.test_virt_drivers doesn't produce a support matrix as documented

Bug #1209421 reported by Joe Gordon
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Won't Fix
Medium
Boden R

Bug Description

nova/tests/virt/test_virt_drivers.py has a class called _VirtDriverTestCase That is used to check which functions virt driver support and to help generate a support matrix.

But nova/tests/virt/test_virt_drivers.py only tets libvirt, and the fake driver, and does not provide any mechanism to automatically generate the support matrix.

This file should test all virt drivers and provide a way to produce a support matrix.

Joe Gordon (jogo)
summary: - nova.tests.virt.test_virt_drivers only tests Libvirt and Fake
+ nova.tests.virt.test_virt_drivers doesn't a support matrix as documented
Revision history for this message
Alessandro Pilotti (alexpilotti) wrote : Re: nova.tests.virt.test_virt_drivers doesn't a support matrix as documented

A common driver TestCase would be more than welcome.

We had a few issues on the Hyper-V driver side due to the lack of a similar solution.
Typically, this happens when the contract of a method changes e.g. by adding a key to a dictionary returned by a method.

tags: added: testing
Changed in nova:
status: New → Confirmed
importance: Undecided → Low
Joe Gordon (jogo)
summary: - nova.tests.virt.test_virt_drivers doesn't a support matrix as documented
+ nova.tests.virt.test_virt_drivers doesn't produce a support matrix as
+ documented
Maithem (maithem)
Changed in nova:
assignee: nobody → Maithem (maithem)
Joe Gordon (jogo)
Changed in nova:
assignee: Maithem (maithem) → nobody
importance: Low → High
Matt Riedemann (mriedem)
tags: added: hyper-v vmware xenserver
Boden R (boden)
Changed in nova:
status: Confirmed → In Progress
assignee: nobody → Boden R (boden)
Revision history for this message
Daniel Berrange (berrange) wrote :

I'm not really convinced that the test_virt_drivers test case is useful at all. For a start it can only work if the virt driver has a way to fake all its external interaction.. This works for libvirt thanks to fakelibvirt, but then by virtue of using fakelibvirt this is not really achieving any useful test coverage that we don't already have via the main libvirt test cases.

On the subject of generating a hypervisor support matrix, I think this test case is even more useless. For libvirt you would need to be able to run it for QEMU, KVM, LXC, Xen, etc and report results for each of them. This is impossible with fakelibvirt, and the unit tests can't use the real libvirt since that would make them heavily dependant on external system state.

If we want to have generated reports of supported APIs per hypervisor, then I think that realistically this has to be done using the tempest test suite, or some similar thing which validates a real world deployment of Nova against a live hypervisor.

So I think this bug should be closed as wontfix, or re-assigned to some other project. The unit tests just aren't the right place to generate a useful hypervisor support matrix.

Revision history for this message
Boden R (boden) wrote :

I spent a few hours looking into this one; adding my 2 cents below..

I do agree with Daniel's assessment above in that the hypervisor support matrix (https://wiki.openstack.org/wiki/HypervisorSupportMatrix) can / should not be generated via unit test based analysis and that likely the correct place for such automated analysis is tempest / live tests or similar. In fact I don't think the compute driver UT methods are enough to determine / build the full hypervisor matrix even if we wanted to.

Note -- I suggest if we agree the UT class should not generate the hyper matrix that we at least remove the comments in test_virt_drivers.py which indicates it does for clarity sake.

However I do believe some potentially useful information can be gleamed on a per hypervisor basis using static analysis. As part of my investigation of this bug, I implemented a quick python script which effectively does the following:

- Analyzes the nova ComputeDriver (base) class and determines the list of methods required for the driver "contract". Additionally methods are marked as required or optional based on if the base driver raises NotImplementedError or just does a pass signifying optional.
- Analyzes the hypervisor virt driver (concrete) driver implementations (classes) and determines which methods the virt driver class implements.
- Outputs a matrix which contains the compute driver methods and their implemented status within the virt driver.

For example see the output from the vmwareapi compute driver here: http://paste.openstack.org/show/112285/

This is fairly low level information at the driver method level, however IMO it provides information which I was unable to extract using UT coverage or any other means.

That said I propose the following:
[1] I will submit a patch to remove the comments in test_virt_drivers which indicates it generates a matrix and also remove the dead code which tracks the not implemented errors.
[2] I will start an email thread to determine if there's any interest / resources to work on a means to generate the hyper matrix outside the UTs (e.g. tempest or elsewhere).
[3] I can clean up my simple static analysis script and make it avail on github if anyone thinks its useful?

Thoughts from others are appreciated.

Revision history for this message
Alessandro Pilotti (alexpilotti) wrote :

IMO static analysis is fairly useless while a dedicated set of tempest tests would do.
For most driver features even some of the existing tests can be used for this purpose.

We could include automatically all the drivers/hypervisors with an associated CI (anyway a prereq for being a supported driver) and optionally a separare matrix for unsupported cases like libvirt/Xen, upon submission of the test results, to be linked in the same wiki page.

Gary Kotton (garyk)
tags: added: libvirt
Changed in nova:
importance: High → Medium
Revision history for this message
Boden R (boden) wrote :

test_virt_drivers actually appears to provide some coverage which I don't see elsewhere, namely in testing the base (common) implemented methods of the ComputeDriver in addition to the FakeDriver.

As noted in a previous comment there is obviously some amount of overlap between what's covered in the test_virt_drivers and in the main libvirt driver tests. However there does appear to be some amount of additional coverage in test_virt_drivers from a libvirt perspective and therefore I don't think we should remove it in it's entirety without the due diligence to ensure all coverage is provided in the main libvirt tests.

That said my plan is to submit a small patch which updates the test_virt_drivers comments to reflect its not used to build the hyper matrix and also a very minor logging change to better reflect the unimplemented class / method / test. Assuming that patch goes through I'll be closing this one out. If we believe the hyper matrix should be generated via tempest or other testing magic I suggest we queue that up as a blueprint for the respective team - tho do date I haven't seen a strong need / desire for such.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.openstack.org/122227

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/122227
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=64a911415b677ddaadc9e350aa1017387da56db6
Submitter: Jenkins
Branch: master

commit 64a911415b677ddaadc9e350aa1017387da56db6
Author: Boden R <email address hidden>
Date: Wed Sep 17 15:20:36 2014 -0400

    Clarify virt driver test comments & log statement

    The current implementation of nova.tests.virt.test_virt_drivers
    indicates NotImplementedError should be caught and logged to
    dynamically build the hypervisor support matrix. This is not a
    realistic / optimal approach for building the support matrix and
    thus this patch updates the code comments accordingly for
    clarity. This patch also modifies the logging statement outputted
    in such scenarios to better clarify which test case method is
    executing.

     Partial-Bug: 1209421

    Change-Id: I5d80cdc06207071548fdb5f6852ca96fa8d79776

Revision history for this message
Boden R (boden) wrote :

As noted in comment #5 a patch has been merged which removes the comments in test_virt_drivers indicating a hypervisor matrix should be generated from the unit test compute driver subclass execution. As discussed (above) in this bug; if hypervisor support matrix dynamic generation is needed, we should seek to implement that at the integration test level to provide a more complete picture of driver support.

Closing this as as won't fix since there are no current plans to generate a matrix during UT.

Changed in nova:
status: In Progress → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.