Support UUIDs and names when refering to a device

Modify _ApplyContainerMods function to lookup NICs/Disks not only by their
index inside the container, but also by their UUID or name. Abstract the
lookup code in new GetItemFromContainer function.

Make type of identifier in "opcode._TestInstSetParamsModList" to be
TInt or TString.

Modify gnt-instance client to not check if identifier is an integer.

Signed-off-by: Christos Stavrakakis <cstavr@grnet.gr>
Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Helga Velroyen <helgav@google.com>

diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index ca56783..460041d 100644 (file)
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -1216,23 +1216,17 @@ def _ConvertNicDiskModifications(mods):
   """
   result = []
 
-  for (idx, params) in mods:
-    if idx == constants.DDM_ADD:
+  for (identifier, params) in mods:
+    if identifier == constants.DDM_ADD:
       # Add item as last item (legacy interface)
       action = constants.DDM_ADD
-      idxno = -1
-    elif idx == constants.DDM_REMOVE:
+      identifier = -1
+    elif identifier == constants.DDM_REMOVE:
       # Remove last item (legacy interface)
       action = constants.DDM_REMOVE
-      idxno = -1
+      identifier = -1
     else:
       # Modifications and adding/removing at arbitrary indices
-      try:
-        idxno = int(idx)
-      except (TypeError, ValueError):
-        raise errors.OpPrereqError("Non-numeric index '%s'" % idx,
-                                   errors.ECODE_INVAL)
-
       add = params.pop(constants.DDM_ADD, _MISSING)
       remove = params.pop(constants.DDM_REMOVE, _MISSING)
       modify = params.pop(constants.DDM_MODIFY, _MISSING)
@@ -1260,7 +1254,7 @@ def _ConvertNicDiskModifications(mods):
       raise errors.OpPrereqError("Not accepting parameters on removal",
                                  errors.ECODE_INVAL)
 
-    result.append((action, idxno, params))
+    result.append((action, identifier, params))
 
   return result
 

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 6cb911c..a4a5779 100644 (file)
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -13123,6 +13123,42 @@ def PrepareContainerMods(mods, private_fn):
   return [(op, idx, params, fn()) for (op, idx, params) in mods]
 
 
+def GetItemFromContainer(identifier, kind, container):
+  """Return the item refered by the identifier.
+
+  @type identifier: string
+  @param identifier: Item index or name or UUID
+  @type kind: string
+  @param kind: One-word item description
+  @type container: list
+  @param container: Container to get the item from
+
+  """
+  # Index
+  try:
+    idx = int(identifier)
+    if idx == -1:
+      # Append
+      absidx = len(container) - 1
+    elif idx < 0:
+      raise IndexError("Not accepting negative indices other than -1")
+    elif idx > len(container):
+      raise IndexError("Got %s index %s, but there are only %s" %
+                       (kind, idx, len(container)))
+    else:
+      absidx = idx
+    return (absidx, container[idx])
+  except ValueError:
+    pass
+
+  for idx, item in enumerate(container):
+    if item.uuid == identifier or item.name == identifier:
+      return (idx, item)
+
+  raise errors.OpPrereqError("Cannot find %s with identifier %s" %
+                             (kind, identifier), errors.ECODE_NOENT)
+
+
 #: Type description for changes as returned by L{ApplyContainerMods}'s
 #: callbacks
 _TApplyContModsCbChanges = \
@@ -13159,25 +13195,26 @@ def ApplyContainerMods(kind, container, chgdesc, mods,
     item and private data object as added by L{PrepareContainerMods}
 
   """
-  for (op, idx, params, private) in mods:
-    if idx == -1:
-      # Append
-      absidx = len(container) - 1
-    elif idx < 0:
-      raise IndexError("Not accepting negative indices other than -1")
-    elif idx > len(container):
-      raise IndexError("Got %s index %s, but there are only %s" %
-                       (kind, idx, len(container)))
-    else:
-      absidx = idx
-
+  for (op, identifier, params, private) in mods:
     changes = None
 
     if op == constants.DDM_ADD:
       # Calculate where item will be added
+      # When adding an item, identifier can only be an index
+      try:
+        idx = int(identifier)
+      except ValueError:
+        raise errors.OpPrereqError("Only possitive integer or -1 is accepted as"
+                                   " identifier for %s" % constants.DDM_ADD,
+                                   errors.ECODE_INVAL)
       if idx == -1:
         addidx = len(container)
       else:
+        if idx < 0:
+          raise IndexError("Not accepting negative indices other than -1")
+        elif idx > len(container):
+          raise IndexError("Got %s index %s, but there are only %s" %
+                           (kind, idx, len(container)))
         addidx = idx
 
       if create_fn is None:
@@ -13194,10 +13231,7 @@ def ApplyContainerMods(kind, container, chgdesc, mods,
         container.insert(idx, item)
     else:
       # Retrieve existing item
-      try:
-        item = container[absidx]
-      except IndexError:
-        raise IndexError("Invalid %s index %s" % (kind, idx))
+      (absidx, item) = GetItemFromContainer(identifier, kind, container)
 
       if op == constants.DDM_REMOVE:
         assert not params

diff --git a/lib/opcodes.py b/lib/opcodes.py
index 0978ee5..7a48115 100644 (file)
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -1645,7 +1645,8 @@ def _TestInstSetParamsModList(fn):
   mod_item_fn = \
     ht.TAnd(ht.TIsLength(3), ht.TItems([
       ht.TElemOf(constants.DDMS_VALUES_WITH_MODIFY),
-      ht.Comment("Disk index, can be negative, e.g. -1 for last disk")(ht.TInt),
+      ht.Comment("Device index, can be negative, e.g. -1 for last disk")
+                 (ht.TOr(ht.TInt, ht.TString)),
       fn,
       ]))
 
@@ -1667,9 +1668,10 @@ class OpInstanceSetParams(OpCode):
     _PForceVariant,
     _PIgnoreIpolicy,
     ("nics", ht.EmptyList, TestNicModifications,
-     "List of NIC changes: each item is of the form ``(op, index, settings)``,"
-     " ``op`` is one of ``%s``, ``%s`` or ``%s``, ``index`` can be either -1"
-     " to refer to the last position, or a zero-based index number; a"
+     "List of NIC changes: each item is of the form"
+     " ``(op, identifier, settings)``, ``op`` is one of ``%s``, ``%s`` or"
+     " ``%s``, ``identifier`` can be a zero-based index number (or -1 to refer"
+     " to the last position), the NIC's UUID of the NIC's name; a"
      " deprecated version of this parameter used the form ``(op, settings)``,"
      " where ``op`` can be ``%s`` to add a new NIC with the specified"
      " settings, ``%s`` to remove the last NIC or a number to modify the"

diff --git a/test/py/ganeti.client.gnt_instance_unittest.py b/test/py/ganeti.client.gnt_instance_unittest.py
index 4e65e99..20d3d86 100755 (executable)
--- a/test/py/ganeti.client.gnt_instance_unittest.py
+++ b/test/py/ganeti.client.gnt_instance_unittest.py
@@ -133,9 +133,6 @@ class TestConvertNicDiskModifications(unittest.TestCase):
       (0, { constants.DDM_REMOVE: True, "param": "value", }),
       ])
     self.assertRaises(errors.OpPrereqError, fn, [
-      ("Hello", {}),
-      ])
-    self.assertRaises(errors.OpPrereqError, fn, [
       (0, {
         constants.DDM_REMOVE: True,
         constants.DDM_ADD: True,
@@ -207,6 +204,38 @@ class TestConvertNicDiskModifications(unittest.TestCase):
         }),
       ])
 
+    # Names and UUIDs
+    self.assertEqual(fn([
+      ('name', {
+        constants.IDISK_MODE: constants.DISK_RDWR,
+        constants.IDISK_NAME: "rename",
+        }),
+      ]), [
+      (constants.DDM_MODIFY, 'name', {
+        constants.IDISK_MODE: constants.DISK_RDWR,
+        constants.IDISK_NAME: "rename",
+        }),
+      ])
+    self.assertEqual(fn([
+      ('024ef14d-4879-400e-8767-d61c051950bf', {
+        constants.DDM_MODIFY: True,
+        constants.IDISK_SIZE: 1024,
+        constants.IDISK_NAME: "name",
+        }),
+      ]), [
+      (constants.DDM_MODIFY, '024ef14d-4879-400e-8767-d61c051950bf', {
+        constants.IDISK_SIZE: 1024,
+        constants.IDISK_NAME: "name",
+        }),
+      ])
+    self.assertEqual(fn([
+      ('name', {
+        constants.DDM_REMOVE: True,
+        }),
+      ]), [
+      (constants.DDM_REMOVE, 'name', {}),
+      ])
+
 
 class TestParseDiskSizes(unittest.TestCase):
   def test(self):