From cb92e7a14b7ee510dcc26c2a14dddaa0fa8f10e4 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Wed, 23 Nov 2011 13:40:46 +0100
Subject: [PATCH] Separate OpNodeEvacuate.mode from iallocator

Until now the iallocator constants for node evacuation
(IALLOCATOR_NEVAC_*) were also used for the opcode. However, it turned
out this was due to a misunderstanding and is incorrect. This patch adds
new constants (with the same values) and changes the affected places.
Fortunately the RAPI client already used good names, so no changes are
necessary.

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

Signed-off-by: Michael Hanselmann <hansmi@google.com>
---
 lib/client/gnt_node.py              |  6 +++---
 lib/cmdlib.py                       | 23 ++++++++++++++++-------
 lib/constants.py                    | 10 ++++++++++
 lib/opcodes.py                      |  2 +-
 test/ganeti.rapi.client_unittest.py |  8 ++++----
 5 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/lib/client/gnt_node.py b/lib/client/gnt_node.py
index 270aff55e..0fe37c7ad 100644
--- a/lib/client/gnt_node.py
+++ b/lib/client/gnt_node.py
@@ -276,11 +276,11 @@ def EvacuateNode(opts, args):
                                " --secondary-only options can be passed",
                                errors.ECODE_INVAL)
   elif opts.primary_only:
-    mode = constants.IALLOCATOR_NEVAC_PRI
+    mode = constants.NODE_EVAC_PRI
   elif opts.secondary_only:
-    mode = constants.IALLOCATOR_NEVAC_SEC
+    mode = constants.NODE_EVAC_SEC
   else:
-    mode = constants.IALLOCATOR_NEVAC_ALL
+    mode = constants.NODE_EVAC_ALL
 
   # Determine affected instances
   fields = []
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 4297b8a0d..3208ebf52 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -10145,6 +10145,15 @@ class LUNodeEvacuate(NoHooksLU):
   """
   REQ_BGL = False
 
+  _MODE2IALLOCATOR = {
+    constants.NODE_EVAC_PRI: constants.IALLOCATOR_NEVAC_PRI,
+    constants.NODE_EVAC_SEC: constants.IALLOCATOR_NEVAC_SEC,
+    constants.NODE_EVAC_ALL: constants.IALLOCATOR_NEVAC_ALL,
+    }
+  assert frozenset(_MODE2IALLOCATOR.keys()) == constants.NODE_EVAC_MODES
+  assert (frozenset(_MODE2IALLOCATOR.values()) ==
+          constants.IALLOCATOR_NEVAC_MODES)
+
   def CheckArguments(self):
     _CheckIAllocatorOrNode(self, "iallocator", "remote_node")
 
@@ -10159,7 +10168,7 @@ class LUNodeEvacuate(NoHooksLU):
         raise errors.OpPrereqError("Can not use evacuated node as a new"
                                    " secondary node", errors.ECODE_INVAL)
 
-      if self.op.mode != constants.IALLOCATOR_NEVAC_SEC:
+      if self.op.mode != constants.NODE_EVAC_SEC:
         raise errors.OpPrereqError("Without the use of an iallocator only"
                                    " secondary instances can be evacuated",
                                    errors.ECODE_INVAL)
@@ -10185,19 +10194,19 @@ class LUNodeEvacuate(NoHooksLU):
     """Builds list of instances to operate on.
 
     """
-    assert self.op.mode in constants.IALLOCATOR_NEVAC_MODES
+    assert self.op.mode in constants.NODE_EVAC_MODES
 
-    if self.op.mode == constants.IALLOCATOR_NEVAC_PRI:
+    if self.op.mode == constants.NODE_EVAC_PRI:
       # Primary instances only
       inst_fn = _GetNodePrimaryInstances
       assert self.op.remote_node is None, \
         "Evacuating primary instances requires iallocator"
-    elif self.op.mode == constants.IALLOCATOR_NEVAC_SEC:
+    elif self.op.mode == constants.NODE_EVAC_SEC:
       # Secondary instances only
       inst_fn = _GetNodeSecondaryInstances
     else:
       # All instances
-      assert self.op.mode == constants.IALLOCATOR_NEVAC_ALL
+      assert self.op.mode == constants.NODE_EVAC_ALL
       inst_fn = _GetNodeInstances
 
     return inst_fn(self.cfg, self.op.node_name)
@@ -10272,7 +10281,7 @@ class LUNodeEvacuate(NoHooksLU):
     elif self.op.iallocator is not None:
       # TODO: Implement relocation to other group
       ial = IAllocator(self.cfg, self.rpc, constants.IALLOCATOR_MODE_NODE_EVAC,
-                       evac_mode=self.op.mode,
+                       evac_mode=self._MODE2IALLOCATOR[self.op.mode],
                        instances=list(self.instance_names))
 
       ial.Run(self.op.iallocator)
@@ -10286,7 +10295,7 @@ class LUNodeEvacuate(NoHooksLU):
       jobs = _LoadNodeEvacResult(self, ial.result, self.op.early_release, True)
 
     elif self.op.remote_node is not None:
-      assert self.op.mode == constants.IALLOCATOR_NEVAC_SEC
+      assert self.op.mode == constants.NODE_EVAC_SEC
       jobs = [
         [opcodes.OpInstanceReplaceDisks(instance_name=instance_name,
                                         remote_node=self.op.remote_node,
diff --git a/lib/constants.py b/lib/constants.py
index e04e9b3b4..f40178195 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -1116,6 +1116,16 @@ IALLOCATOR_NEVAC_MODES = frozenset([
   IALLOCATOR_NEVAC_ALL,
   ])
 
+# Node evacuation
+NODE_EVAC_PRI = "primary-only"
+NODE_EVAC_SEC = "secondary-only"
+NODE_EVAC_ALL = "all"
+NODE_EVAC_MODES = frozenset([
+  NODE_EVAC_PRI,
+  NODE_EVAC_SEC,
+  NODE_EVAC_ALL,
+  ])
+
 # Job queue
 JOB_QUEUE_VERSION = 1
 JOB_QUEUE_LOCK_FILE = QUEUE_DIR + "/lock"
diff --git a/lib/opcodes.py b/lib/opcodes.py
index ed017cc16..6a922c946 100644
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -1014,7 +1014,7 @@ class OpNodeEvacuate(OpCode):
     _PNodeName,
     ("remote_node", None, ht.TMaybeString, "New secondary node"),
     ("iallocator", None, ht.TMaybeString, "Iallocator for computing solution"),
-    ("mode", ht.NoDefault, ht.TElemOf(constants.IALLOCATOR_NEVAC_MODES),
+    ("mode", ht.NoDefault, ht.TElemOf(constants.NODE_EVAC_MODES),
      "Node evacuation mode"),
     ]
   OP_RESULT = TJobIdListOnly
diff --git a/test/ganeti.rapi.client_unittest.py b/test/ganeti.rapi.client_unittest.py
index 1067656d3..d5ab404b2 100755
--- a/test/ganeti.rapi.client_unittest.py
+++ b/test/ganeti.rapi.client_unittest.py
@@ -166,9 +166,9 @@ class TestConstants(unittest.TestCase):
     self.assertEqual(client.JOB_STATUS_ALL, constants.JOB_STATUS_ALL)
 
     # Node evacuation
-    self.assertEqual(client.NODE_EVAC_PRI, constants.IALLOCATOR_NEVAC_PRI)
-    self.assertEqual(client.NODE_EVAC_SEC, constants.IALLOCATOR_NEVAC_SEC)
-    self.assertEqual(client.NODE_EVAC_ALL, constants.IALLOCATOR_NEVAC_ALL)
+    self.assertEqual(client.NODE_EVAC_PRI, constants.NODE_EVAC_PRI)
+    self.assertEqual(client.NODE_EVAC_SEC, constants.NODE_EVAC_SEC)
+    self.assertEqual(client.NODE_EVAC_ALL, constants.NODE_EVAC_ALL)
 
     # Legacy name
     self.assertEqual(client.JOB_STATUS_WAITLOCK, constants.JOB_STATUS_WAITING)
@@ -866,7 +866,7 @@ class GanetiRapiClientTests(testutils.GanetiTestCase):
     self.rapi.AddResponse(serializer.DumpJson([rlib2._NODE_EVAC_RES1]))
     self.rapi.AddResponse("8888")
     job_id = self.client.EvacuateNode("node-3", iallocator="hail", dry_run=True,
-                                      mode=constants.IALLOCATOR_NEVAC_ALL,
+                                      mode=constants.NODE_EVAC_ALL,
                                       early_release=True)
     self.assertEqual(8888, job_id)
     self.assertItems(["node-3"])
-- 
GitLab