From e44406f8baa8d92acedfc69c528d6afe41af22e1 Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Sat, 3 Oct 2009 01:51:59 -0400 Subject: [PATCH] Implement locks for upgrade, add d.upgrade() Signed-off-by: Edward Z. Yang --- TODO | 8 +------- wizard/app/mediawiki.py | 2 +- wizard/command/migrate.py | 25 +------------------------ wizard/command/upgrade.py | 37 ++++++++++++++++++------------------- wizard/deploy.py | 8 +++++++- wizard/util.py | 32 ++++++++++++++++++++++++++++++++ 6 files changed, 60 insertions(+), 52 deletions(-) diff --git a/TODO b/TODO index d1b0f52..37c6a00 100644 --- a/TODO +++ b/TODO @@ -15,13 +15,9 @@ TODO NOW: to determine version. Make parallel-find.pl have this have greater precedence. This also means, however, that we get full mediawiki-1.2.3-2-abcdef names (Have patch, pending testing and commit) -- Make the installer use 'wizard install' /or/ do a migration +- Make deployed installer use 'wizard install' /or/ do a migration after doing a normal install (the latter makes it easier for mass-rollbacks). -- Have the upgrader do locking (.scripts/lock, probably) - -- Relax MediaWiki regexes to terminate on semicolon, and not - require its own line. - Better error message if daemon/scripts-security-upd is not on scripts-security-upd list @@ -32,8 +28,6 @@ TODO NOW: do in both directions). All 1.11.0 installs except four have the other (check diff -u with all in /root) -- Make upgrade and install take version as a parameter - - Redo Wordpress conversion, with an eye for automating everything possible (such as downloading the tarball and unpacking) diff --git a/wizard/app/mediawiki.py b/wizard/app/mediawiki.py index bfac9c7..6b39c8f 100644 --- a/wizard/app/mediawiki.py +++ b/wizard/app/mediawiki.py @@ -84,7 +84,7 @@ class Application(deploy.Application): if result.find("Installation successful") == -1: raise install.Failure() os.rename('config/LocalSettings.php', 'LocalSettings.php') - def upgrade(self, version, options): + def upgrade(self, d, version, options): sh = shell.Shell() if not os.path.isfile("AdminSettings.php"): sh.call("git", "checkout", "mediawiki-" + str(version), "--", "AdminSettings.php") diff --git a/wizard/command/migrate.py b/wizard/command/migrate.py index 4e5837d..3949ae8 100644 --- a/wizard/command/migrate.py +++ b/wizard/command/migrate.py @@ -47,16 +47,8 @@ def main(argv, baton): tag = version.scripts_tag # XXX: turn this into a context - try: + with util.LockDirectory(".scripts-migrate-lock"): try: - try: - os.open(".scripts-migrate-lock", os.O_CREAT | os.O_EXCL) - except OSError as e: - if e.errno == errno.EEXIST: - raise DirectoryLockedError - elif e.errno == errno.EACCES: - raise command.PermissionsError(dir) - raise if options.force: perform_force(options) make_repository(sh, options, repo, tag) check_variables(deployment, options) @@ -67,11 +59,6 @@ def main(argv, baton): shutil.rmtree(".scripts") if os.path.exists(".git"): shutil.rmtree(".git") - finally: - try: - os.unlink(".scripts-migrate-lock") - except OSError: - pass def parse_args(argv, baton): usage = """usage: %prog migrate [ARGS] DIR @@ -174,13 +161,3 @@ ERROR: Directory already contains a .git and both of these directories will be removed. """ -class DirectoryLockedError(Error): - def __init__(self, dir): - self.dir = dir - def __str__(self): - return """ - -ERROR: Could not acquire lock on directory. Maybe there is -another migration process running? -""" - diff --git a/wizard/command/upgrade.py b/wizard/command/upgrade.py index 2350592..051ee22 100644 --- a/wizard/command/upgrade.py +++ b/wizard/command/upgrade.py @@ -78,28 +78,27 @@ def main(argv, baton): # perform database backup backup = d.backup(options) # XXX: frob .htaccess to make site inaccessible - # XXX: need locking - # git merge (which performs a fast forward) - # - merge could fail (race); that's /really/ dangerous. with util.IgnoreKeyboardInterrupts(): - sh.call("git", "pull", temp_wc_dir, "master") - version_obj = distutils.version.LooseVersion(version.partition('-')[2]) - try: - # run update script - d.application.upgrade(version_obj, options) - d.verifyWeb() - except app.UpgradeFailure: - logging.warning("Upgrade failed: rolling back") - perform_restore(d, backup) - raise - except deploy.WebVerificationError: - logging.warning("Web verification failed: rolling back") - perform_restore(d, backup) - raise app.UpgradeFailure("Upgrade caused website to become inaccessible; site was rolled back") + with util.LockDirectory(".scripts-upgrade-lock"): + # git merge (which performs a fast forward) + sh.call("git", "pull", temp_wc_dir, "master") + version_obj = distutils.version.LooseVersion(version.partition('-')[2]) + try: + # run update script + d.upgrade(version_obj, options) + d.verifyWeb() + except app.UpgradeFailure: + logging.warning("Upgrade failed: rolling back") + perform_restore(d, backup) + raise + except deploy.WebVerificationError: + logging.warning("Web verification failed: rolling back") + perform_restore(d, backup) + raise app.UpgradeFailure("Upgrade caused website to become inaccessible; site was rolled back") # XXX: frob .htaccess to make site accessible - # XXX: - check if .htaccess changed, first. Upgrade + # to do this, check if .htaccess changed, first. Upgrade # process might have frobbed it. Don't be - # particularly worried if the segment dissappeared + # particularly worried if the segment disappeared def perform_restore(d, backup): # You don't want d.restore() because it doesn't perform diff --git a/wizard/deploy.py b/wizard/deploy.py index 0676454..f2b3729 100644 --- a/wizard/deploy.py +++ b/wizard/deploy.py @@ -105,6 +105,12 @@ class Deployment(object): as such dir will generally not equal :attr:`location`. """ return self.application.parametrize(self, dir) + def upgrade(self, version, options): + """ + Performs an upgrae of database schemas and other non-versioned data. + """ + with util.ChangeDirectory(self.location): + return self.application.upgrade(self, version, options) def backup(self, options): """ Performs a backup of database schemas and other non-versioned data. @@ -383,7 +389,7 @@ class Application(object): working directory is a deployment. """ raise NotImplemented - def upgrade(self, version, options): + def upgrade(self, deployment, version, options): """ Run for 'wizard upgrade' to upgrade database schemas and other non-versioned data in an application. This assumes that diff --git a/wizard/util.py b/wizard/util.py index 621dea1..2d9c72f 100644 --- a/wizard/util.py +++ b/wizard/util.py @@ -83,6 +83,27 @@ class IgnoreKeyboardInterrupts(object): def __exit__(self, *args): signal.signal(signal.SIGINT, signal.default_int_handler) +class LockDirectory(object): + """ + Context for locking a directory. + """ + def __init__(self, lockfile): + self.lockfile = lockfile + def __enter__(self): + try: + os.open(self.lockfile, os.O_CREAT | os.O_EXCL) + except OSError as e: + if e.errno == errno.EEXIST: + raise DirectoryLockedError(os.getcwd()) + elif e.errno == errno.EACCES: + raise command.PermissionsError(os.getcwd()) + raise + def __exit__(self, *args): + try: + os.unlink(self.lockfile) + except OSError: + pass + def chdir(dir): """ Changes a directory, but has special exceptions for certain @@ -293,3 +314,14 @@ class PermissionsError(IOError): class NoSuchDirectoryError(IOError): errno = errno.ENOENT + +class DirectoryLockedError(wizard.Error): + def __init__(self, dir): + self.dir = dir + def __str__(self): + return """ + +ERROR: Could not acquire lock on directory. Maybe there is +another migration process running? +""" + -- 2.45.0