diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py index e8f67a3de..86503a66b 100644 --- a/ipapython/install/cli.py +++ b/ipapython/install/cli.py @@ -316,10 +316,10 @@ class ConfigureTool(admintool.AdminTool): if self.use_private_ccache: with private_ccache(): super(ConfigureTool, self).run() - cfgr.run() + return cfgr.run() else: super(ConfigureTool, self).run() - cfgr.run() + return cfgr.run() @staticmethod def __signal_handler(signum, frame): diff --git a/ipapython/install/common.py b/ipapython/install/common.py index 05dfadfcb..ca1e8fd30 100644 --- a/ipapython/install/common.py +++ b/ipapython/install/common.py @@ -109,7 +109,7 @@ def installer(cls): def uninstaller(cls): - class Uninstaller(Continuous, cls, Installable): + class Uninstaller(cls, Installable): def __init__(self, **kwargs): super(Uninstaller, self).__init__(uninstalling=True, **kwargs) diff --git a/ipapython/install/core.py b/ipapython/install/core.py index 7ded343d9..2ed41f63e 100644 --- a/ipapython/install/core.py +++ b/ipapython/install/core.py @@ -361,7 +361,7 @@ class Configurable(six.with_metaclass(abc.ABCMeta, object)): self.validate() if self.__state == _EXECUTE_PENDING: - self.execute() + return self.execute() def validate(self): """ @@ -384,9 +384,13 @@ class Configurable(six.with_metaclass(abc.ABCMeta, object)): """ Run the execution part of the configurable. """ + return_value = 0 - for _nothing in self._executor(): - pass + for rval in self._executor(): + if rval is not None and rval > return_value: + return_value = rval + + return return_value def _executor(self): """ diff --git a/ipatests/test_integration/test_uninstallation.py b/ipatests/test_integration/test_uninstallation.py new file mode 100644 index 000000000..d7d6f6277 --- /dev/null +++ b/ipatests/test_integration/test_uninstallation.py @@ -0,0 +1,79 @@ +# +# Copyright (C) 2018 FreeIPA Contributors see COPYING for license +# + +""" +Module provides tests that uninstallation is successful. + +It is important not to leave the remote system in an inconsistent +state. Every failed uninstall should successfully remove remaining +pieces if possible. +""" + +from ipatests.test_integration.base import IntegrationTest +from ipatests.pytest_plugins.integration import tasks +from ipaplatform.paths import paths +from ipaserver.install.installutils import realm_to_serverid +from ipaserver.install import dsinstance + + +class TestUninstallBase(IntegrationTest): + + @classmethod + def install(cls, mh): + tasks.install_master(cls.master, setup_dns=False) + + def test_failed_uninstall(self): + self.master.run_command(['ipactl', 'stop']) + + serverid = realm_to_serverid(self.master.domain.realm) + instance_name = ''.join([dsinstance.DS_INSTANCE_PREFIX, serverid]) + + try: + # Moving the DS instance out of the way will cause the + # uninstaller to raise an exception and return with a + # non-zero return code. + self.master.run_command([ + '/usr/bin/mv', + '%s/%s' % (paths.ETC_DIRSRV, instance_name), + '%s/%s.test' % (paths.ETC_DIRSRV, instance_name) + ]) + + cmd = self.master.run_command([ + 'ipa-server-install', + '--uninstall', '-U'], + raiseonerr=False + ) + assert cmd.returncode == 1 + finally: + # Be paranoid. If something really went wrong then DS may + # be marked as uninstalled so server cert will still be + # tracked and the instances may remain. This can cause + # subsequent installations to fail so be thorough. + ds = dsinstance.DsInstance() + ds_running = ds.is_running() + if ds_running: + ds.stop(serverid) + + # Moving it back should allow the uninstall to finish + # successfully. + self.master.run_command([ + '/usr/bin/mv', + '%s/%s.test' % (paths.ETC_DIRSRV, instance_name), + '%s/%s' % (paths.ETC_DIRSRV, instance_name) + ]) + + # DS has been marked as uninstalled so force the issue + ds.stop_tracking_certificates(serverid) + + self.master.run_command([ + paths.REMOVE_DS_PL, + '-i', instance_name + ]) + + cmd = self.master.run_command([ + 'ipa-server-install', + '--uninstall', '-U'], + raiseonerr=False + ) + assert cmd.returncode == 0