2013/2/12 Michele Tartara <[email protected]>:
> --- a/lib/client/gnt_instance.py
> +++ b/lib/client/gnt_instance.py
> @@ -631,7 +631,9 @@ def _RebootInstance(name, opts):
> return opcodes.OpInstanceReboot(instance_name=name,
> reboot_type=opts.reboot_type,
> ignore_secondaries=opts.ignore_secondaries,
> - shutdown_timeout=opts.shutdown_timeout)
> + shutdown_timeout=opts.shutdown_timeout,
> + reason_text=opts.reason,
> +
> reason_source=constants.INSTANCE_REASON_CLI)
Why are you using two different options instead of a tuple with (source, text)?
> --- a/lib/rapi/rlib2.py
> +++ b/lib/rapi/rlib2.py
> @@ -1040,6 +1040,8 @@ class
> R_2_instances_name_reboot(baserlib.OpcodeResource):
> self.queryargs.get("type", [constants.INSTANCE_REBOOT_HARD])[0],
> "ignore_secondaries":
> bool(self._checkIntVariable("ignore_secondaries")),
> "dry_run": self.dryRun(),
> + "reason_text": self._checkStringVariable("reason_text",
> default="Reboot"),
> + "reason_source": constants.INSTANCE_REASON_RAPI
And another case of hardcoded defaults (see below).
> --- a/lib/server/noded.py
> +++ b/lib/server/noded.py
> +def _DefaultAlternative(value, default):
> + return value if value else default
This wouldn't work with Python 2.4. Please don't use inline-if yet.
> class MlockallRequestExecutor(http.server.HttpServerRequestExecutor):
> """Subclass ensuring request handlers are locked in RAM.
>
> @@ -633,7 +645,11 @@ class NodeRequestHandler(http.server.HttpServerHandler):
> instance = objects.Instance.FromDict(params[0])
> reboot_type = params[1]
> shutdown_timeout = params[2]
> - return backend.InstanceReboot(instance, reboot_type, shutdown_timeout)
> + reason_text = _DefaultAlternative(params[3], "Reboot")
Please don't hardcode this inline, but make it a module-level constant.
> + reason_source = params[4]
> + reason = backend.InstReason(reason_text, reason_source)
> + return backend.InstanceReboot(instance, reboot_type, shutdown_timeout,
> + reason)
> --- a/test/py/ganeti.rapi.client_unittest.py
> +++ b/test/py/ganeti.rapi.client_unittest.py
> @@ -594,13 +594,15 @@ class GanetiRapiClientTests(testutils.GanetiTestCase):
> def testRebootInstance(self):
> self.rapi.AddResponse("6146")
> job_id = self.client.RebootInstance("i-bar", reboot_type="hard",
> - ignore_secondaries=True,
> dry_run=True)
> + ignore_secondaries=True,
> dry_run=True,
> + reason_text="Updates")
> self.assertEqual(6146, job_id)
> self.assertHandler(rlib2.R_2_instances_name_reboot)
> self.assertItems(["i-bar"])
> self.assertDryRun()
> self.assertQuery("type", ["hard"])
> self.assertQuery("ignore_secondaries", ["1"])
> + self.assertQuery("reason_text", ["Updates"])
You should also test for the case where you don't specify “reason_text” at all.
Michael
