<html><body>
<p><tt><font size="2">John Griffith <john.griffith@solidfire.com> wrote on 03/10/2013 08:32:56 AM:<br>
<br>
> From: John Griffith <john.griffith@solidfire.com></font></tt><br>
<tt><font size="2">> To: OpenStack Development Mailing List <openstack-dev@lists.openstack.org>, </font></tt><br>
<tt><font size="2">> Date: 03/10/2013 08:36 AM</font></tt><br>
<tt><font size="2">> Subject: Re: [openstack-dev] [Cinder] test_preattach_status_volume <br>
> test failure</font></tt><br>
<tt><font size="2">> <br>
> <br>
</font></tt><br>
<tt><font size="2">> On Sat, Mar 9, 2013 at 4:54 PM, Michael J Fork <mjfork@us.ibm.com> wrote:</font></tt><br>
<tt><font size="2">> John Griffith <john.griffith@solidfire.com> wrote on 03/09/2013 12:09:41 AM:<br>
> <br>
> > From: John Griffith <john.griffith@solidfire.com><br>
> > To: OpenStack Development Mailing List <openstack-dev@lists.openstack.org>, <br>
> > Date: 03/09/2013 12:22 AM<br>
> > Subject: Re: [openstack-dev] [Cinder] test_preattach_status_volume <br>
> > test failure</font></tt><br>
<tt><font size="2">> <br>
> > <br>
> > <br>
> <br>
> > On Fri, Mar 8, 2013 at 1:07 AM, Michael J Fork <mjfork@us.ibm.com> wrote:<br>
> > After updating the Cinder oslo rpc libraries to the latest (see <br>
> > <a href="https://review.openstack.org/#/c/23822/">https://review.openstack.org/#/c/23822/</a>), Cinder <br>
> > test_preattach_status_volume fails with the following:<br>
> > <br>
> > ======================================================================<br>
> > FAIL: Ensure volume goes into pre-attaching state<br>
> > ----------------------------------------------------------------------<br>
> > Traceback (most recent call last):<br>
> > File "/mnt/home/openstack/dev/cinder/cinder/tests/test_volume.py",<br>
> > line 308, in test_preattach_status_volume<br>
> > self.assertEqual(vol['status'], "available")<br>
> > <br>
> > and the test<br>
> > <br>
> > def test_preattach_status_volume(self):<br>
> > """Ensure volume goes into pre-attaching state"""<br>
> > instance_uuid = '12345678-1234-5678-1234-567812345678'<br>
> > mountpoint = "/dev/sdf"<br>
> > volume = db.volume_create(self.context, {'size': 1,<br>
> > 'status': 'available'})<br>
> > volume_id = volume['id']<br>
> > <br>
> > volume_api = cinder.volume.api.API()<br>
> > volume_api.attach(self.context, volume, instance_uuid, mountpoint)<br>
> > <br>
> > vol = db.volume_get(self.context, volume_id)<br>
> > self.assertEqual(vol['status'], "available")<br>
> > self.assertEqual(vol['attach_status'], None)<br>
> > self.assertEqual(vol['instance_uuid'], None)<br>
> > <br>
> > I would expect volume_api.attach call to set the state to "in-use" <br>
> > (as the error shows it was) vs still being "available". Is this a <br>
> > legitimate bug that needs fixed or is the error caused by something <br>
> > in the oslo update?<br>
> > <br>
> > Thanks.<br>
> > <br>
> > Michael<br>
> > <br>
> > -------------------------------------------------<br>
> > Michael Fork<br>
> > Architect, OpenStack Development<br>
> > IBM Systems & Technology Group<br>
> > <br>
> > _______________________________________________<br>
> > OpenStack-dev mailing list<br>
> > OpenStack-dev@lists.openstack.org<br>
> > <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
> <br>
> > There's a bug in here (although I suspect it's a bug in the unit <br>
> > test code), just haven't figured out how the oslo change exposed it.<br>
> > The status should be "in-use" as you pointed out, if you run the <br>
> > test individually or even just test_volume:VolumeTestCase, it <br>
> > passes. So there's something wonky in the test setup somewhere.<br>
</font></tt><br>
<tt><font size="2">> I did some digging, and it looks like a subsequent refactoring (<br>
> <a href="https://github.com/openstack/cinder/commit/">https://github.com/openstack/cinder/commit/</a><br>
> 2331d3336a6adf4fc13a3b187e91a5d1b1f7c723) of the original commit (<br>
> <a href="https://github.com/openstack/cinder/commit/">https://github.com/openstack/cinder/commit/</a><br>
> e27b171883c1b87b8d2fdf0994947d4b93e640d9) modified the code path <br>
> here but didn't retain the original purpose of this test (which was <br>
> to ensure the "attaching" state was set with an instance UUID on the<br>
> way to being "attached"). However, I don't know the right way to <br>
> fix. Looking at attach_volume in cinder/volume/manager.py, I can <br>
> see the volume_update call right before attach)volume on the driver <br>
> is called. This test needs to be inserted right after <br>
> volume_update. How is that best done?</font></tt><br>
<tt><font size="2">> <br>
> <br>
> > _______________________________________________<br>
> > OpenStack-dev mailing list<br>
> > OpenStack-dev@lists.openstack.org<br>
> > <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
> <br>
> Michael<br>
> <br>
> -------------------------------------------------<br>
> Michael Fork<br>
> Architect, OpenStack Development<br>
> IBM Systems & Technology Group</font></tt><br>
<tt><font size="2">> <br>
> _______________________________________________<br>
> OpenStack-dev mailing list<br>
> OpenStack-dev@lists.openstack.org<br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</font></tt><br>
<tt><font size="2">> Hey Michael,</font></tt><br>
<tt><font size="2">> <br>
> Thanks for following up on this, I tracked down the commit as you <br>
> did and started looking at things. What is interesting here though <br>
> (and what distracted me a bit) is the fact that if you run the <br>
> individual test it makes it's way through the manager code path and <br>
> works as the test is written. Same holds true if you run the entire<br>
> file "test_volume".</font></tt><br>
<tt><font size="2">> <br>
> It seems that something isn't getting cleaned up properly by another<br>
> test somewhere or an assumption was made when this test was written.<br>
> I have not figured out the root cause of the issue. This is one of <br>
> my gripes about a large percentage of the unit tests, they're a bit <br>
> convoluted and don't necessarily really test what one would expect them to.</font></tt><br>
<tt><font size="2">> <br>
> I'll leave it up to you, if you'd like to disable that test we can <br>
> log a bug and note that it's expected to be a "unit test" issue and <br>
> not an issue in the code and I can revisit later. </font></tt><br>
<br>
<tt><font size="2">Skipped the test as part of </font></tt><tt><font size="2"><a href="https://review.openstack.org/#/c/23822/">https://review.openstack.org/#/c/23822/</a> and opened </font></tt><tt><font size="2"><a href="https://bugs.launchpad.net/cinder/+bug/1153108">https://bugs.launchpad.net/cinder/+bug/1153108</a></font></tt><tt><font size="2"> to track.<br>
<br>
> Thanks,</font></tt><br>
<tt><font size="2">> John</font></tt><br>
<tt><font size="2">> <br>
> _______________________________________________<br>
> OpenStack-dev mailing list<br>
> OpenStack-dev@lists.openstack.org<br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</font></tt><br>
<font size="2" face="sans-serif">Michael<br>
<br>
-------------------------------------------------<br>
Michael Fork<br>
Architect, OpenStack Development<br>
IBM Systems & Technology Group</font><br>
</body></html>