From 048eeb2b8faa15bfb94205f908538ae729d6ca27 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Wed, 15 Feb 2012 13:14:04 +0100
Subject: [PATCH] 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>
---
Makefile.am | 3 +-
lib/storage.py | 21 +++-
test/data/vgreduce-removemissing-2.02.02.txt | 7 ++
.../vgreduce-removemissing-2.02.66-fail.txt | 34 ++++++
.../vgreduce-removemissing-2.02.66-ok.txt | 2 +
test/data/vgs-missing-pvs-2.02.02.txt | 5 +
test/data/vgs-missing-pvs-2.02.66.txt | 2 +
test/ganeti.storage_unittest.py | 113 ++++++++++++++++++
8 files changed, 181 insertions(+), 6 deletions(-)
create mode 100644 test/data/vgreduce-removemissing-2.02.02.txt
create mode 100644 test/data/vgreduce-removemissing-2.02.66-fail.txt
create mode 100644 test/data/vgreduce-removemissing-2.02.66-ok.txt
create mode 100644 test/data/vgs-missing-pvs-2.02.02.txt
create mode 100644 test/data/vgs-missing-pvs-2.02.66.txt
create mode 100755 test/ganeti.storage_unittest.py
diff --git a/Makefile.am b/Makefile.am
index 9e84594ca..29a0db821 100644
--- 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 d382be53c..d77d80bee 100644
--- 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
index 000000000..db29420e2
--- /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
index 000000000..a2ca050b6
--- /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
index 000000000..deb3ce286
--- /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
index 000000000..2946beacb
--- /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
index 000000000..fc7304789
--- /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
index 000000000..add074379
--- /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()
--
GitLab