On Tue, 12 Jan 2016 at 14:02 'Klaus Aehlig' via ganeti-devel <
[email protected]> wrote:
> The upgrade of a Ganeti cluster is done in several
> high-level steps ("Draining queue", "Pausing the watcher",
> "Stopping daemons", ...). Log those headings as well in
> order to simplify reading the log file; with these headings,
> it is more easy to understand which goal is aimed for with
> all the micro-step RunCmd log entries.
>
> Signed-off-by: Klaus Aehlig <[email protected]>
> ---
> lib/client/gnt_cluster.py | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
> index bffc163..59fdc28 100644
> --- a/lib/client/gnt_cluster.py
> +++ b/lib/client/gnt_cluster.py
> @@ -41,6 +41,7 @@ import time
> import OpenSSL
> import tempfile
> import itertools
> +import logging
>
> from ganeti.cli import *
> from ganeti import bootstrap
> @@ -2016,6 +2017,7 @@ def _UpgradeBeforeConfigurationChange(versionstring):
> lambda: utils.RunCmd(["rm", "-f", pathutils.INTENT_TO_UPGRADE]))
>
> ToStdout("Draining queue")
> + logging.info("Draining queue")
> client = GetClient()
> client.SetQueueDrainFlag(True)
>
> @@ -2028,10 +2030,12 @@ def
> _UpgradeBeforeConfigurationChange(versionstring):
> return (False, rollback)
>
> ToStdout("Pausing the watcher for one hour.")
> + logging.info("Pausing the watcher for one hour.")
> rollback.append(lambda: GetClient().SetWatcherPause(None))
> GetClient().SetWatcherPause(time.time() + 60 * 60)
>
> ToStdout("Stopping daemons on master node.")
> + logging.info("Stopping daemons on master node.")
> if not _RunCommandAndReport([pathutils.DAEMON_UTIL, "stop-all"]):
> return (False, rollback)
>
> @@ -2040,6 +2044,7 @@ def _UpgradeBeforeConfigurationChange(versionstring):
> return (False, rollback)
>
> ToStdout("Stopping daemons everywhere.")
> + logging.info("Stopping daemons everywhere.")
> rollback.append(lambda: _VerifyCommand([pathutils.DAEMON_UTIL,
> "start-all"]))
> badnodes = _VerifyCommand([pathutils.DAEMON_UTIL, "stop-all"])
> if badnodes:
> @@ -2048,6 +2053,7 @@ def _UpgradeBeforeConfigurationChange(versionstring):
>
> backuptar = os.path.join(pathutils.BACKUP_DIR, "ganeti%d.tar" %
> time.time())
> ToStdout("Backing up configuration as %s" % backuptar)
> + logging.info("Backing up configuration as %s", backuptar)
> if not _RunCommandAndReport(["mkdir", "-p", pathutils.BACKUP_DIR]):
> return (False, rollback)
>
> @@ -2076,6 +2082,7 @@ def _VersionSpecificDowngrade():
> @return: True upon success
> """
> ToStdout("Performing version-specific downgrade tasks.")
> + logging.info("Performing version-specific downgrade tasks.")
>
> nodes = ssconf.SimpleStore().GetOnlineNodeList()
> cluster_name = ssconf.SimpleStore().GetClusterName()
> @@ -2128,6 +2135,7 @@ def _SwitchVersionAndConfig(versionstring,
> downgrade):
> rollback = []
> if downgrade:
> ToStdout("Downgrading configuration")
> + logging.info("Downgrading configuration")
> if not _RunCommandAndReport([pathutils.CFGUPGRADE, "--downgrade",
> "-f"]):
> return (False, rollback)
> # Note: version specific downgrades need to be done before switching
> @@ -2140,6 +2148,7 @@ def _SwitchVersionAndConfig(versionstring,
> downgrade):
> # safer to push through the up/dowgrade than to try to roll it back.
>
> ToStdout("Switching to version %s on all nodes" % versionstring)
> + logging.info("Switching to version %s on all nodes", versionstring)
> rollback.append(lambda: _SetGanetiVersion(constants.DIR_VERSION))
> badnodes = _SetGanetiVersion(versionstring)
> if badnodes:
> @@ -2155,6 +2164,7 @@ def _SwitchVersionAndConfig(versionstring,
> downgrade):
>
> if not downgrade:
> ToStdout("Upgrading configuration")
> + logging.info("Upgrading configuration")
> if not _RunCommandAndReport([pathutils.CFGUPGRADE, "-f"]):
> return (False, rollback)
>
> @@ -2180,6 +2190,7 @@ def _UpgradeAfterConfigurationChange(oldversion):
> returnvalue = 0
>
> ToStdout("Ensuring directories everywhere.")
> + logging.info("Ensuring directories everywhere.")
> badnodes = _VerifyCommand([pathutils.ENSURE_DIRS])
> if badnodes:
> ToStderr("Warning: failed to ensure directories on %s." %
> @@ -2187,16 +2198,19 @@ def _UpgradeAfterConfigurationChange(oldversion):
> returnvalue = 1
>
> ToStdout("Starting daemons everywhere.")
> + logging.info("Starting daemons everywhere.")
> badnodes = _VerifyCommand([pathutils.DAEMON_UTIL, "start-all"])
> if badnodes:
> ToStderr("Warning: failed to start daemons on %s." % (",
> ".join(badnodes),))
> returnvalue = 1
>
> ToStdout("Redistributing the configuration.")
> + logging.info("Redistributing the configuration.")
> if not _RunCommandAndReport(["gnt-cluster", "redist-conf",
> "--yes-do-it"]):
> returnvalue = 1
>
> ToStdout("Restarting daemons everywhere.")
> + logging.info("Restarting daemons everywhere.")
> badnodes = _VerifyCommand([pathutils.DAEMON_UTIL, "stop-all"])
> badnodes.extend(_VerifyCommand([pathutils.DAEMON_UTIL, "start-all"]))
> if badnodes:
> @@ -2205,20 +2219,24 @@ def _UpgradeAfterConfigurationChange(oldversion):
> returnvalue = 1
>
> ToStdout("Undraining the queue.")
> + logging.info("Undraining the queue.")
> if not _RunCommandAndReport(["gnt-cluster", "queue", "undrain"]):
> returnvalue = 1
>
> _RunCommandAndReport(["rm", "-f", pathutils.INTENT_TO_UPGRADE])
>
> ToStdout("Running post-upgrade hooks")
> + logging.info("Running post-upgrade hooks")
> if not _RunCommandAndReport([pathutils.POST_UPGRADE, oldversion]):
> returnvalue = 1
>
> ToStdout("Unpausing the watcher.")
> + logging.info("Unpausing the watcher.")
> if not _RunCommandAndReport(["gnt-cluster", "watcher", "continue"]):
> returnvalue = 1
>
> ToStdout("Verifying cluster.")
> + logging.info("Verifying cluster.")
> if not _RunCommandAndReport(["gnt-cluster", "verify"]):
> returnvalue = 1
>
> --
> 2.6.0.rc2.230.g3dd15c0
>
>
Simply repeating everything looks a bit ugly to me. Consider putting the
log message in a variable and use that, and consider making a helper
function that takes the message and calls both ToStdout and logging.info.
But well, your call.
LGTM, thanks
--
Helga Velroyen
Software Engineer
[email protected]
Google Germany GmbH
Erika-Mann-Strasse 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.
This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.
