KeyError from storwize driver when 'host' is not provided to terminate_connection

Bug #1244257 reported by Brad Pokorny on 2013-10-24
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Medium
Jay Bryant
Havana
Medium
Avishay Traeger

Bug Description

When user tries to delete an instance, if the nova compute driver for the instance's host is down, the nova api code goes into _local_delete(). The following code creates a fake "connector" without a 'host' key/vale.

        for bdm in bdms:
            if bdm['volume_id']:
                # NOTE(vish): We don't have access to correct volume
                # connector info, so just pass a fake
                # connector. This can be improved when we
                # expose get_volume_connector to rpc.
                connector = {'ip': '127.0.0.1', 'initiator': 'iqn.fake'}
                self.volume_api.terminate_connection(context,
                                                     bdm['volume_id'],
                                                     connector)

When code flow reaches the cinder driver, which expects a 'host', a "KeyError: 'host'" exception is raised, and deletion of the instance fails.

The failure in deletion is as expected since the nova driver is not up. This issue is requesting a modification in nova api code, so that if the nova compute server is not available, a more meaningful error message ("nova compute server is not available") could be returned, before getting to the cinder code.

Matt Riedemann (mriedem) wrote :

The volume_api block in _local_delete is catching an exception but not the KeyError so the instance deletion would fail here since a KeyError is not an Exception.

tags: added: api compute
Matt Riedemann (mriedem) wrote :

Ignore previous comment, was thinking of something else. Looks like this is already handled on master for icehouse, it's not on 2013.2 for havana:

https://github.com/openstack/nova/commit/8e304db5497d2b31d711fc16433b624d5c1d13c6

https://review.openstack.org/#/c/46534/

The related bug is not listed as havana-backport-potential, and I'm not sure if the stable maintainers would accept it, but someone could try.

Changed in nova:
status: New → Triaged
importance: Undecided → Medium
Matt Riedemann (mriedem) wrote :

Would be good if there was a paste of the stack trace here. Does specifying host=None fix the KeyError on the cinder side or will it just fail later because the host is None?

Matt Riedemann (mriedem) wrote :

I've confirmed the problem is down in the cinder storwize driver here:

https://github.com/openstack/cinder/blob/2013.2/cinder/volume/drivers/storwize_svc.py#L476

The code should be doing connector.get('host') so it returns None. I've seen similar problems with the storwize driver not getting host safely and then changes being made in nova to workaround it (thinking with the hyper-v driver).

So I'm going to change this to a cinder bug and fix there.

affects: nova → cinder
Matt Riedemann (mriedem) wrote :

Searching on connector['host'] in cinder also shows that the hp drivers could have the same problem, so it's a bit more pervasive and should be cleaned up in those drivers too.

Changed in cinder:
assignee: nobody → Matt Riedemann (mriedem)
summary: - Need more meaningful message if instance deletion fails because nova
- compute driver is not running
+ KeyError from storwize driver when 'host' is not provided
Matt Riedemann (mriedem) on 2013-10-24
summary: - KeyError from storwize driver when 'host' is not provided
+ KeyError from storwize driver when 'host' is not provided to
+ terminate_connection
Avishay Traeger (avishay-il) wrote :

Thanks for this. I assumed 'host' would always be provided.

tags: added: drivers havana-backport-potential storwize-svc
removed: api compute
tags: added: storwize
removed: storwize-svc
Changed in cinder:
milestone: none → icehouse-1
Matt Riedemann (mriedem) wrote :

Removing myself as the assignee. I was originally going to just handle the KeyError in the storwize driver's terminate_connection method but then I noticed there are some other instances of connector['host'] in the code in other drivers as well and sort of got on the fence about how widespread I wanted to change things, and whether or not nova should always be sending something, even if it's 127.0.0.1, but if I did that would it break anything else....and things quickly spiraled out.

So the tactical fix is to just do connector.get('host') and let the code handle the None value which it already does.

Changed in cinder:
assignee: Matt Riedemann (mriedem) → nobody
Changed in cinder:
assignee: nobody → Avishay Traeger (avishay-il)
Changed in cinder:
milestone: icehouse-1 → icehouse-2

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

Changed in cinder:
status: Triaged → In Progress
Changed in cinder:
assignee: Avishay Traeger (avishay-il) → Jay Bryant (jsbryant)

Reviewed: https://review.openstack.org/58602
Committed: http://github.com/openstack/cinder/commit/8185d1b5db421441ebba128f23f840a42f9bf050
Submitter: Jenkins
Branch: master

commit 8185d1b5db421441ebba128f23f840a42f9bf050
Author: Avishay Traeger <email address hidden>
Date: Tue Nov 26 21:10:22 2013 +0200

    Fix Storwize terminate_connection with no host

    Nova may pass a connector to Cinder with no 'host' field, which was
    causing a KeyError in the Storwize driver. This patch resolves this case
    by doing the following:
    1. If the volume is mapped to only 1 host, unmap it
    2. If the volume was not mapped or mapped to multiple hosts, print a
       warning but don't raise an exception

    Change-Id: I0ec1c24adbdfbcf1c4868b4981a2e2618d4b411c
    Closes-Bug: #1244257

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2013-12-04
Changed in cinder:
milestone: icehouse-2 → icehouse-1
status: Fix Committed → Fix Released

Reviewed: https://review.openstack.org/59634
Committed: http://github.com/openstack/cinder/commit/b73976fddc1e10e84ca6a2f46c160d2d58e56311
Submitter: Jenkins
Branch: stable/havana

commit b73976fddc1e10e84ca6a2f46c160d2d58e56311
Author: Avishay Traeger <email address hidden>
Date: Tue Nov 26 21:10:22 2013 +0200

    Fix Storwize terminate_connection with no host

    Nova may pass a connector to Cinder with no 'host' field, which was
    causing a KeyError in the Storwize driver. This patch resolves this case
    by doing the following:
    1. If the volume is mapped to only 1 host, unmap it
    2. If the volume was not mapped or mapped to multiple hosts, print a
       warning but don't raise an exception

    Change-Id: I0ec1c24adbdfbcf1c4868b4981a2e2618d4b411c
    Closes-Bug: #1244257
    (cherry picked from commit 8185d1b5db421441ebba128f23f840a42f9bf050)

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

Other bug subscribers