tempest volume transfer test can race against other tenants

[Sean Dague]

The tempest volume transfer test includes the following:

        # List all volume transfers, there's only one in this test
        resp, body = self.client.list_volume_transfers()
        self.assertEqual(200, resp.status)
        self.assertEqual(volume['id'], body[0]['volume_id'])

There is a race here, and it's unclear if it's racing against the other tenant (running the XML test) or a previous operation that we did ourselves. We should figure out what the right behavior is here, as [0] may or may not be correct.

[Sean Dague (sdague)]

Marking this as also possibly a cinder bug, because it's unclear to me if we have a bad test, or we found cinder behavior that's not right.

This was seen in http://logs.openstack.org/78/62078/12/check/check-tempest-dsvm-full/3b500e9/

[David Kranz (david-kranz)]

This happened a few times in the last week, but many more times today. I looked at the tempest code in test_volume_transfers.py carefully and do not see a race if all the volume calls are scoped to an isolated pair of tenants which they seem to be. But this feature is new in Havana and involves operations between tenants so I would suspect a bug in cinder where somehow an old transfer is somehow getting included in the returned list.

[Ollie Leahy (oliver-leahy-l)]

As far as I can see this problem is happening because the call to get transfers at line 109 in test_volume_transfers.py is getting a list which includes transfers belonging to other tenants. So if it happens that another tempest volume transfer test is in progress in another client, then the list will include that other client's transfers and the assert fails at line 111.

At the moment I'm looking into why get transfers is returning more entries than it should, it could be a cinder problem, or it could be that the client creds have more rights than they should have.

[David Kranz (david-kranz)]

I think this is the same bug as https://bugs.launchpad.net/cinder/+bug/1255292. The point I was making is that each run of this test (due to tenant islolation code) creates two new brand new tenants and creates one transfer. So there should always be exactly one transfer listed. Is that incorrect? If not, then the problem has to be that cinder is getting confused about tenant tracking of these transfers.

[OpenStack Infra (hudson-openstack)]

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

[Ollie Leahy (oliver-leahy-l)]

I think all you're saying is correct.

I'm guessing that when two tests are running simultaneously in different vm's they use the same database. If each creates a tenant and a transfer, then each tenant can see the other's transfer. I wasn't able to exactly reproduce that, but I did reproduce the situation where a single vm created two tenants and transfers that generated a similar signature.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/63167
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=d017dbacc89233ec0f786f5af5d760b02b67a7cb
Submitter: Jenkins
Branch: master

commit d017dbacc89233ec0f786f5af5d760b02b67a7cb
Author: Ollie Leahy <email address hidden>
Date: Thu Dec 19 16:11:29 2013 +0000

    Fix sqlalchemy bug in transfer_get_all_by_project

    Because of incorrect use of the sqlalchemy methods joinedload and filter,
    incorrect lists of volume transfers were being returned by
    transfer_get_all_by_project().

    Change-Id: I378a512611591eec896edd5f217e75f9ff0f8ad3
    Closes-Bug: 1262432