Upgrades, remove saved version file on success
* created on_success method which upgrade script runs if upgrade succeed, don't fail upgrade in case of errors * remove saved version files for all upgrades from working directories It solves several problems: 1. user runs upgrade 5.0 -> 5.1 which fails upgrade system saves version which we upgrade from in file working_dir/5.1/version.yaml. Then user runs upgrade 5.0 -> 5.0.1 which successfully upgraded. Then user runs again upgrade 5.0.1 -> 5.1, but there is saved file working_dir/5.1/version.yaml which contains 5.0 version, and upgrade system thinks that it's upgrading from 5.0 version, as result it tries to make database dump from wrong version of container. 2. without this hack user can run upgrade second time and loose his data, this hack prevents this case because before upgrade checker will use current version instead of saved version to determine version which we run upgrade from. Change-Id: I5e6ae6ba2ae2e60b9812e131d2a7c533f4a38ab6 Related-bug: #1349833
This commit is contained in:
parent
aed9ca4c68
commit
001ffbd156
@ -190,6 +190,7 @@ def config(update_path):
|
||||
new_version = get_version_from_config(new_upgrade_version_path)
|
||||
new_version_path = join('/etc/fuel', new_version, 'version.yaml')
|
||||
|
||||
version_files_mask = '/var/lib/fuel_upgrade/*/version.yaml'
|
||||
working_directory = join('/var/lib/fuel_upgrade', new_version)
|
||||
|
||||
from_version_path = join(working_directory, 'version.yaml')
|
||||
|
@ -39,6 +39,13 @@ class UpgradeEngine(object):
|
||||
"""Rollback all the changes, generally used in case of failed upgrade.
|
||||
"""
|
||||
|
||||
@abc.abstractmethod
|
||||
def on_success(self):
|
||||
"""Here engine can execute some simple actions,
|
||||
errors during the execution of this action will
|
||||
be ignored and rollback won't be ran
|
||||
"""
|
||||
|
||||
@abc.abstractproperty
|
||||
def required_free_space(self):
|
||||
"""Required free space for upgarde
|
||||
|
@ -51,5 +51,8 @@ class BootstrapUpgrader(UpgradeEngine):
|
||||
@property
|
||||
def required_free_space(self):
|
||||
return get_required_size_for_actions(
|
||||
self.config.bootstrap['actions'], self.config.update_path
|
||||
)
|
||||
self.config.bootstrap['actions'], self.config.update_path)
|
||||
|
||||
def on_success(self):
|
||||
"""Do nothing for this engine
|
||||
"""
|
||||
|
@ -94,6 +94,33 @@ class DockerUpgrader(UpgradeEngine):
|
||||
self.stop_fuel_containers()
|
||||
self.supervisor.restart_and_wait()
|
||||
|
||||
def on_success(self):
|
||||
"""Remove saved version files for all upgrades
|
||||
|
||||
NOTE(eli): It solves several problems:
|
||||
|
||||
1. user runs upgrade 5.0 -> 5.1 which fails
|
||||
upgrade system saves version which we upgrade
|
||||
from in file working_dir/5.1/version.yaml.
|
||||
Then user runs upgrade 5.0 -> 5.0.1 which
|
||||
successfully upgraded. Then user runs again
|
||||
upgrade 5.0.1 -> 5.1, but there is saved file
|
||||
working_dir/5.1/version.yaml which contains
|
||||
5.0 version, and upgrade system thinks that
|
||||
it's upgrading from 5.0 version, as result
|
||||
it tries to make database dump from wrong
|
||||
version of container.
|
||||
|
||||
2. without this hack user can run upgrade
|
||||
second time and loose his data, this hack
|
||||
prevents this case because before upgrade
|
||||
checker will use current version instead
|
||||
of saved version to determine version which
|
||||
we run upgrade from.
|
||||
"""
|
||||
for version_file in glob.glob(self.config.version_files_mask):
|
||||
utils.remove(version_file)
|
||||
|
||||
@property
|
||||
def required_free_space(self):
|
||||
"""Required free space to run upgrade
|
||||
|
@ -93,6 +93,10 @@ class HostSystemUpgrader(UpgradeEngine):
|
||||
self.version_file.switch_to_previous()
|
||||
self.remove_repo_config()
|
||||
|
||||
def on_success(self):
|
||||
"""Do nothing for this engine
|
||||
"""
|
||||
|
||||
def update_repo(self):
|
||||
"""Add new centos repository
|
||||
"""
|
||||
|
@ -123,6 +123,10 @@ class OpenStackUpgrader(UpgradeEngine):
|
||||
|
||||
logger.info('rollback is done!')
|
||||
|
||||
def on_success(self):
|
||||
"""Do nothing for this engine
|
||||
"""
|
||||
|
||||
def install_releases(self):
|
||||
# check releases for existing in nailgun side
|
||||
releases = self._get_unique_releases(
|
||||
|
@ -40,10 +40,11 @@ class TestBootstrapUpgrader(BaseTestCase):
|
||||
|
||||
self.called_once(self.upgrader._action_manager.undo)
|
||||
|
||||
def test_on_success_does_not_raise_exceptions(self):
|
||||
self.upgrader.on_success()
|
||||
|
||||
@mock.patch('fuel_upgrade.utils.os.path.isdir', return_value=True)
|
||||
@mock.patch('fuel_upgrade.utils.dir_size', return_value=42)
|
||||
def test_required_free_space(self, _, __):
|
||||
result = self.upgrader.required_free_space
|
||||
self.assertEqual(result, {
|
||||
'/var/www/nailgun/9999_bootstrap': 42,
|
||||
})
|
||||
self.assertEqual(result, {'/var/www/nailgun/9999_bootstrap': 42})
|
||||
|
@ -98,6 +98,16 @@ class TestDockerUpgrader(BaseTestCase):
|
||||
self.called_once(self.version_mock.save_current)
|
||||
self.called_once(self.version_mock.switch_to_previous)
|
||||
|
||||
@mock.patch('fuel_upgrade.engines.docker_engine.utils')
|
||||
@mock.patch('fuel_upgrade.engines.docker_engine.glob.glob',
|
||||
return_value=['file1', 'file2'])
|
||||
def test_on_success(self, glob_mock, utils_mock):
|
||||
self.upgrader.on_success()
|
||||
glob_mock.assert_called_once_with(self.fake_config.version_files_mask)
|
||||
self.assertEqual(
|
||||
utils_mock.remove.call_args_list,
|
||||
[(('file1',),), (('file2',),)])
|
||||
|
||||
def test_stop_fuel_containers(self):
|
||||
non_fuel_images = [
|
||||
'first_image_1.0', 'second_image_2.0', 'third_image_2.0']
|
||||
|
@ -71,6 +71,9 @@ class TestHostSystemUpgrader(BaseTestCase):
|
||||
self.called_once(self.version_mock.save_current)
|
||||
self.called_once(self.version_mock.switch_to_previous)
|
||||
|
||||
def test_on_success_does_not_raise_exceptions(self):
|
||||
self.upgrader.on_success()
|
||||
|
||||
@mock.patch('fuel_upgrade.engines.host_system.utils')
|
||||
def test_remove_repo_config(self, utils_mock):
|
||||
self.upgrader.remove_repo_config()
|
||||
|
@ -113,6 +113,9 @@ class TestOpenStackUpgrader(BaseTestCase):
|
||||
self.called_once(self.upgrader.action_manager.do)
|
||||
self.called_once(i_releases)
|
||||
|
||||
def test_on_success_does_not_raise_exceptions(self):
|
||||
self.upgrader.on_success()
|
||||
|
||||
@mock.patch(
|
||||
'fuel_upgrade.engines.openstack.OpenStackUpgrader.install_releases')
|
||||
def test_upgrade_with_errors(self, i_releases):
|
||||
|
@ -91,3 +91,21 @@ class TestUpgradeManager(BaseTestCase):
|
||||
|
||||
engine_mock.upgrade.assert_called_once_with()
|
||||
self.method_was_not_called(engine_mock.rollback)
|
||||
|
||||
def test_upgrade_run_on_success_methods(self):
|
||||
engines = [mock.Mock(), mock.Mock(), mock.Mock()]
|
||||
upgrader = UpgradeManager(**self.default_args(upgraders=engines))
|
||||
upgrader.run()
|
||||
|
||||
for engine in engines:
|
||||
self.called_once(engine.on_success)
|
||||
|
||||
def test_upgrade_does_not_fail_if_on_success_method_raise_error(self):
|
||||
error_engine = mock.Mock()
|
||||
error_engine.on_success.side_effect = Exception('error')
|
||||
engines = [mock.Mock(), error_engine, mock.Mock()]
|
||||
upgrader = UpgradeManager(**self.default_args(upgraders=engines))
|
||||
upgrader.run()
|
||||
|
||||
for engine in engines:
|
||||
self.called_once(engine.on_success)
|
||||
|
@ -60,8 +60,27 @@ class UpgradeManager(object):
|
||||
logger.error('*** UPGRADE FAILED')
|
||||
raise
|
||||
|
||||
self._on_success()
|
||||
logger.info('*** UPGRADE DONE SUCCESSFULLY')
|
||||
|
||||
def _on_success(self):
|
||||
"""Run on_success method for engines,
|
||||
skip method call if there were some errors,
|
||||
because if upgrade succeed we shouldn't
|
||||
fail it.
|
||||
"""
|
||||
for upgrader in self._upgraders:
|
||||
try:
|
||||
logger.debug(
|
||||
'%s: run on_success method',
|
||||
upgrader.__class__.__name__)
|
||||
upgrader.on_success()
|
||||
except Exception as exc:
|
||||
logger.exception(
|
||||
'%s: skip the engine because failed to '
|
||||
'execute on_success method: "%s"',
|
||||
upgrader.__class__.__name__, exc)
|
||||
|
||||
def rollback(self):
|
||||
logger.debug('Run rollback')
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user