lvremove is prompting for verification and thus doesn't actually remove the LV in stable/icehouse

[John Griffith]

Just noticed this in the c-vol logs of a test run while looking at another issue:

2014-07-02 13:19:34.234 31225 DEBUG cinder.brick.local_dev.lvm [req-65ea7090-95ec-4ee9-806e-1f469fa0f28d 54a86f955e344b0384d73f6829bff751 e805dcef4b6642e498f2e1ddd448545d - - -] Error reported running lvremove: CMD: sudo cinder-rootwrap /etc/cinder/rootwrap.conf lvremove stack-volumes/volume-12dfebb9-1371-42d2-a695-8f32aec35d6e, RESPONSE: Do you really want to remove and DISCARD logical volume volume-12dfebb9-1371-42d2-a695-8f32aec35d6e? [y/n]:
  Logical volume volume-12dfebb9-1371-42d2-a695-8f32aec35d6e not removed

Quick check in logstash this goes back quite a ways.

[Thang Pham (thang-pham)]

That is odd. The code uses the -f option, which should not require confirmation:

cinder/brick/local_dev/lvm.py:
    self._execute(
        'lvremove',
        '--config', LVM_CONFIG,
        '-f',
        '%s/%s' % (self.vg_name, name),
        root_helper=self._root_helper, run_as_root=True)

[Kashyap Chamarthy (kashyapc)]

The affecting log from 'gate-grenade-dsvm test'[1]

  [1] http://logs.openstack.org/90/102490/4/gate/gate-grenade-dsvm/41903e8/logs/old/screen-c-vol.txt.gz#_2014-07-02_13_19_41_174

[John Griffith (john-griffith)]

Turns out that this is not as bad as I first thought. The code actually drops into a retry/force when it hits this and finishes things up appropriately. It is a bit ugly to have all of this in the logs, and rather annoying that we're doubling the lvremove calls every time though. Normally I'd say it didn't warrant a backport if it was just logging, but it's a logic error IMO and creates a performance impact. I think we should just use the -f in the first call so we don't get the prompt.

Backporting the current master code is not a good option because it's churned so much that it would be a very significant change and not really worth the risk IMO.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/104243

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/104243
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=570c90f8fbfb632187c5b95ef19b9ebbfe7d3053
Submitter: Jenkins
Branch: stable/icehouse

commit 570c90f8fbfb632187c5b95ef19b9ebbfe7d3053
Author: John Griffith <email address hidden>
Date: Wed Jul 2 09:49:46 2014 -0600

    Add the -f option back to lvremove

    Part of commit I4703133180567090878ea5047dd29d9f97ad85ab
    add some logic to only use the -f flag on lvremove incase
    of a failure condition during the retry.

    The problem is that the initial call results in a prompt
    pretty much every single time you call lvremove. As such
    that call fails every time and we go in to the retry with
    force which succeeds.

    This seems pretty wasteful, and there's no value that I can
    see in continuing to run a cmd every time that we know is
    going to fail.

    This patch adds a -f back on to the first lvremove to eliminate
    this problem.

    Change-Id: I49a4882382f026cd286d0eda605a91cc7a574745
    Closes-Bug: 1336811

[Sean McGinnis (sean-mcginnis)]

The fix for this appears to have been carried forward from Icehouse: https://review.openstack.org/#/c/104243/2