test_preattach_status_volume is invalid

Bug #1153108 reported by Michael Fork
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
Medium
Avishay Traeger

Bug Description

test_preattach_status_volume in test_volume is being skipped and needs updated based on discussion this thread: http://lists.openstack.org/pipermail/openstack-dev/2013-March/006498.html

From the thread:

Michael Fork wrote:

> I did some digging, and it looks like a subsequent refactoring (
> https://github.com/openstack/cinder/commit/2331d3336a6adf4fc13a3b187e91a5d1b1f7c723)
> of the original commit (
> https://github.com/openstack/cinder/commit/e27b171883c1b87b8d2fdf0994947d4b93e640d9)
> modified the code path here but didn't retain the original purpose of this
> test (which was to ensure the "attaching" state was set with an instance
> UUID on the way to being "attached"). However, I don't know the right way
> to fix. Looking at attach_volume in cinder/volume/manager.py, I can see
> the volume_update call right before attach)volume on the driver is called.
> This test needs to be inserted right after volume_update. How is that
> best done?

John Griffith wrote:

Thanks for following up on this, I tracked down the commit as you did and
started looking at things. What is interesting here though (and what
distracted me a bit) is the fact that if you run the individual test it
makes it's way through the manager code path and works as the test is
written. Same holds true if you run the entire file "test_volume".

It seems that something isn't getting cleaned up properly by another test
somewhere or an assumption was made when this test was written. I have not
figured out the root cause of the issue. This is one of my gripes about a
large percentage of the unit tests, they're a bit convoluted and don't
necessarily really test what one would expect them to.

I'll leave it up to you, if you'd like to disable that test we can log a
bug and note that it's expected to be a "unit test" issue and not an issue
in the code and I can revisit later.

Changed in cinder:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
Avishay Traeger (avishay-il) wrote :

I will try to figure this out.

Changed in cinder:
assignee: nobody → Avishay Traeger (avishay-il)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

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

Changed in cinder:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.openstack.org/24991
Committed: http://github.com/openstack/cinder/commit/d1b37a8471ba5c5d5da315ea39335890b00a52f1
Submitter: Jenkins
Branch: master

commit d1b37a8471ba5c5d5da315ea39335890b00a52f1
Author: Avishay Traeger <email address hidden>
Date: Thu Mar 21 15:08:04 2013 +0200

    Clean up started volume services in tests.

    The test_preattach_status_volume test was failing due to state left by
    another test. The interim solution was to skip the aforementioned test.
    I tracked down the problem to 4 tests in test_admin_actions.py that
    call self.start_service('volume', host='test'), and don't stop the
    service. Stopping it in all 4 tests solves the problem.

    HOWEVER, I don't know why these started services caused this particular
    test to fail. I am still working to figure that out, but maybe someone
    has an idea.

    Change-Id: I2a5caaae9f61e7c7617c934917a10cf1cf3c0db6
    Fixes: bug 1153108

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in cinder:
milestone: none → havana-1
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in cinder:
milestone: havana-1 → 2013.2
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.