Open Stack

On 8/21/2014 11:37 AM, Clark Boylan wrote:
>
>
> On Thu, Aug 21, 2014, at 09:25 AM, Matt Riedemann wrote:
>>
>>
>> On 8/21/2014 10:23 AM, Daniel P. Berrange wrote:
>>> On Thu, Aug 21, 2014 at 11:14:33AM -0400, Solly Ross wrote:
>>>> (reply inline)
>>>>
>>>> ----- Original Message -----
>>>>> From: "Daniel P. Berrange" <berrange at redhat.com>
>>>>> To: "OpenStack Development Mailing List (not for usage questions)" <openstack-dev at lists.openstack.org>
>>>>> Sent: Thursday, August 21, 2014 11:05:18 AM
>>>>> Subject: Re: [openstack-dev] [nova][libvirt] Non-readonly connection to libvirt in unit tests
>>>>>
>>>>> On Thu, Aug 21, 2014 at 10:52:42AM -0400, Solly Ross wrote:
>>>>>> FYI, the context of this is that I would like to be able to test some
>>>>>> of the libvirt storage pool code against a live file system, as we
>>>>>> currently test the storage pool code.  To do this, we need at least to
>>>>>> be able to get a proper connection to a session daemon.  IMHO, since
>>>>>> these calls aren't "expensive", so to speak, it should be fine to have
>>>>>> them run against a real libvirt.
>>>>>
>>>>> No it really isn't OK to run against the real libvirt host system when
>>>>> in the unit tests. Unit tests must *not* rely on external system state
>>>>> in this way because it will lead to greater instability and unreliability
>>>>> of our unit tests. If you want to test stuff against the real libvirt
>>>>> storage pools then that becomes a functional / integration test suite
>>>>> which is pretty much what tempest is targetting.
>>>>
>>>> That's all well and good, but we *currently* manipulates the actual file
>>>> system manually in tests.  Should we then say that we should never manipulate
>>>> the actual file system either?  In that case, there are some tests which
>>>> need to be refactored.
>>>
>>> Places where the tests manipulate the filesystem though should be doing
>>> so in an isolated playpen directory, not in the "live" location where
>>> a deployed nova runs, so that's not the same thing.
>>>
>>>>>>> So If we require libvirt-python for tests and that requires
>>>>>>> libvirt-bin, what's stopping us from just removing fakelibvirt since
>>>>>>> it's kind of useless now anyway, right?
>>>>>>
>>>>>> The thing about fakelibvirt is that it allows us to operate against
>>>>>> against a libvirt API without actually doing libvirt-y things like
>>>>>> launching VMs.  Now, libvirt does have a "test:///default" URI that
>>>>>> IIRC has similar functionality, so we could start to phase out fake
>>>>>> libvirt in favor of that.  However, there are probably still some
>>>>>> spots where we'll want to use fakelibvirt.
>>>>>
>>>>> I'm actually increasingly of the opinion that we should not in fact
>>>>> be trying to use the real libvirt library in the unit tests at all
>>>>> as it is not really adding any value. We typically nmock out all the
>>>>> actual API calls we exercise so despite "using" libvirt-python we
>>>>> are not in fact exercising its code or even validating that we're
>>>>> passing the correct numbers of parameters to API calls. Pretty much
>>>>> all we really relying on is the existance of the various global
>>>>> constants that are defined, and that has been nothing but trouble
>>>>> because the constants may or may not be defined depending on the
>>>>> version.
>>>>
>>>> Isn't that what 'test:///default' is supposed to be?  A version of libvirt
>>>> with libvirt not actually touching the rest of the system?
>>>
>>> Yes, that is what it allows for, however, even if we used that URI we
>>> still wouldn't be actually exercising any of the libvirt code in any
>>> meaningful way because our unit tests mock out all the API calls that
>>> get touched. So using libvirt-python + test:///default URI doesn't
>>> really seem to buy us anything, but it does still mean that developers
>>> need to have libvirt installed in order to run  the unit tests. I'm
>>> not convinced that is a beneficial tradeoff.
>>>
>>>>> The downside of fakelibvirt is that it is a half-assed implementation
>>>>> of libvirt that we evolve in an adhoc fashion. I'm exploring the idea
>>>>> of using pythons introspection abilities to query the libvirt-python
>>>>> API and automatically generate a better 'fakelibvirt' that we can
>>>>> guarantee to match the signatures of the real libvirt library. If we
>>>>> had something like that which we had more confidence in, then we could
>>>>> make the unit tests use that unconditionally. This would make our unit
>>>>> tests more reliable since we would not be suspectible to different API
>>>>> coverage in different libvirt module versions which have tripped us up
>>>>> so many times
>>>
>>> Regards,
>>> Daniel
>>>
>>
>> +1000 to removing the need to have libvirt installed to run unit tests,
>> but that's what I'm seeing today unless I'm mistaken since we require
>> libvirt-python which requires libvirt as already pointed out.
>>
>> If you revert the change to require libvirt-python and try to run the
>> unit tests, it fails, see bug 1357437 [1].
>>
>> [1] https://bugs.launchpad.net/nova/+bug/1357437
>>
> Reverting the change to require libvirt-python is insufficient. That
> revert will flip back to using system packages and include libvirt
> python lib from your operating system. Libvirt will still be required
> just via a different avenue (nova does try to fall back on its fake
> libvirt but iirc that doesn't always work so well).
>
> If you want to stop depending on libvirt in unittests you should remove
> the libvirt-python requirement and not try to use the system package
> installed version of libvirt. Then the fake libvirt will have to keep
> working and will be tested in the gate.
>
> Clark
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>

Yeah I'm not suggesting we revert that change to test-requirements, I'm 
more agreeing with the sentiment to use fakelibvirt only and make that 
thing work (and keep it current).

-- 

Thanks,

Matt Riedemann