On Thu, Nov 06, 2014 at 12:30:44AM +0200, Alex Pyrgiotis wrote:
> 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
see above.
> + 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
>
LGTM
--
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
