[openstack-dev] [nova][ironic] what to do with unit test failures from ironic api contract

Day, Phil philip.day at hp.com
Mon Jun 16 09:42:31 UTC 2014


From: David Shrewsbury [mailto:shrewsbury.dave at gmail.com]
Sent: 14 June 2014 02:10
To: OpenStack Development Mailing List (not for usage questions)
Cc: Shrewsbury, David; Van Der Veen, Devananda
Subject: Re: [openstack-dev] [nova][ironic] what to do with unit test failures from ironic api contract

Hi!

On Fri, Jun 13, 2014 at 9:30 AM, Day, Phil <philip.day at hp.com<mailto:philip.day at hp.com>> wrote:
Hi Folks,

A recent change introduced a unit test to “warn/notify developers” when they make a change which will break the out of tree Ironic virt driver:   https://review.openstack.org/#/c/98201

Ok – so my change (https://review.openstack.org/#/c/68942) broke it as it adds some extra parameters to the virt drive power_off() method – and so I now feel suitable warned and notified – but am not really clear what I’m meant to do next.

So far I’ve:

-          Modified the unit test in my Nova patch so it now works

-          Submitted an Ironic patch to add the extra parameters (https://review.openstack.org/#/c/99932/)

As far as I can see there’s no way to create a direct dependency from the Ironic change to my patch – so I guess its down to the Ironic folks to wait and accept it in the correct sequence ?

Thanks for bringing up this question.

98201 was added at the suggestion of Sean Dague during a conversation
in #openstack-infra to help prevent terrible breakages that affect the gate.
What wasn't discussed, however, is how we should coordinate these changes
going forward.

As for your change, I think what you've done is exactly what we had hoped would
be done. In your particular case, I don't see any need for Nova dev's to not go ahead
and approve 68942 *before* 99932 since defaults are added to the arguments. The
question is, how do we coordinate such changes if a change DOES actually break
ironic?

One suggestion is that if test_ironic_api_contracts.py<https://review.openstack.org/#/c/68942/15/nova/tests/virt/test_ironic_api_contracts.py> is ever changed, Nova require
the Ironic PTL (or a core dev) to vote before approving. That seems sensible to me.
There might be an easier way of coordinating that I'm overlooking, though.

-Dave
--
David Shrewsbury (Shrews)



Hi Dave,

I agree that co-ordination is the key here – if the Ironic change is approved first then Nova and Ironic will continue to work, but there is a risk that the Nova change gets blocked / modified after the Ironic commit which would be painful.

If the Nova change is committed first then Ironic will of course be broken until its change is committed.

I’ll add a pointer and a note to the corresponding change in each of the patches.

Phil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140616/3ecf1da5/attachment.html>


More information about the OpenStack-dev mailing list