PR#49641: Issue 49612 - lib389 remove_ds_instance() does not remove systemd units - 389-ds-base

389-ds-base

#49641 Issue 49612 - lib389 remove_ds_instance() does not remove systemd units

Closed 3 years ago by spichugi. Opened 6 years ago by spichugi.

spichugi/389-ds-base remove_ds_fix into master

Issue 49612 - lib389 remove_ds_instance() does not remove systemd units

Simon Pichugin • 6 years ago

bc181f6

src/lib389/lib389/__init__.py

file modified

+2 -1

		`@@ -415,7 +415,8 @@`

		`# The lack of this value basically rules it out in most cases`
		`self.isLocal = True`
		`- self.ds_paths = Paths(instance=self)`
		`+ self.ds_paths = Paths(serverid, instance=self)`
		`+ self.serverid = serverid`

		`# Do we have ldapi settings?`
		`# Do we really need .strip() on this?`

src/lib389/lib389/instance/remove.py

file modified

+16

		`@@ -8,6 +8,7 @@`

		`import os`
		`import shutil`
		`+ import subprocess`

		`def remove_ds_instance(dirsrv):`
		`"""`
		`@@ -41,6 +42,17 @@`
		`_log.debug("Checking for instance marker at %s" % marker_path)`
		`assert os.path.exists(marker_path)`

		`+ # Move the config_dir to config_dir.removed`
		`+ config_dir = dirsrv.ds_paths.config_dir`
		`+ config_dir_rm = "{}.removed".format(config_dir)`
		`+`
		`+ if os.path.exists(config_dir_rm):`
		`+ _log.debug("Removing previously existed %s" % config_dir_rm)`
		`+ shutil.rmtree(config_dir_rm)`
		`+`
		`+ _log.debug("Copying %s to %s" % (config_dir, config_dir_rm))`
		`+ shutil.copytree(config_dir, config_dir_rm)`
		`+`
		`# Remove these paths:`
		`# for path in ('backup_dir', 'cert_dir', 'config_dir', 'db_dir',`
		`# 'ldif_dir', 'lock_dir', 'log_dir', 'run_dir'):`
		`@@ -52,6 +64,10 @@`
		`os.remove(marker_path)`
		`_log.debug("Removing %s" % marker_path)`

		`+ # Remove the systemd symlink`
		`+ _log.debug("Removing the systemd symlink")`
		`+ subprocess.check_call(["systemctl", "disable", "dirsrv@{}".format(dirsrv.serverid)])`
		`+`
		`# Done!`
		`_log.debug("Complete")`

src/lib389/lib389/tests/instance/remove_test.py

file added

+53

		`@@ -0,0 +1,53 @@`
		`+ # --- BEGIN COPYRIGHT BLOCK ---`
		`+ # Copyright (C) 2018 Red Hat, Inc.`
		`+ # All rights reserved.`
		`+ #`
		`+ # License: GPL (version 3 or any later version).`
		`+ # See LICENSE for details.`
		`+ # --- END COPYRIGHT BLOCK ---`
		`+`
		`+ import os`
		`+ import subprocess`
		`+ import pytest`
		`+ from lib389.instance.remove import remove_ds_instance`
		`+ from lib389._constants import ReplicaRole`
		`+ from lib389.topologies import create_topology`
		`+`
		`+`
		`+ @pytest.fixture(scope="module")`
		`+ def topology_st(request):`
		`+ """Create DS standalone instance"""`
		`+`
		`+ topology = create_topology({ReplicaRole.STANDALONE: 1})`
		`+`
		`+ def fin():`
		`+ if topology.standalone.exists():`
		`+ topology.standalone.delete()`
		`+ request.addfinalizer(fin)`
		`+`
		`+ return topology`
		`+`
		`+`
		`+ def test_basic(topology_st):`
		`+ """Check that all DS directories and systemd items were removed"""`
		`+`
		`+ inst = topology_st.standalone`
		`+`
		`+ remove_ds_instance(inst)`
		`+`
		`+ paths = [inst.ds_paths.backup_dir,`
		`+ inst.ds_paths.cert_dir,`
		`+ inst.ds_paths.config_dir,`
		`+ inst.ds_paths.db_dir,`
		`+ inst.get_changelog_dir(),`
		`+ inst.ds_paths.ldif_dir,`
		`+ inst.ds_paths.lock_dir,`
		`+ inst.ds_paths.log_dir,`
		`+ "{}/sysconfig/dirsrv-{}".format(inst.ds_paths.sysconf_dir, inst.serverid)]`
		`+ for path in paths:`
		`+ assert not os.path.exists(path)`
		`+`
		`+ try:`
		`+ subprocess.check_output(['systemctl', 'is-enabled', 'dirsrv@{}'.format(inst.serverid)], encoding='utf-8')`
		`+ except subprocess.CalledProcessError as ex:`
		`+ assert "disabled" in ex.output`

spichugi commented 6 years ago

Bug description: When running remove_ds_instance(), it does not remove
the systemd files that identify this instance.
Also, in previous DS versions, the /etc/dirsrv/slapd-INSTANCE
would have been moved to /etc/dirsrv/slapd-INSTANCE.removed.
This does not happen now.

Fix description: Change remove_ds_instance function so it disables systemd unit
and, in the result, it will remove symlinks. Also make the function
move the config_dir to config_dir.removed as remove-ds.pl does.
Add a basic test to src/lib389/lib389/tests/instance/remove_test.py
Fix local_simple_allocate, so it sets serverid properly.

Reviewed by: ?

https://pagure.io/389-ds-base/issue/49612

rebased onto bc181f6

6 years ago

Pull-Request has been merged by spichugi

6 years ago

spichugi commented 3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/2700

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi