PR#51038: Issue 49731 - undo db_home_dir under /dev/shm/dirsrv for now - 389-ds-base

389-ds-base

#51038 Issue 49731 - undo db_home_dir under /dev/shm/dirsrv for now

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

mreynolds/389-ds-base dbhome into master

Issue 49731 - undo db_home_dir under /dev/shm/dirsrv for now

Mark Reynolds • 3 years ago

3ca89e3

ldap/admin/src/defaults.inf.in

file modified

+1 -1

		`@@ -58,7 +58,7 @@`
		`audit_log = @localstatedir@/log/dirsrv/slapd-{instance_name}/audit`
		`error_log = @localstatedir@/log/dirsrv/slapd-{instance_name}/errors`
		`db_dir = @localstatedir@/lib/dirsrv/slapd-{instance_name}/db`
		`- db_home_dir = /dev/shm/dirsrv/slapd-{instance_name}`
		`+ db_home_dir = @localstatedir@/lib/dirsrv/slapd-{instance_name}/db`
		`backup_dir = @localstatedir@/lib/dirsrv/slapd-{instance_name}/bak`
		`ldif_dir = @localstatedir@/lib/dirsrv/slapd-{instance_name}/ldif`

ldap/servers/slapd/util.c

file modified

+1 -1

		`@@ -471,7 +471,7 @@`
		`void`
		`replace_char(char *str, char c, char c2)`
		`{`
		`- for (size_t i = 0; (str != NULL) && (str[i] != NULL); i++) {`
		`+ for (size_t i = 0; (str != NULL) && (str[i] != '\0'); i++) {`
		`if (c == str[i]) {`
		`str[i] = c2;`
		`}`

rpm/389-ds-base.spec.in

file modified

-2

		`@@ -431,7 +431,6 @@`
		`mkdir -p $RPM_BUILD_ROOT/var/log/%{pkgname}`
		`mkdir -p $RPM_BUILD_ROOT/var/lib/%{pkgname}`
		`mkdir -p $RPM_BUILD_ROOT/var/lock/%{pkgname}`
		`- mkdir -p $RPM_BUILD_ROOT/dev/shm/%{pkgname}`

		`# for systemd`
		`mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/systemd/system/%{groupname}.wants`
		`@@ -637,7 +636,6 @@`
		`%{_prefix}/lib/sysctl.d/*`
		`%dir %{_localstatedir}/lib/%{pkgname}`
		`%dir %{_localstatedir}/log/%{pkgname}`
		`- %dir /dev/shm/%{pkgname}`
		`%ghost %dir %{_localstatedir}/lock/%{pkgname}`
		`%exclude %{_sbindir}/ldap-agent*`
		`%exclude %{_mandir}/man1/ldap-agent.1.gz`

src/lib389/lib389/instance/setup.py

file modified

+2 -2

		`@@ -747,7 +747,7 @@`

		`# Create all the needed paths`
		`# we should only need to make bak_dir, cert_dir, config_dir, db_dir, ldif_dir, lock_dir, log_dir, run_dir?`
		`- for path in ('backup_dir', 'cert_dir', 'db_dir', 'db_home_dir', 'ldif_dir', 'lock_dir', 'log_dir', 'run_dir'):`
		`+ for path in ('backup_dir', 'cert_dir', 'db_dir', 'ldif_dir', 'lock_dir', 'log_dir', 'run_dir'):`
		`self.log.debug("ACTION: creating %s", slapd[path])`
		`try:`
		`os.umask(0o007) # For parent dirs that get created -> sets 770 for perms`
		`@@ -868,7 +868,7 @@`
		`# Do selinux fixups`
		`if general['selinux']:`
		`selinux_paths = ('backup_dir', 'cert_dir', 'config_dir', 'db_dir',`
		`- 'db_home_dir', 'ldif_dir', 'lock_dir', 'log_dir',`
		`+ 'ldif_dir', 'lock_dir', 'log_dir',`
		`'run_dir', 'schema_dir', 'tmp_dir')`
		`for path in selinux_paths:`
		`selinux_restorecon(slapd[path])`

mreynolds commented 3 years ago

Bug Description:

There are several issues with using /dec/shm/disrv/ for the db home directory. Containers have issues, and system reboots can cause issues too.

Fix Description:

Using just /dev/shm/slapd-INST solves all the permission issues, but that requires a new selinux label, so for now we will just set the db home directory to the database directory (effectively disabling the change).

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

firstyear commented 3 years ago

Ack from me

tbordaz commented 3 years ago

congratulation @mreynolds for this tough investigations. Ack for me as well.

Just out of curiosity and for the recording. The problem being permission to access /dev/shm/dirsrv and /dev/shm/dirsrv/slapd-INST.
A discussed option were to create /dev/shm/dirsrv by systemd (solving reboot and selinux constraint) and /dev/shm/dirsrv/slapd-INST by the DS backend initialization.
Was this option failing as well or abandoned because of the urgency/risk to fix the issue or something else ?

mreynolds commented 3 years ago

congratulation @mreynolds for this tough investigations. Ack for me as well.
Just out of curiosity and for the recording. The problem being permission to access /dev/shm/dirsrv and /dev/shm/dirsrv/slapd-INST.
A discussed option were to create /dev/shm/dirsrv by systemd (solving reboot and selinux constraint) and /dev/shm/dirsrv/slapd-INST by the DS backend initialization.
Was this option failing as well or abandoned because of the urgency/risk to fix the issue or something else ?

No we just need to drop the entire /dev/shm/dirsrv/ approach. We should just allow the instance at start up to create /dev/shm/slapd-inst (once they make a new selinux rule for those directories). I had a community member on Debian also verify that this approach works correctly and doesn't cause anything strange to happen after reboots.

So once we get the new selinux labels created (bug is pending), then we can set the default to /dev/shm/slapd-inst in defaults.inf and we are done. No other changes will be needed.

mreynolds commented 3 years ago

congratulation @mreynolds for this tough investigations. Ack for me as well.
Just out of curiosity and for the recording. The problem being permission to access /dev/shm/dirsrv and /dev/shm/dirsrv/slapd-INST.
A discussed option were to create /dev/shm/dirsrv by systemd (solving reboot and selinux constraint) and /dev/shm/dirsrv/slapd-INST by the DS backend initialization.
Was this option failing as well or abandoned because of the urgency/risk to fix the issue or something else ?

No we just need to drop the entire /dev/shm/dirsrv/ approach. We should just allow the instance at start up to create /dev/shm/slapd-inst (once they make a new selinux rule for those directories). I had a community member on Debian also verify that this approach works correctly and doesn't cause anything strange to happen after reboots.
So once we get the new selinux labels created (bug is pending), then we can set the default to /dev/shm/slapd-inst in defaults.inf and we are done. No other changes will be needed.

Well we still need to do some container magic for cases where dscontainer is not used.

rebased onto 3ca89e3

3 years ago

Pull-Request has been merged by mreynolds

3 years ago