Extend _ApplyContainerMods to accept an attach and detach function. These functions operate in the same way as the add and remove functions, minus the disk creation and disk removal parts.
Also, add two functions that should raise an exception if one attempts to attach/detach a NIC. Finally, update the tests in order to work with the new _ApplyContainerMods function. Signed-off-by: Alex Pyrgiotis <[email protected]> diff --git a/lib/cmdlib/instance_set_params.py b/lib/cmdlib/instance_set_params.py index a4cc576..65e2a6c 100644 --- a/lib/cmdlib/instance_set_params.py +++ b/lib/cmdlib/instance_set_params.py @@ -58,7 +58,7 @@ from ganeti.cmdlib.instance_storage import CalculateFileStorageDir, \ ComputeDiskSizePerVG, ComputeDisksInfo, CreateDisks, \ CreateSingleBlockDev, GenerateDiskTemplate, \ IsExclusiveStorageEnabledNodeUuid, ShutdownInstanceDisks, \ - WaitForSync, WipeOrCleanupDisks + WaitForSync, WipeOrCleanupDisks, AssembleInstanceDisks from ganeti.cmdlib.instance_utils import BuildInstanceHookEnvByObject, \ NICToTuple, CheckNodeNotDrained, CopyLockList, \ ReleaseLocks, CheckNodeVmCapable, CheckTargetNodeIPolicy, \ @@ -783,8 +783,8 @@ class LUInstanceSetParams(LogicalUnit): # Verify disk changes (operating on a copy) inst_disks = self.cfg.GetInstanceDisks(self.instance.uuid) disks = copy.deepcopy(inst_disks) - ApplyContainerMods("disk", disks, None, self.diskmod, None, - _PrepareDiskMod, None) + ApplyContainerMods("disk", disks, None, self.diskmod, None, None, + _PrepareDiskMod, None, None) utils.ValidateDeviceNames("disk", disks) if len(disks) > constants.MAX_DISKS: raise errors.OpPrereqError("Instance has too many disks (%d), cannot add" @@ -1160,6 +1160,10 @@ class LUInstanceSetParams(LogicalUnit): {}, cluster, pnode_uuid) return (None, None) + def _PrepareNicAttach(_, __, ___): + raise errors.OpPrereqError("Attach operation is not supported for NICs", + errors.ECODE_INVAL) + def _PrepareNicMod(_, nic, params, private): self._PrepareNicModification(params, private, nic.ip, nic.network, nic.nicparams, cluster, pnode_uuid) @@ -1171,10 +1175,15 @@ class LUInstanceSetParams(LogicalUnit): if net is not None and ip is not None: self.cfg.ReleaseIp(net, ip, self.proc.GetECId()) + def _PrepareNicDetach(_, __, ___): + raise errors.OpPrereqError("Detach operation is not supported for NICs", + errors.ECODE_INVAL) + # Verify NIC changes (operating on copy) nics = [nic.Copy() for nic in self.instance.nics] - ApplyContainerMods("NIC", nics, None, self.nicmod, - _PrepareNicCreate, _PrepareNicMod, _PrepareNicRemove) + ApplyContainerMods("NIC", nics, None, self.nicmod, _PrepareNicCreate, + _PrepareNicAttach, _PrepareNicMod, _PrepareNicRemove, + _PrepareNicDetach) if len(nics) > constants.MAX_NICS: raise errors.OpPrereqError("Instance has too many network interfaces" " (%d), cannot add more" % constants.MAX_NICS, @@ -1186,8 +1195,8 @@ class LUInstanceSetParams(LogicalUnit): # Operate on copies as this is still in prereq nics = [nic.Copy() for nic in self.instance.nics] ApplyContainerMods("NIC", nics, self._nic_chgdesc, self.nicmod, - self._CreateNewNic, self._ApplyNicMods, - self._RemoveNic) + self._CreateNewNic, None, self._ApplyNicMods, + self._RemoveNic, None) # Verify that NIC names are unique and valid utils.ValidateDeviceNames("NIC", nics) self._new_nics = nics @@ -1588,6 +1597,36 @@ class LUInstanceSetParams(LogicalUnit): if not self.instance.disks_active: ShutdownInstanceDisks(self, self.instance, disks=[disk]) + def _AttachDisk(self, idx, params, _): + """Attaches an existing disk to an instance. + + """ + disk = self.GenericGetDiskInfo(**params) # pylint: disable=W0142 + self.cfg.AttachInstanceDisk(self.instance.uuid, disk, idx) + + # re-read the instance from the configuration + self.instance = self.cfg.GetInstanceInfo(self.instance.uuid) + + changes = [ + ("disk/%d" % idx, + "attach:size=%s,mode=%s" % (disk.size, disk.mode)), + ] + + if self.op.hotplug: + disks_ok, _, results = AssembleInstanceDisks(self, self.instance, + disks=[disk]) + if not disks_ok: + changes.append(("disk/%d" % idx, "assemble:failed")) + return disk, changes + + _, link_name, uri = results[0].payload + msg = self._HotplugDevice(constants.HOTPLUG_ACTION_ADD, + constants.HOTPLUG_TARGET_DISK, + disk, (link_name, uri), idx) + changes.append(("disk/%d" % idx, msg)) + + return (disk, changes) + def _ModifyDisk(self, idx, disk, params, _): """Modifies a disk. @@ -1645,6 +1684,27 @@ class LUInstanceSetParams(LogicalUnit): return hotmsg + def _DetachDisk(self, idx, root, _): + """Detaches a disk from an instance. + + """ + hotmsg = "" + if self.op.hotplug: + hotmsg = self._HotplugDevice(constants.HOTPLUG_ACTION_REMOVE, + constants.HOTPLUG_TARGET_DISK, + root, None, idx) + + # Always shutdown the disk before detaching. + ShutdownInstanceDisks(self, self.instance, [root]) + + # Remove disk from config + self.cfg.DetachInstanceDisk(self.instance.uuid, root.uuid) + + # re-read the instance from the configuration + self.instance = self.cfg.GetInstanceInfo(self.instance.uuid) + + return hotmsg + def _CreateNewNic(self, idx, params, private): """Creates data structure for a new network interface. @@ -1746,8 +1806,9 @@ class LUInstanceSetParams(LogicalUnit): # Apply disk changes inst_disks = self.cfg.GetInstanceDisks(self.instance.uuid) ApplyContainerMods("disk", inst_disks, result, self.diskmod, - self._CreateNewDisk, self._ModifyDisk, - self._RemoveDisk, post_add_fn=self._PostAddDisk) + self._CreateNewDisk, self._AttachDisk, self._ModifyDisk, + self._RemoveDisk, self._DetachDisk, + post_add_fn=self._PostAddDisk) if self.op.disk_template: if __debug__: diff --git a/lib/cmdlib/instance_utils.py b/lib/cmdlib/instance_utils.py index f3b6a40..5a9849c 100644 --- a/lib/cmdlib/instance_utils.py +++ b/lib/cmdlib/instance_utils.py @@ -873,8 +873,8 @@ def PrepareContainerMods(mods, private_fn): def ApplyContainerMods(kind, container, chgdesc, mods, - create_fn, modify_fn, remove_fn, - post_add_fn=None): + create_fn, attach_fn, modify_fn, remove_fn, + detach_fn, post_add_fn=None): """Applies descriptions in C{mods} to C{container}. @type kind: string @@ -890,6 +890,10 @@ def ApplyContainerMods(kind, container, chgdesc, mods, receives absolute item index, parameters and private data object as added by L{PrepareContainerMods}, returns tuple containing new item and changes as list + @type attach_fn: callable + @param attach_fn: Callback for attaching an existing item to a container + (L{constants.DDM_ATTACH}); receives absolute item index and item UUID or + name, returns tuple containing new item and changes as list @type modify_fn: callable @param modify_fn: Callback for modifying an existing item (L{constants.DDM_MODIFY}); receives absolute item index, item, parameters @@ -898,6 +902,9 @@ def ApplyContainerMods(kind, container, chgdesc, mods, @type remove_fn: callable @param remove_fn: Callback on removing item; receives absolute item index, item and private data object as added by L{PrepareContainerMods} + @type detach_fn: callable + @param detach_fn: Callback on detaching item; receives absolute item index, + item and private data object as added by L{PrepareContainerMods} @type post_add_fn: callable @param post_add_fn: Callable for post-processing a newly created item after it has been put into the container. It receives the index of the new item @@ -919,6 +926,18 @@ def ApplyContainerMods(kind, container, chgdesc, mods, if post_add_fn is not None: post_add_fn(addidx, item) + elif op == constants.DDM_ATTACH: + addidx = GetIndexFromIdentifier(identifier, kind, container) + if attach_fn is None: + item = params + else: + (item, changes) = attach_fn(addidx, params, private) + + InsertItemToIndex(int(identifier), item, container) + + if post_add_fn is not None: + post_add_fn(addidx, item) + else: # Retrieve existing item (absidx, item) = GetItemFromContainer(identifier, kind, container) @@ -935,6 +954,18 @@ def ApplyContainerMods(kind, container, chgdesc, mods, assert container[absidx] == item del container[absidx] + elif op == constants.DDM_DETACH: + assert not params + + changes = [("%s/%s" % (kind, absidx), "detach")] + + if detach_fn is not None: + msg = detach_fn(absidx, item, private) + if msg: + changes.append(("%s/%s" % (kind, absidx), msg)) + + assert container[absidx] == item + del container[absidx] elif op == constants.DDM_MODIFY: if modify_fn is not None: changes = modify_fn(absidx, item, params, private) diff --git a/test/py/cmdlib/instance_unittest.py b/test/py/cmdlib/instance_unittest.py index bc44dd4..32c7496 100644 --- a/test/py/cmdlib/instance_unittest.py +++ b/test/py/cmdlib/instance_unittest.py @@ -963,7 +963,7 @@ class TestApplyContainerMods(unittest.TestCase): container = [] chgdesc = [] instance_utils.ApplyContainerMods("test", container, chgdesc, [], None, - None, None) + None, None, None, None) self.assertEqual(container, []) self.assertEqual(chgdesc, []) @@ -976,8 +976,8 @@ class TestApplyContainerMods(unittest.TestCase): (constants.DDM_ADD, 0, "Start"), (constants.DDM_ADD, -1, "End"), ], None) - instance_utils.ApplyContainerMods("test", container, chgdesc, mods, - None, None, None) +instance_utils.ApplyContainerMods("test", container, chgdesc, mods, + None, None, None, None, None) self.assertEqual(container, ["Start", "Hello", "World", "End"]) self.assertEqual(chgdesc, []) @@ -987,8 +987,8 @@ class TestApplyContainerMods(unittest.TestCase): (constants.DDM_ADD, 5, "four"), (constants.DDM_ADD, 7, "xyz"), ], None) - instance_utils.ApplyContainerMods("test", container, chgdesc, mods, - None, None, None) +instance_utils.ApplyContainerMods("test", container, chgdesc, mods, + None, None, None, None, None) self.assertEqual(container, ["zero", "Start", "Hello", "Added", "World", "four", "End", "xyz"]) @@ -999,7 +999,8 @@ class TestApplyContainerMods(unittest.TestCase): (constants.DDM_ADD, idx, "error"), ], None) self.assertRaises(IndexError, instance_utils.ApplyContainerMods, - "test", container, None, mods, None, None, None) + "test", container, None, mods, None, None, None, None, + None) def testRemoveError(self): for idx in [0, 1, 2, 100, -1, -4]: @@ -1007,13 +1008,13 @@ class TestApplyContainerMods(unittest.TestCase): (constants.DDM_REMOVE, idx, None), ], None) self.assertRaises(IndexError, instance_utils.ApplyContainerMods, - "test", [], None, mods, None, None, None) + "test", [], None, mods, None, None, None, None, None) mods = instance_utils.PrepareContainerMods([ (constants.DDM_REMOVE, 0, object()), ], None) self.assertRaises(AssertionError, instance_utils.ApplyContainerMods, - "test", [""], None, mods, None, None, None) + "test", [""], None, mods, None, None, None, None, None) def testAddError(self): for idx in range(-100, -1) + [100]: @@ -1021,7 +1022,7 @@ class TestApplyContainerMods(unittest.TestCase): (constants.DDM_ADD, idx, None), ], None) self.assertRaises(IndexError, instance_utils.ApplyContainerMods, - "test", [], None, mods, None, None, None) + "test", [], None, mods, None, None, None, None, None) def testRemove(self): container = ["item 1", "item 2"] @@ -1031,8 +1032,8 @@ class TestApplyContainerMods(unittest.TestCase): (constants.DDM_ADD, -1, "bbb"), ], None) chgdesc = [] - instance_utils.ApplyContainerMods("test", container, chgdesc, mods, - None, None, None) +instance_utils.ApplyContainerMods("test", container, chgdesc, mods, + None, None, None, None, None) self.assertEqual(container, ["item 1", "item 2", "bbb"]) self.assertEqual(chgdesc, [ ("test/2", "remove"), @@ -1046,8 +1047,8 @@ class TestApplyContainerMods(unittest.TestCase): (constants.DDM_MODIFY, 1, "c"), ], None) chgdesc = [] - instance_utils.ApplyContainerMods("test", container, chgdesc, mods, - None, None, None) +instance_utils.ApplyContainerMods("test", container, chgdesc, mods, + None, None, None, None, None) self.assertEqual(container, ["item 1", "item 2"]) self.assertEqual(chgdesc, []) @@ -1056,7 +1057,8 @@ class TestApplyContainerMods(unittest.TestCase): (constants.DDM_MODIFY, idx, "error"), ], None) self.assertRaises(IndexError, instance_utils.ApplyContainerMods, - "test", container, None, mods, None, None, None) + "test", container, None, mods, None, None, None, None, + None) @staticmethod def _CreateTestFn(idx, params, private): @@ -1090,8 +1092,8 @@ class TestApplyContainerMods(unittest.TestCase): (constants.DDM_ADD, 1, "More"), ], mock.Mock) instance_utils.ApplyContainerMods("test", container, chgdesc, mods, - self._CreateTestFn, self._ModifyTestFn, - self._RemoveTestFn) + self._CreateTestFn, None, self._ModifyTestFn, + self._RemoveTestFn, None) self.assertEqual(container, [ (000, "Start"), (100, "More"), -- 1.7.10.4
