Code review comment for lp:~citrix-openstack/nova/xenapi-netinject-prop

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

> Getting XenAPIMigrateInstance (test_xenapi.py) asking for passwords during
> tests:
>
> XenAPIMigrateInstance
> test_finish_resize [sudo] password for swalsh:
> [sudo] password for swalsh:
> OK
>

Thanks for spotting this Sandy! Finish_resize calls _create_vm which in turn calls preconfigure_instance (which writes /etc/network/interfaces). We were not stubbing out utils.execute for the migrate test case.

I already pushed updated code, pleasee see if the tests run fine now.
Another way of doing this is disabling the flag xenapi_inject_image for the migrate use case. If you think that would make more sense, I'll do that.

> Otherwise, impressive branch ... nothing serious to say.
>
> 641 + # it the caller does not provide template object and data
>
> "If the caller ..."
>
> But largely I'm going to trust Trey on ensuring the approach is sound (as he's
> closer to the problem than I)
>

get_injectables is a routine 'shared' between libvirt and nova. In the libvirt backend Cheetah.Template is loaded with a dynamic import as a global variable, and the template file is read when the LibVirtConnection instance is initialized.

We don't do that in XenAPI and I'm a bit wary of using global variables, so I put the template class and the template file as optional variables and then dynamically set them if they are not set from the caller (xenapi backend in this case).
> Once he approves, I will too.

« Back to merge proposal