Unintended keyword arg to utils.execute prevents cinder-volume service from starting

Bug #1175207 reported by Alexander Saint Croix on 2013-05-01
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Cinder
High
John Griffith
Grizzly
High
Avishay Traeger

Bug Description

On Folsom, I was having trouble working with Cinder. Volume creation would show "error" state, existing volumes could not be attached to instances, and attempting to delete existing volumes would show them hung in a "deleting" state. I tracked the problem back to the cinder-volume service, which was failing on startup, yielding the following stack trace:

CRITICAL cinder [-] Got unknown keyword args to utils.execute: {'old_name': None}
TRACE cinder Traceback (most recent call last):
TRACE cinder File "/usr/bin/cinder-volume", line 48, in <module>
TRACE cinder service.wait()
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/service.py", line 422, in wait
TRACE cinder _launcher.wait()
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/service.py", line 127, in wait
TRACE cinder service.wait()
TRACE cinder File "/usr/lib/python2.7/dist-packages/eventlet/greenthread.py", line 166, in wait
TRACE cinder return self._exit_event.wait()
TRACE cinder File "/usr/lib/python2.7/dist-packages/eventlet/event.py", line 116, in wait
TRACE cinder return hubs.get_hub().switch()
TRACE cinder File "/usr/lib/python2.7/dist-packages/eventlet/hubs/hub.py", line 177, in switch
TRACE cinder return self.greenlet.switch()
TRACE cinder File "/usr/lib/python2.7/dist-packages/eventlet/greenthread.py", line 192, in main
TRACE cinder result = function(*args, **kwargs)
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/service.py", line 88, in run_server
TRACE cinder server.start()
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/service.py", line 159, in start
TRACE cinder self.manager.init_host()
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/volume/manager.py", line 101, in init_host
TRACE cinder self.driver.ensure_export(ctxt, volume)
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/volume/driver.py", line 352, in ensure_export
TRACE cinder old_name=old_name )
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/volume/iscsi.py", line 198, in create_iscsi_target
TRACE cinder self._new_target(name, tid, **kwargs)
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/volume/iscsi.py", line 211, in _new_target
TRACE cinder **kwargs)
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/volume/iscsi.py", line 60, in _run
TRACE cinder self._execute(self._cmd, *args, run_as_root=True, **kwargs)
TRACE cinder File "/usr/lib/python2.7/dist-packages/cinder/utils.py", line 146, in execute
TRACE cinder 'to utils.execute: %r') % kwargs)
TRACE cinder Error: Got unknown keyword args to utils.execute: {'old_name': None}

It seems that the fix for https://bugs.launchpad.net/cinder/+bug/1065702 might have caused a side effect that led to the service failing to start.

In cinder/volume/driver.py, the "ISCSIDriver.ensure_export" method passes an "old_name" parameter to the "tgtadm.create_iscsi_target(...)" method. This method does not handle the parameter explicitly, but accepts it as a **kwargs dictionary which is neither accessed nor handled, but merely passed along to driver.py.

So, this "old_name" : None mapping improperly propagates down the call stack until it reaches utils.py, where it triggers an error as an unknown keyword and causes the service to fail.

Since the method where the bug originates seems to indicate the value is not even needed, and since it is never accessed or handled down the call stack, I was able to remove it from the method with no apparent negative consequences, like so:

# NOTE(jdg): For TgtAdm case iscsi_name is the ONLY param we need
# should clean this all up at some point in the future
self.tgtadm.create_iscsi_target(iscsi_name, iscsi_target,
                                0, volume_path,
                                check_exit_code=False,
                                old_name=old_name # REMOVE THIS LINE
                                )

With this parameter removed, I was able to successfully start my cinder-volume service to start normally and immediately begin to resolve its backlog of volume creation and deletion tasks.

John Griffith (john-griffith) wrote :

Hi Alexander,

Yes, I came across this via a post on the mail-list yesterday but hadn't had time to file a bug yet. Thanks for logging this.

Changed in cinder:
status: New → Triaged
importance: Undecided → High
assignee: nobody → John Griffith (john-griffith)
milestone: none → havana-1
no longer affects: cinder/folsom
John Griffith (john-griffith) wrote :

By the way... it is used if you've configured tgtadm, but you're using iet so it's not needed/necessary. The kwarg actually needs to be removed down at the iet call, ideally this wouldn't have just passed in the full kwargs anyway, but that's another story.

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

Changed in cinder:
status: Triaged → In Progress

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

commit db991e6638533fb019fa5dcf59235d7302ae426e
Author: John Griffith <email address hidden>
Date: Wed May 1 11:26:18 2013 -0600

    Remove old_name from kwargs when using IET helper.

    The IET driver passes the input kwargs from export
    directly to the IET driver, but one of the keys here
    is specifically for migration and a bug associated with
    going from nova-vol to cinder-uuid's.

    This patch just checks in the IET code if we have the key set
    and if so pops it out before passing through to iet.

    Fixes bug: 1175207

    Change-Id: I965bdfbe078d61b906aebc48961c1806a9fb0c59

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2013-05-29
Changed in cinder:
status: Fix Committed → Fix Released
tags: added: grizzly-backport-potential

Reviewed: https://review.openstack.org/47721
Committed: http://github.com/openstack/cinder/commit/8aedc3ed1cd6dd59362882baddc222491cf064e7
Submitter: Jenkins
Branch: stable/grizzly

commit 8aedc3ed1cd6dd59362882baddc222491cf064e7
Author: John Griffith <email address hidden>
Date: Wed May 1 11:26:18 2013 -0600

    Remove old_name from kwargs when using IET helper.

    The IET driver passes the input kwargs from export
    directly to the IET driver, but one of the keys here
    is specifically for migration and a bug associated with
    going from nova-vol to cinder-uuid's.

    This patch just checks in the IET code if we have the key set
    and if so pops it out before passing through to iet.

    Cherry picked: db991e6638533fb019fa5dcf59235d7302ae426e
    Fixes bug: 1175207
    Fixes bug: 1212206

    Change-Id: I965bdfbe078d61b906aebc48961c1806a9fb0c59

tags: added: in-stable-grizzly
Thierry Carrez (ttx) on 2013-10-17
Changed in cinder:
milestone: havana-1 → 2013.2
Alan Pevec (apevec) on 2014-03-31
tags: removed: grizzly-backport-potential in-stable-grizzly
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers