Workaround changed LVM behaviour

The vgreduce command has changed behaviour from when we initially
wrote the code (2.02.02 versus 2.02.66, 4 years delta):

- if there are LVs which will be impacted, it requires --force
- otherwise refuses to proceed, but it still returns exit code 0

We handle this by looking to see if it returns "Wrote out consistent
volume group" (behaviour unchanged), or if it complains about
"--force"; in the case it didn't complete, we retry the operation.

We improve a bit the checking of "vgs", as it uses to fail silently
and we didn't detect it.

New tests for this function should test, I believe, all the expected
variations; at the least we now have data files with the expected
output.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

diff --git a/Makefile.am b/Makefile.am
index 9e84594..29a0db8 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -683,6 +683,7 @@ python_tests = \
        test/ganeti.runtime_unittest.py \
        test/ganeti.serializer_unittest.py \
        test/ganeti.ssh_unittest.py \
+       test/ganeti.storage_unittest.py \
        test/ganeti.tools.ensure_dirs_unittest.py \
        test/ganeti.uidpool_unittest.py \
        test/ganeti.utils.algo_unittest.py \
@@ -1077,7 +1078,7 @@ check-local: check-dirs
        fi; \
        for file in doc/iallocator.rst doc/hooks.rst; do \
                if test "`sed -ne '4 p' $(top_srcdir)/$$file`" != \
-                       "Documents Ganeti version $$expver"; then \
+                       "Documents Ganeti version $$expver"; then \
                        echo "Incorrect version in $$file, expected $$expver"; \
                        exit 1; \
                fi; \

diff --git a/lib/storage.py b/lib/storage.py
index d382be5..d77d80b 100644 (file)
--- a/lib/storage.py
+++ b/lib/storage.py
@@ -1,7 +1,7 @@
 #
 #
 
-# Copyright (C) 2009, 2011 Google Inc.
+# Copyright (C) 2009, 2011, 2012 Google Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -406,6 +406,7 @@ class LvmVgStorage(_LvmBase):
 
   """
   LIST_COMMAND = "vgs"
+  VGREDUCE_COMMAND = "vgreduce"
 
   # Make sure to update constants.VALID_STORAGE_FIELDS when changing field
   # definitions.
@@ -418,7 +419,7 @@ class LvmVgStorage(_LvmBase):
     (constants.SF_ALLOCATABLE, [], True),
     ]
 
-  def _RemoveMissing(self, name):
+  def _RemoveMissing(self, name, _runcmd_fn=utils.RunCmd):
     """Runs "vgreduce --removemissing" on a volume group.
 
     @type name: string
@@ -428,13 +429,23 @@ class LvmVgStorage(_LvmBase):
     # Ignoring vgreduce exit code. Older versions exit with an error even tough
     # the VG is already consistent. This was fixed in later versions, but we
     # cannot depend on it.
-    result = utils.RunCmd(["vgreduce", "--removemissing", name])
+    result = _runcmd_fn([self.VGREDUCE_COMMAND, "--removemissing", name])
 
     # Keep output in case something went wrong
     vgreduce_output = result.output
 
-    result = utils.RunCmd(["vgs", "--noheadings", "--nosuffix", name])
-    if result.failed:
+    # work around newer LVM version
+    if ("Wrote out consistent volume group" not in vgreduce_output or
+        "vgreduce --removemissing --force" in vgreduce_output):
+      # we need to re-run with --force
+      result = _runcmd_fn([self.VGREDUCE_COMMAND, "--removemissing",
+                           "--force", name])
+      vgreduce_output += "\n" + result.output
+
+    result = _runcmd_fn([self.LIST_COMMAND, "--noheadings",
+                         "--nosuffix", name])
+    # we also need to check the output
+    if result.failed or "Couldn't find device with uuid" in result.output:
       raise errors.StorageError(("Volume group '%s' still not consistent,"
                                  " 'vgreduce' output: %r,"
                                  " 'vgs' output: %r") %

diff --git a/test/data/vgreduce-removemissing-2.02.02.txt b/test/data/vgreduce-removemissing-2.02.02.txt
new file mode 100644 (file)
index 0000000..db29420
--- /dev/null
+++ b/test/data/vgreduce-removemissing-2.02.02.txt
@@ -0,0 +1,7 @@
+  Couldn't find device with uuid 'gg4cmC-4lrT-EN1v-39OA-6S2b-6eEI-wWlJJJ'.
+  Couldn't find all physical volumes for volume group xenvg.
+  Couldn't find device with uuid 'gg4cmC-4lrT-EN1v-39OA-6S2b-6eEI-wWlJJJ'.
+  Couldn't find all physical volumes for volume group xenvg.
+  Couldn't find device with uuid 'gg4cmC-4lrT-EN1v-39OA-6S2b-6eEI-wWlJJJ'.
+  Couldn't find device with uuid 'gg4cmC-4lrT-EN1v-39OA-6S2b-6eEI-wWlJJJ'.
+  Wrote out consistent volume group xenvg

diff --git a/test/data/vgreduce-removemissing-2.02.66-fail.txt b/test/data/vgreduce-removemissing-2.02.66-fail.txt
new file mode 100644 (file)
index 0000000..a2ca050
--- /dev/null
+++ b/test/data/vgreduce-removemissing-2.02.66-fail.txt
@@ -0,0 +1,34 @@
+  Couldn't find device with uuid bHRa26-svpL-ihJX-e0S4-2HNz-wAAi-AlBFtl.
+  WARNING: Partial LV 4ba7abfa-8459-43b6-b00f-c016244980f0.disk0 needs to be repaired or removed. 
+  WARNING: Partial LV e972960d-4e35-46b2-9cda-7029916b28c1.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV e972960d-4e35-46b2-9cda-7029916b28c1.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV 4fa40b51-dd4d-4fd9-aef1-35cc3a0f1f11.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV 4fa40b51-dd4d-4fd9-aef1-35cc3a0f1f11.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV 0a184b34-1270-4f1a-94df-86da2167cfee.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV 0a184b34-1270-4f1a-94df-86da2167cfee.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV 7e49c8a9-9c65-4e76-810e-bd3d7a1d97a9.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV 7e49c8a9-9c65-4e76-810e-bd3d7a1d97a9.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV 290a3fd4-c035-4fbe-9a18-f5a0889bd45d.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV 290a3fd4-c035-4fbe-9a18-f5a0889bd45d.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV c579be32-c041-4f1b-ae3e-c58aac9c2593.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV c579be32-c041-4f1b-ae3e-c58aac9c2593.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV 47524563-3788-4a89-a61f-4274134dea73.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV 47524563-3788-4a89-a61f-4274134dea73.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV ede9f706-a0dc-4202-96f2-1728240bbf05.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV ede9f706-a0dc-4202-96f2-1728240bbf05.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV 731d9f1b-3f2f-4860-85b3-217a36b9c48e.disk1_data needs to be repaired or removed. 
+  WARNING: Partial LV 731d9f1b-3f2f-4860-85b3-217a36b9c48e.disk1_meta needs to be repaired or removed. 
+  WARNING: Partial LV f449ccfd-4e6b-42d6-9a52-838371988ab5.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV f449ccfd-4e6b-42d6-9a52-838371988ab5.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV 69bb4f61-fd0c-4c89-a57f-5285ae99b3bd.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV 9c29c24a-97ed-4fc7-b479-7a3385365a71.disk0 needs to be repaired or removed. 
+  WARNING: Partial LV a919d93e-0f51-4e4d-9018-e25ee7d5b36b.disk0 needs to be repaired or removed. 
+  WARNING: Partial LV d2501e6b-56a4-43b6-8856-471e5d49e892.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV d2501e6b-56a4-43b6-8856-471e5d49e892.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV 31a1f85a-ecc8-40c0-88aa-e694626906a3.disk0 needs to be repaired or removed. 
+  WARNING: Partial LV d124d70a-4776-4e00-bf0d-43511c29c534.disk0_data needs to be repaired or removed. 
+  WARNING: Partial LV d124d70a-4776-4e00-bf0d-43511c29c534.disk0_meta needs to be repaired or removed. 
+  WARNING: Partial LV f73b4499-34ec-4f70-a543-e43152a8644a.disk0 needs to be repaired or removed. 
+  WARNING: There are still partial LVs in VG xenvg.
+  To remove them unconditionally use: vgreduce --removemissing --force.
+  Proceeding to remove empty missing PVs.

diff --git a/test/data/vgreduce-removemissing-2.02.66-ok.txt b/test/data/vgreduce-removemissing-2.02.66-ok.txt
new file mode 100644 (file)
index 0000000..deb3ce2
--- /dev/null
+++ b/test/data/vgreduce-removemissing-2.02.66-ok.txt
@@ -0,0 +1,2 @@
+  Couldn't find device with uuid NzfYON-F7ky-1Szf-aGf1-v8Xa-Bt1W-8V3bou.
+  Wrote out consistent volume group xenvg

diff --git a/test/data/vgs-missing-pvs-2.02.02.txt b/test/data/vgs-missing-pvs-2.02.02.txt
new file mode 100644 (file)
index 0000000..2946bea
--- /dev/null
+++ b/test/data/vgs-missing-pvs-2.02.02.txt
@@ -0,0 +1,5 @@
+  Couldn't find device with uuid 'gg4cmC-4lrT-EN1v-39OA-6S2b-6eEI-wWlJJJ'.
+  Couldn't find all physical volumes for volume group xenvg.
+  Couldn't find device with uuid 'gg4cmC-4lrT-EN1v-39OA-6S2b-6eEI-wWlJJJ'.
+  Couldn't find all physical volumes for volume group xenvg.
+  Volume group xenvg not found

diff --git a/test/data/vgs-missing-pvs-2.02.66.txt b/test/data/vgs-missing-pvs-2.02.66.txt
new file mode 100644 (file)
index 0000000..fc73047
--- /dev/null
+++ b/test/data/vgs-missing-pvs-2.02.66.txt
@@ -0,0 +1,2 @@
+  Couldn't find device with uuid bHRa26-svpL-ihJX-e0S4-2HNz-wAAi-AlBFtl.
+  xenvg   2  52   0 wz-pn- 1.31t 1.07t

diff --git a/test/ganeti.storage_unittest.py b/test/ganeti.storage_unittest.py
new file mode 100755 (executable)
index 0000000..add0743
--- /dev/null
+++ b/test/ganeti.storage_unittest.py
@@ -0,0 +1,113 @@
+#!/usr/bin/python
+#
+
+# Copyright (C) 2012 Google Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+
+"""Script for testing ganeti.storage"""
+
+import re
+import unittest
+import random
+
+from ganeti import constants
+from ganeti import utils
+from ganeti import compat
+from ganeti import errors
+from ganeti import storage
+
+import testutils
+
+
+class TestVGReduce(testutils.GanetiTestCase):
+  VGNAME = "xenvg"
+  LIST_CMD = storage.LvmVgStorage.LIST_COMMAND
+  VGREDUCE_CMD = storage.LvmVgStorage.VGREDUCE_COMMAND
+
+  def _runCmd(self, cmd, **kwargs):
+    if not self.run_history:
+      self.fail("Empty run results")
+    exp_cmd, result = self.run_history.pop(0)
+    self.assertEqual(cmd, exp_cmd)
+    return result
+
+  def testOldVersion(self):
+    lvmvg = storage.LvmVgStorage()
+    stdout = self._ReadTestData("vgreduce-removemissing-2.02.02.txt")
+    vgs_fail = self._ReadTestData("vgs-missing-pvs-2.02.02.txt")
+    self.run_history = [
+      ([self.VGREDUCE_CMD, "--removemissing", self.VGNAME],
+       utils.RunResult(0, None, stdout, "", "", None, None)),
+      ([self.LIST_CMD, "--noheadings", "--nosuffix", self.VGNAME],
+       utils.RunResult(0, None, "", "", "", None, None)),
+      ]
+    lvmvg._RemoveMissing(self.VGNAME, _runcmd_fn=self._runCmd)
+    self.assertEqual(self.run_history, [])
+    for ecode, out in [(1, ""), (0, vgs_fail)]:
+      self.run_history = [
+        ([self.VGREDUCE_CMD, "--removemissing", self.VGNAME],
+         utils.RunResult(0, None, stdout, "", "", None, None)),
+        ([self.LIST_CMD, "--noheadings", "--nosuffix", self.VGNAME],
+         utils.RunResult(ecode, None, out, "", "", None, None)),
+        ]
+      self.assertRaises(errors.StorageError, lvmvg._RemoveMissing, self.VGNAME,
+                        _runcmd_fn=self._runCmd)
+      self.assertEqual(self.run_history, [])
+
+  def testNewVersion(self):
+    lvmvg = storage.LvmVgStorage()
+    stdout1 = self._ReadTestData("vgreduce-removemissing-2.02.66-fail.txt")
+    stdout2 = self._ReadTestData("vgreduce-removemissing-2.02.66-ok.txt")
+    vgs_fail = self._ReadTestData("vgs-missing-pvs-2.02.66.txt")
+    # first: require --fail, check that it's used
+    self.run_history = [
+      ([self.VGREDUCE_CMD, "--removemissing", self.VGNAME],
+       utils.RunResult(0, None, stdout1, "", "", None, None)),
+      ([self.VGREDUCE_CMD, "--removemissing", "--force", self.VGNAME],
+       utils.RunResult(0, None, stdout2, "", "", None, None)),
+      ([self.LIST_CMD, "--noheadings", "--nosuffix", self.VGNAME],
+       utils.RunResult(0, None, "", "", "", None, None)),
+      ]
+    lvmvg._RemoveMissing(self.VGNAME, _runcmd_fn=self._runCmd)
+    self.assertEqual(self.run_history, [])
+    # second: make sure --fail is not used if not needed
+    self.run_history = [
+      ([self.VGREDUCE_CMD, "--removemissing", self.VGNAME],
+       utils.RunResult(0, None, stdout2, "", "", None, None)),
+      ([self.LIST_CMD, "--noheadings", "--nosuffix", self.VGNAME],
+       utils.RunResult(0, None, "", "", "", None, None)),
+      ]
+    lvmvg._RemoveMissing(self.VGNAME, _runcmd_fn=self._runCmd)
+    self.assertEqual(self.run_history, [])
+    # third: make sure we error out if vgs doesn't find the volume
+    for ecode, out in [(1, ""), (0, vgs_fail)]:
+      self.run_history = [
+        ([self.VGREDUCE_CMD, "--removemissing", self.VGNAME],
+         utils.RunResult(0, None, stdout1, "", "", None, None)),
+        ([self.VGREDUCE_CMD, "--removemissing", "--force", self.VGNAME],
+         utils.RunResult(0, None, stdout2, "", "", None, None)),
+        ([self.LIST_CMD, "--noheadings", "--nosuffix", self.VGNAME],
+         utils.RunResult(ecode, None, out, "", "", None, None)),
+        ]
+      self.assertRaises(errors.StorageError, lvmvg._RemoveMissing, self.VGNAME,
+                        _runcmd_fn=self._runCmd)
+      self.assertEqual(self.run_history, [])
+
+
+if __name__ == "__main__":
+  testutils.GanetiTestProgram()