From 539d65ba090ea0c10336d40774d59fbf67b07b2d Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Mon, 17 Oct 2011 15:58:19 +0200
Subject: [PATCH] RAPI: Fix resource for replacing disks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit d1c172deb4f inadvertently changes the
β/2/instances/[instance_name]/replace-disksβ resource to use body
parameters. There were no QA tests and the issue wasn't noticed.
This patch re-introduces support for query parameters and adds a QA
test.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Andrea Spadaccini <spadaccio@google.com>
---
doc/rapi.rst | 15 +++++++-------
lib/rapi/client.py | 15 ++++++--------
lib/rapi/rlib2.py | 32 +++++++++++++++++++++--------
qa/ganeti-qa.py | 1 +
qa/qa_rapi.py | 8 ++++++++
test/ganeti.rapi.client_unittest.py | 7 ++-----
test/ganeti.rapi.rlib2_unittest.py | 8 ++++++++
7 files changed, 56 insertions(+), 30 deletions(-)
diff --git a/doc/rapi.rst b/doc/rapi.rst
index 7ed29a803..97a1112ff 100644
--- a/doc/rapi.rst
+++ b/doc/rapi.rst
@@ -789,16 +789,15 @@ It supports the following commands: ``POST``.
``POST``
~~~~~~~~
-Takes the parameters ``mode`` (one of ``replace_on_primary``,
-``replace_on_secondary``, ``replace_new_secondary`` or
-``replace_auto``), ``disks`` (comma separated list of disk indexes),
-``remote_node`` and ``iallocator``.
+Returns a job ID.
+
+Body parameters:
-Either ``remote_node`` or ``iallocator`` needs to be defined when using
-``mode=replace_new_secondary``.
+.. opcode_params:: OP_INSTANCE_REPLACE_DISKS
+ :exclude: instance_name
-``mode`` is a mandatory parameter. ``replace_auto`` tries to determine
-the broken disk(s) on its own and replacing it.
+Ganeti 2.4 and below used query parameters. Those are deprecated and
+should no longer be used.
``/2/instances/[instance_name]/activate-disks``
diff --git a/lib/rapi/client.py b/lib/rapi/client.py
index dac670b1b..be39202c7 100644
--- a/lib/rapi/client.py
+++ b/lib/rapi/client.py
@@ -956,7 +956,7 @@ class GanetiRapiClient(object): # pylint: disable=R0904
(GANETI_RAPI_VERSION, instance)), query, None)
def ReplaceInstanceDisks(self, instance, disks=None, mode=REPLACE_DISK_AUTO,
- remote_node=None, iallocator=None, dry_run=False):
+ remote_node=None, iallocator=None):
"""Replaces disks on an instance.
@type instance: str
@@ -971,8 +971,6 @@ class GanetiRapiClient(object): # pylint: disable=R0904
@type iallocator: str or None
@param iallocator: instance allocator plugin to use (for use with
replace_auto mode)
- @type dry_run: bool
- @param dry_run: whether to perform a dry run
@rtype: string
@return: job id
@@ -982,18 +980,17 @@ class GanetiRapiClient(object): # pylint: disable=R0904
("mode", mode),
]
- if disks:
+ # TODO: Convert to body parameters
+
+ if disks is not None:
query.append(("disks", ",".join(str(idx) for idx in disks)))
- if remote_node:
+ if remote_node is not None:
query.append(("remote_node", remote_node))
- if iallocator:
+ if iallocator is not None:
query.append(("iallocator", iallocator))
- if dry_run:
- query.append(("dry-run", 1))
-
return self._SendRequest(HTTP_POST,
("/%s/instances/%s/replace-disks" %
(GANETI_RAPI_VERSION, instance)), query, None)
diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
index 0a958da11..97ad7cd42 100644
--- a/lib/rapi/rlib2.py
+++ b/lib/rapi/rlib2.py
@@ -999,16 +999,19 @@ def _ParseInstanceReplaceDisksRequest(name, data):
# Parse disks
try:
- raw_disks = data["disks"]
+ raw_disks = data.pop("disks")
except KeyError:
pass
else:
- if not ht.TListOf(ht.TInt)(raw_disks): # pylint: disable=E1102
- # Backwards compatibility for strings of the format "1, 2, 3"
- try:
- data["disks"] = [int(part) for part in raw_disks.split(",")]
- except (TypeError, ValueError), err:
- raise http.HttpBadRequest("Invalid disk index passed: %s" % str(err))
+ if raw_disks:
+ if ht.TListOf(ht.TInt)(raw_disks): # pylint: disable=E1102
+ data["disks"] = raw_disks
+ else:
+ # Backwards compatibility for strings of the format "1, 2, 3"
+ try:
+ data["disks"] = [int(part) for part in raw_disks.split(",")]
+ except (TypeError, ValueError), err:
+ raise http.HttpBadRequest("Invalid disk index passed: %s" % str(err))
return baserlib.FillOpcode(opcodes.OpInstanceReplaceDisks, data, override)
@@ -1021,7 +1024,20 @@ class R_2_instances_name_replace_disks(baserlib.R_Generic):
"""Replaces disks on an instance.
"""
- op = _ParseInstanceReplaceDisksRequest(self.items[0], self.request_body)
+ if self.request_body:
+ body = self.request_body
+ elif self.queryargs:
+ # Legacy interface, do not modify/extend
+ body = {
+ "remote_node": self._checkStringVariable("remote_node", default=None),
+ "mode": self._checkStringVariable("mode", default=None),
+ "disks": self._checkStringVariable("disks", default=None),
+ "iallocator": self._checkStringVariable("iallocator", default=None),
+ }
+ else:
+ body = {}
+
+ op = _ParseInstanceReplaceDisksRequest(self.items[0], body)
return baserlib.SubmitJob([op])
diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py
index 8b0c33889..a2a4eb058 100755
--- a/qa/ganeti-qa.py
+++ b/qa/ganeti-qa.py
@@ -382,6 +382,7 @@ def RunHardwareFailureTests(instance, pnode, snode):
if qa_config.TestEnabled("instance-replace-disks"):
othernode = qa_config.AcquireNode(exclude=[pnode, snode])
try:
+ RunTestIf("rapi", qa_rapi.TestRapiInstanceReplaceDisks, instance)
RunTest(qa_instance.TestReplaceDisks,
instance, pnode, snode, othernode)
finally:
diff --git a/qa/qa_rapi.py b/qa/qa_rapi.py
index 02218463c..a4c5921ce 100644
--- a/qa/qa_rapi.py
+++ b/qa/qa_rapi.py
@@ -618,6 +618,14 @@ def TestRapiInstanceReinstall(instance):
_WaitForRapiJob(_rapi_client.ReinstallInstance(instance["name"]))
+def TestRapiInstanceReplaceDisks(instance):
+ """Test replacing instance disks via RAPI"""
+ _WaitForRapiJob(_rapi_client.ReplaceInstanceDisks(instance["name"],
+ mode=constants.REPLACE_DISK_AUTO, disks=[]))
+ _WaitForRapiJob(_rapi_client.ReplaceInstanceDisks(instance["name"],
+ mode=constants.REPLACE_DISK_SEC, disks="0"))
+
+
def TestRapiInstanceModify(instance):
"""Test modifying instance via RAPI"""
def _ModifyInstance(**kwargs):
diff --git a/test/ganeti.rapi.client_unittest.py b/test/ganeti.rapi.client_unittest.py
index d7af45ad3..d441c1abc 100755
--- a/test/ganeti.rapi.client_unittest.py
+++ b/test/ganeti.rapi.client_unittest.py
@@ -669,24 +669,21 @@ class GanetiRapiClientTests(testutils.GanetiTestCase):
def testReplaceInstanceDisks(self):
self.rapi.AddResponse("999")
job_id = self.client.ReplaceInstanceDisks("instance-name",
- disks=[0, 1], dry_run=True, iallocator="hail")
+ disks=[0, 1], iallocator="hail")
self.assertEqual(999, job_id)
self.assertHandler(rlib2.R_2_instances_name_replace_disks)
self.assertItems(["instance-name"])
self.assertQuery("disks", ["0,1"])
self.assertQuery("mode", ["replace_auto"])
self.assertQuery("iallocator", ["hail"])
- self.assertDryRun()
self.rapi.AddResponse("1000")
job_id = self.client.ReplaceInstanceDisks("instance-bar",
- disks=[1], mode="replace_on_secondary", remote_node="foo-node",
- dry_run=True)
+ disks=[1], mode="replace_on_secondary", remote_node="foo-node")
self.assertEqual(1000, job_id)
self.assertItems(["instance-bar"])
self.assertQuery("disks", ["1"])
self.assertQuery("remote_node", ["foo-node"])
- self.assertDryRun()
self.rapi.AddResponse("5175")
self.assertEqual(5175, self.client.ReplaceInstanceDisks("instance-moo"))
diff --git a/test/ganeti.rapi.rlib2_unittest.py b/test/ganeti.rapi.rlib2_unittest.py
index 6584c38e8..de8274556 100755
--- a/test/ganeti.rapi.rlib2_unittest.py
+++ b/test/ganeti.rapi.rlib2_unittest.py
@@ -529,6 +529,14 @@ class TestParseInstanceReplaceDisksRequest(unittest.TestCase):
self.assertFalse(hasattr(op, "iallocator"))
self.assertFalse(hasattr(op, "disks"))
+ def testNoDisks(self):
+ self.assertRaises(http.HttpBadRequest, self.Parse, "inst20661", {})
+
+ for disks in [None, "", {}]:
+ self.assertRaises(http.HttpBadRequest, self.Parse, "inst20661", {
+ "disks": disks,
+ })
+
def testWrong(self):
self.assertRaises(http.HttpBadRequest, self.Parse, "inst",
{ "mode": constants.REPLACE_DISK_AUTO,
--
GitLab