On Thu, Sep 26, 2013 at 11:43 AM, Jose A. Lopes <[email protected]>wrote:
> 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'. > After _ApplyContainerMods it's not known which disks were actually created. I only want to call PostAddDisk for the new disks (not for changed or existing ones), not for all of them, so that's the main reason for the new callback. > > > """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 ? > Whenever the call_blockdev_getmirrorstatus method on rpc is called, the function specified in side_effect is called instead. And this function (the lambda really) only returns a successful result for the node which was queried. See http://www.voidspace.org.uk/python/mock/index.html for the documentation of Python Mock. > > 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 > -- Thomas Thrainer | Software Engineer | [email protected] | 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
