On Thu, Sep 26, 2013 at 07:47:33AM +0200, Thomas Thrainer wrote:
> When creating an instance, gnt-instance waits for instance disks to
> sync. Inconsistently, this was not the case for adding a disk to an
> instance. This patch changes the default behavior to wait for sync when
> adding a disk, but honor the --no-wait-for-sync option which
> `gnt-instance modify` already supports.
>
> Also adapt unit tests to mock the additional RPC call.
>
> Signed-off-by: Thomas Thrainer <[email protected]>
> ---
> lib/cmdlib/instance.py | 19 +++++++++++++++++--
> test/py/cmdlib/instance_unittest.py | 4 ++++
> 2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index 005004b..93d7ebe 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -2136,7 +2136,8 @@ def GetItemFromContainer(identifier, kind, container):
>
>
> def _ApplyContainerMods(kind, container, chgdesc, mods,
> - create_fn, modify_fn, remove_fn):
> + create_fn, modify_fn, remove_fn,
> + post_add_fn=None):
It seems you can implement this feature without using a hook. After
calling the '_ApplyContainerMods', you execute the 'PostAddDisk' code.
The reason I'm trying to avoid the hook is because we are adding a
parameter to the function which is only going to be used once, in this
particular case. It's does not seem general. Moreover, the
'PostAddDisk' does seem to depend on values that are only available
during the execution of '_ApplyContainerMods', that is, it depends on
values that are also available after the execution
'_ApplyContainerMods'.
> """Applies descriptions in C{mods} to C{container}.
>
> @type kind: string
> @@ -2160,6 +2161,10 @@ 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 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
> + and the new item as parameters.
>
> """
> for (op, identifier, params, private) in mods:
> @@ -2196,6 +2201,10 @@ def _ApplyContainerMods(kind, container, chgdesc, mods,
> assert idx <= len(container)
> # list.insert does so before the specified index
> container.insert(idx, item)
> +
> + if post_add_fn is not None:
> + post_add_fn(addidx, item)
> +
> else:
> # Retrieve existing item
> (absidx, item) = GetItemFromContainer(identifier, kind, container)
> @@ -3229,6 +3238,12 @@ class LUInstanceSetParams(LogicalUnit):
> ("disk/%d" % idx, "add:size=%s,mode=%s" % (disk.size, disk.mode)),
> ])
>
> + def _PostAddDisk(self, _, disk):
> + if not WaitForSync(self, self.instance, disks=[disk],
> + oneshot=not self.op.wait_for_sync):
> + raise errors.OpExecError("Failed to sync disks of %s" %
> + self.instance.name)
> +
> @staticmethod
> def _ModifyDisk(idx, disk, params, _):
> """Modifies a disk.
> @@ -3345,7 +3360,7 @@ class LUInstanceSetParams(LogicalUnit):
> # Apply disk changes
> _ApplyContainerMods("disk", self.instance.disks, result, self.diskmod,
> self._CreateNewDisk, self._ModifyDisk,
> - self._RemoveDisk)
> + self._RemoveDisk, post_add_fn=self._PostAddDisk)
> _UpdateIvNames(0, self.instance.disks)
>
> if self.op.disk_template:
> diff --git a/test/py/cmdlib/instance_unittest.py
> b/test/py/cmdlib/instance_unittest.py
> index 89c7972..b1caa62 100644
> --- a/test/py/cmdlib/instance_unittest.py
> +++ b/test/py/cmdlib/instance_unittest.py
> @@ -1780,6 +1780,10 @@ class TestLUInstanceSetParams(CmdlibTestCase):
> self.RpcResultsBuilder() \
> .CreateSuccessfulNodeResult(self.master, True)
>
> + self.rpc.call_blockdev_getmirrorstatus.side_effect = \
> + lambda node, _: self.RpcResultsBuilder() \
> + .CreateSuccessfulNodeResult(node, [])
> +
Can you explain what this does bit ^ does ?
Thanks,
Jose
> def testNoChanges(self):
> op = self.CopyOpCode(self.op)
> self.ExecOpCodeExpectOpPrereqError(op, "No changes submitted")
> --
> 1.8.4
>
--
Jose Antonio Lopes
Ganeti Engineering
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
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
