Open Stack

On 03/12/2014 05:51 AM, Daniel P. Berrange wrote:
> On Tue, Mar 11, 2014 at 03:31:19PM -0500, Matt Riedemann wrote:
>>
>>
>> On 3/11/2014 3:11 PM, Jay Pipes wrote:
>>> On Tue, 2014-03-11 at 14:18 -0500, Matt Riedemann wrote:
>>>>
>>>> On 3/10/2014 11:20 AM, Dmitry Borodaenko wrote:
>>>>> On Fri, Mar 7, 2014 at 8:55 AM, Sean Dague <sean at dague.net> wrote:
>>>>>> On 03/07/2014 11:16 AM, Russell Bryant wrote:
>>>>>>> On 03/07/2014 04:19 AM, Daniel P. Berrange wrote:
>>>>>>>> On Thu, Mar 06, 2014 at 12:20:21AM -0800, Andrew Woodward wrote:
>>>>>>>>> I'd Like to request A FFE for the remaining patches in the Ephemeral
>>>>>>>>> RBD image support chain
>>>>>>>>>
>>>>>>>>> https://review.openstack.org/#/c/59148/
>>>>>>>>> https://review.openstack.org/#/c/59149/
>>>>>>>>>
>>>>>>>>> are still open after their dependency
>>>>>>>>> https://review.openstack.org/#/c/33409/ was merged.
>>>>>>>>>
>>>>>>>>> These should be low risk as:
>>>>>>>>> 1. We have been testing with this code in place.
>>>>>>>>> 2. It's nearly all contained within the RBD driver.
>>>>>>>>>
>>>>>>>>> This is needed as it implements an essential functionality that has
>>>>>>>>> been missing in the RBD driver and this will become the second release
>>>>>>>>> it's been attempted to be merged into.
>>>>>>>>
>>>>>>>> Add me as a sponsor.
>>>>>>>
>>>>>>> OK, great.  That's two.
>>>>>>>
>>>>>>> We have a hard deadline of Tuesday to get these FFEs merged (regardless
>>>>>>> of gate status).
>>>>>>>
>>>>>>
>>>>>> As alt release manager, FFE approved based on Russell's approval.
>>>>>>
>>>>>> The merge deadline for Tuesday is the release meeting, not end of day.
>>>>>> If it's not merged by the release meeting, it's dead, no exceptions.
>>>>>
>>>>> Both commits were merged, thanks a lot to everyone who helped land
>>>>> this in Icehouse! Especially to Russel and Sean for approving the FFE,
>>>>> and to Daniel, Michael, and Vish for reviewing the patches!
>>>>>
>>>>
>>>> There was a bug reported today [1] that looks like a regression in this
>>>> new code, so we need people involved in this looking at it as soon as
>>>> possible because we have a proposed revert in case we need to yank it
>>>> out [2].
>>>>
>>>> [1] https://bugs.launchpad.net/nova/+bug/1291014
>>>> [2]
>>>> https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bug/1291014,n,z
>>>
>>> Note that I have identified the source of the problem and am pushing a
>>> patch shortly with unit tests.
>>>
>>
>> My concern is how much else where assumes nova is working with the
>> glance v2 API because there was a nova blueprint [1] to make nova
>> work with the glance V2 API but that never landed in Icehouse, so
>> I'm worried about wack-a-mole type problems here, especially since
>> there is no tempest coverage for testing multiple image location
>> support via nova.
>>
>> [1] https://blueprints.launchpad.net/nova/+spec/use-glance-v2-api
> 
> Does anyone understand how we can have missed this glance API compat
> problem in gate and/or day-to-day development. Presumably the people
> developing this feature were using a standard devstack environment
> and so would have been relying on whatever is currently committed
> in tree, and so not impacted by whatever blueprint did not land.
> So why would it have worked for them and passed gate tests but then
> fail in this way due to glance API changes ?

It's a little complicated, and comes down to a few reasons.

First, it's a client compatibility issue. Tempest doesn't test the
clients (mostly) because the API is our unbreakable interface, not the
clients (that being said, client compatibility should be something those
teams strive for). The clients actually hide too much of the API, so
testing through the clients won't give the Tempest API tests strict
enough results.

So direct testing of this wasn't expected.

We do a ton of indirect testing as well. Where we call Nova and it calls
Glance. Because no one made progress on -
https://blueprints.launchpad.net/nova/+spec/use-glance-v2-api, no one
got to the point of realizing it was non trivial to enable on the gate
side. So all the indirect calls are v1 still (
http://logs.openstack.org/29/79329/1/check/check-tempest-dsvm-full/cea4ff0/logs/screen-g-api.txt.gz?level=INFO

There is a third way we could have caught this, which is the scenario
tests in tempest, which use the official clients. Probably for the same
reasons as #2, those haven't been enabled on v2. Realistically the
scenario tests probably wouldn't have caught this break anyway, because
they aren't trying to test the entire parameter space, they are testing
through paths. And all the indirect calls would still have been v1,
which is where things are.


The fix, glanceclient should protect the user from this API change. It
seems kind of crazy that it didn't.


But honestly, this isn't the only challenge here. Cinder is in the same
boat (actually a worse boat, because it's v2 API coverage is very very
low in tempest), and those patches to enable this in Nova were nixed in
Icehouse. So again, their v2 API is massively less tested because we
lose all the indirect calls. Keystone, same issue. Gate is on v2,
largely due to the fact that the clients don't support keystone v3. A
set of patches is working their way through to make Tempest itself to be
able to switch between Keystone v2 / v3, because we actually do a bunch
of indirect calls to create tenants during a gate run (1 per class), so
that would be something.


This is definitely a challenge of dual APIs.

That being said, now is a great time in the cycle to be contributing
Tempest tests to shore this up. The trees remain open for new tests on
integrated projects for exactly these reasons, right up until the RCs
are all out.

	-Sean

-- 
Sean Dague
Samsung Research America
sean at dague.net / sean.dague at samsung.com
http://dague.net

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140312/60af4ea8/attachment.pgp>