>From the modified log line it looks like it was intentional, but as we discussed I don't see a good reason for this. Reinstalling OS image without changing the configuration creates an inconsistency between the state of the world and state of record and we want to _always_ minimise this. Also is it happening only on master ? Other than that LGTM.
On Mon, Nov 28, 2016 at 1:58 PM, 'Federico Morg Pareschi' via ganeti-devel < [email protected]> wrote: > As per issue #1193, instance reinstalls with modified OS parameters > did not persist among reinstalls and were not saved to the cluster's > config data. With this patch, this problem should be fixed. > > Signed-off-by: Federico Morg Pareschi <[email protected]> > --- > lib/cmdlib/instance_operation.py | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/lib/cmdlib/instance_operation.py > b/lib/cmdlib/instance_operation.py > index 048b1e4..9a14ad0 100644 > --- a/lib/cmdlib/instance_operation.py > +++ b/lib/cmdlib/instance_operation.py > @@ -429,8 +429,7 @@ class LUInstanceReinstall(LogicalUnit): > os_image = objects.GetOSImage(self.op.osparams) > > if os_image is not None: > - feedback_fn("Using OS image '%s', not changing instance" > - " configuration" % os_image) > + feedback_fn("Using OS image '%s'" % os_image) > else: > os_image = objects.GetOSImage(self.instance.osparams) > > @@ -447,6 +446,11 @@ class LUInstanceReinstall(LogicalUnit): > self.LogInfo("No OS scripts or OS image specified or found in the" > " instance's configuration, nothing to install") > else: > + if self.op.osparams is not None: > + self.instance.osparams = self.op.osparams > + if self.op.osparams_private is not None: > + self.instance.osparams_private = self.op.osparams_private > + self.cfg.Update(self.instance, feedback_fn) > StartInstanceDisks(self, self.instance, None) > self.instance = self.cfg.GetInstanceInfo(self.instance.uuid) > try: > -- > 2.8.0.rc3.226.g39d4020 > >
