LGTM. Thanks, Jose
On May 22 14:56, 'Helga Velroyen' via ganeti-devel wrote: > This 'main' function was too big anyway. > > Signed-off-by: Helga Velroyen <[email protected]> > --- > tools/cfgupgrade | 136 > +++++++++++++++++++++++++++++++------------------------ > 1 file changed, 78 insertions(+), 58 deletions(-) > > diff --git a/tools/cfgupgrade b/tools/cfgupgrade > index 5025448..7d64cd6 100755 > --- a/tools/cfgupgrade > +++ b/tools/cfgupgrade > @@ -544,13 +544,7 @@ def DowngradeAll(config_data): > DowngradeInstances(config_data) > > > -def main(): > - """Main program. > - > - """ > - global options, args # pylint: disable=W0603 > - > - # Option parsing > +def _ParseOptions(): > parser = optparse.OptionParser(usage="%prog [--debug|--verbose] [--force]") > parser.add_option("--dry-run", dest="dry_run", > action="store_true", > @@ -575,8 +569,10 @@ def main(): > parser.add_option("--downgrade", > help="Downgrade to the previous stable version", > action="store_true", dest="downgrade", default=False) > - (options, args) = parser.parse_args() > + return parser.parse_args() > + > > +def _ComposePaths(): > # We need to keep filenames locally because they might be renamed between > # versions. > options.data_dir = os.path.abspath(options.data_dir) > @@ -595,19 +591,8 @@ def main(): > options.WATCHER_STATEFILE = options.data_dir + "/watcher.data" > options.FILE_STORAGE_PATHS_FILE = options.conf_dir + "/file-storage-paths" > > - SetupLogging() > - > - # Option checking > - if args: > - raise Error("No arguments expected") > - if options.downgrade and not options.no_verify: > - options.no_verify = True > - > - # Check master name > - if not (CheckHostname(options.SSCONF_MASTER_NODE) or > options.ignore_hostname): > - logging.error("Aborting due to hostname mismatch") > - sys.exit(constants.EXIT_FAILURE) > > +def _AskUser(): > if not options.force: > if options.downgrade: > usertext = ("The configuration is going to be DOWNGRADED to version > %s.%s" > @@ -625,6 +610,76 @@ def main(): > if not cli.AskUser(usertext): > sys.exit(constants.EXIT_FAILURE) > > + > +def _Downgrade(config_major, config_minor, config_version, config_data, > + config_revision): > + if not ((config_major == TARGET_MAJOR and config_minor == TARGET_MINOR) or > + (config_major == DOWNGRADE_MAJOR and > + config_minor == DOWNGRADE_MINOR)): > + raise Error("Downgrade supported only from the latest version (%s.%s)," > + " found %s (%s.%s.%s) instead" % > + (TARGET_MAJOR, TARGET_MINOR, config_version, config_major, > + config_minor, config_revision)) > + DowngradeAll(config_data) > + > + > +def _TestLoadingConfigFile(): > + # test loading the config file > + all_ok = True > + if not (options.dry_run or options.no_verify): > + logging.info("Testing the new config file...") > + cfg = config.ConfigWriter(cfg_file=options.CONFIG_DATA_PATH, > + accept_foreign=options.ignore_hostname, > + offline=True) > + # if we reached this, it's all fine > + vrfy = cfg.VerifyConfig() > + if vrfy: > + logging.error("Errors after conversion:") > + for item in vrfy: > + logging.error(" - %s", item) > + all_ok = False > + else: > + logging.info("File loaded successfully after upgrading") > + del cfg > + > + if options.downgrade: > + action = "downgraded" > + out_ver = "%s.%s" % (DOWNGRADE_MAJOR, DOWNGRADE_MINOR) > + else: > + action = "upgraded" > + out_ver = constants.RELEASE_VERSION > + if all_ok: > + cli.ToStderr("Configuration successfully %s to version %s.", > + action, out_ver) > + else: > + cli.ToStderr("Configuration %s to version %s, but there are errors." > + "\nPlease review the file.", action, out_ver) > + > + > +def main(): > + """Main program. > + > + """ > + global options, args # pylint: disable=W0603 > + > + (options, args) = _ParseOptions() > + _ComposePaths() > + > + SetupLogging() > + > + # Option checking > + if args: > + raise Error("No arguments expected") > + if options.downgrade and not options.no_verify: > + options.no_verify = True > + > + # Check master name > + if not (CheckHostname(options.SSCONF_MASTER_NODE) or > options.ignore_hostname): > + logging.error("Aborting due to hostname mismatch") > + sys.exit(constants.EXIT_FAILURE) > + > + _AskUser() > + > # Check whether it's a Ganeti configuration directory > if not (os.path.isfile(options.CONFIG_DATA_PATH) and > os.path.isfile(options.SERVER_PEM_PATH) and > @@ -654,14 +709,8 @@ def main(): > > # Downgrade to the previous stable version > if options.downgrade: > - if not ((config_major == TARGET_MAJOR and config_minor == TARGET_MINOR) > or > - (config_major == DOWNGRADE_MAJOR and > - config_minor == DOWNGRADE_MINOR)): > - raise Error("Downgrade supported only from the latest version (%s.%s)," > - " found %s (%s.%s.%s) instead" % > - (TARGET_MAJOR, TARGET_MINOR, config_version, config_major, > - config_minor, config_revision)) > - DowngradeAll(config_data) > + _Downgrade(config_major, config_minor, config_version, config_data, > + config_revision) > > # Upgrade from 2.{0..10} to 2.12 > elif config_major == 2 and config_minor in range(0, 12): > @@ -699,36 +748,7 @@ def main(): > " inconsistent state and needs manual intervention.") > raise > > - # test loading the config file > - all_ok = True > - if not (options.dry_run or options.no_verify): > - logging.info("Testing the new config file...") > - cfg = config.ConfigWriter(cfg_file=options.CONFIG_DATA_PATH, > - accept_foreign=options.ignore_hostname, > - offline=True) > - # if we reached this, it's all fine > - vrfy = cfg.VerifyConfig() > - if vrfy: > - logging.error("Errors after conversion:") > - for item in vrfy: > - logging.error(" - %s", item) > - all_ok = False > - else: > - logging.info("File loaded successfully after upgrading") > - del cfg > - > - if options.downgrade: > - action = "downgraded" > - out_ver = "%s.%s" % (DOWNGRADE_MAJOR, DOWNGRADE_MINOR) > - else: > - action = "upgraded" > - out_ver = constants.RELEASE_VERSION > - if all_ok: > - cli.ToStderr("Configuration successfully %s to version %s.", > - action, out_ver) > - else: > - cli.ToStderr("Configuration %s to version %s, but there are errors." > - "\nPlease review the file.", action, out_ver) > + _TestLoadingConfigFile() > > > if __name__ == "__main__": > -- > 1.9.1.423.g4596e3a > -- 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
