Open Stack

John Griffith <john.griffith at solidfire.com> wrote on 03/10/2013 08:32:56
AM:

> From: John Griffith <john.griffith at solidfire.com>
> To: OpenStack Development Mailing List
<openstack-dev at lists.openstack.org>,
> Date: 03/10/2013 08:36 AM
> Subject: Re: [openstack-dev] [Cinder] test_preattach_status_volume
> test failure
>
>

> On Sat, Mar 9, 2013 at 4:54 PM, Michael J Fork <mjfork at us.ibm.com> wrote:
> John Griffith <john.griffith at solidfire.com> wrote on 03/09/2013 12:09:41
AM:
>
> > From: John Griffith <john.griffith at solidfire.com>
> > To: OpenStack Development Mailing List
<openstack-dev at lists.openstack.org>,
> > Date: 03/09/2013 12:22 AM
> > Subject: Re: [openstack-dev] [Cinder] test_preattach_status_volume
> > test failure
>
> >
> >
>
> > On Fri, Mar 8, 2013 at 1:07 AM, Michael J Fork <mjfork at us.ibm.com>
wrote:
> > After updating the Cinder oslo rpc libraries to the latest (see
> > https://review.openstack.org/#/c/23822/), Cinder
> > test_preattach_status_volume fails with the following:
> >
> > ======================================================================
> > FAIL: Ensure volume goes into pre-attaching state
> > ----------------------------------------------------------------------
> > Traceback (most recent call last):
> >   File "/mnt/home/openstack/dev/cinder/cinder/tests/test_volume.py",
> > line 308, in test_preattach_status_volume
> >     self.assertEqual(vol['status'], "available")
> >
> > and the test
> >
> >     def test_preattach_status_volume(self):
> >         """Ensure volume goes into pre-attaching state"""
> >         instance_uuid = '12345678-1234-5678-1234-567812345678'
> >         mountpoint = "/dev/sdf"
> >         volume = db.volume_create(self.context, {'size': 1,
> >                                                  'status':
'available'})
> >         volume_id = volume['id']
> >
> >         volume_api = cinder.volume.api.API()
> >         volume_api.attach(self.context, volume, instance_uuid,
mountpoint)
> >
> >         vol = db.volume_get(self.context, volume_id)
> >         self.assertEqual(vol['status'], "available")
> >         self.assertEqual(vol['attach_status'], None)
> >         self.assertEqual(vol['instance_uuid'], None)
> >
> > I would expect volume_api.attach call to set the state to "in-use"
> > (as the error shows it was) vs still being "available". Is this a
> > legitimate bug that needs fixed or is the error caused by something
> > in the oslo update?
> >
> > Thanks.
> >
> > Michael
> >
> > -------------------------------------------------
> > Michael Fork
> > Architect, OpenStack Development
> > IBM Systems & Technology Group
> >
> > _______________________________________________
> > OpenStack-dev mailing list
> > OpenStack-dev at lists.openstack.org
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
> > There's a bug in here (although I suspect it's a bug in the unit
> > test code), just haven't figured out how the oslo change exposed it.
> >  The status should be "in-use" as you pointed out, if you run the
> > test individually or even just test_volume:VolumeTestCase, it
> > passes.  So there's something wonky in the test setup somewhere.

> 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?
>
>
> > _______________________________________________
> > OpenStack-dev mailing list
> > OpenStack-dev at lists.openstack.org
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
> Michael
>
> -------------------------------------------------
> Michael Fork
> Architect, OpenStack Development
> IBM Systems & Technology Group
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

> Hey Michael,
>
> 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.

Skipped the test as part of https://review.openstack.org/#/c/23822/ and
opened https://bugs.launchpad.net/cinder/+bug/1153108 to track.

> Thanks,
> John
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Michael

-------------------------------------------------
Michael Fork
Architect, OpenStack Development
IBM Systems & Technology Group
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130310/8d3f14e1/attachment.html>