PR#50125: Issue 50123 - with_tmpfiles_d is associated to systemd - 389-ds-base

389-ds-base

#50125 Issue 50123 - with_tmpfiles_d is associated to systemd

Closed 3 years ago by spichugi. Opened 5 years ago by firstyear.

firstyear/389-ds-base 50123-system-tmpfiles into master

Ticket 50123 - with_tmpfiles_d is associated to systemd

William Brown • 5 years ago

0666b52

Makefile.am

file modified

+2 -1

		`@@ -327,6 +327,7 @@`
		`gdbautoloaddir = $(prefixdir)/share/gdb/auto-load$(sbindir)`
		`cockpitdir = $(prefixdir)/share/cockpit@cockpitdir@`
		`metainfodir = $(prefixdir)/share/metainfo/389-console`
		`+ tmpfiles_d = @tmpfiles_d@`

		`# This has to be hardcoded to /lib - $libdir changes between lib/lib64, but`
		`# sysctl.d is always in /lib.`
		`@@ -2290,7 +2291,7 @@`
		`-e 's,@with_fhs_opt\@,@with_fhs_opt@,g' \`
		`-e 's,@with_selinux\@,@with_selinux@,g' \`
		`-e 's,@with_systemd\@,$(WITH_SYSTEMD),g' \`
		`- -e 's,@with_tmpfiles_d\@,@with_tmpfiles_d@,g' \`
		`+ -e 's,@tmpfiles_d\@,$(tmpfiles_d),g' \`
		`-e 's,@perlexec\@,@perlexec@,g' \`
		`-e 's,@pythonexec\@,@pythonexec@,g' \`
		`-e 's,@sttyexec\@,@sttyexec@,g' \`

configure.ac

file modified

-19

		`@@ -501,25 +501,6 @@`
		`defaultuser=dirsrv`
		`defaultgroup=dirsrv`

		`- if test -z "$with_tmpfiles_d" ; then`
		`- if test -d $sysconfdir/tmpfiles.d ; then`
		`- with_tmpfiles_d='$(sysconfdir)/tmpfiles.d'`
		`- fi`
		`- fi`
		`- AC_MSG_CHECKING(for --with-tmpfiles-d)`
		`- AC_ARG_WITH(tmpfiles-d,`
		`- AS_HELP_STRING([--with-tmpfiles-d=PATH],`
		`- [system uses tmpfiles.d to handle temp files/dirs (default: $with_tmpfiles_d)])`
		`- )`
		`- if test "$with_tmpfiles_d" = yes ; then`
		`- AC_MSG_ERROR([You must specify --with-tmpfiles-d=/full/path/to/tmpfiles.d directory])`
		`- elif test "$with_tmpfiles_d" = no ; then`
		`- with_tmpfiles_d=`
		`- else`
		`- AC_MSG_RESULT([$with_tmpfiles_d])`
		`- fi`
		`- AC_SUBST(with_tmpfiles_d)`
		`-`
		`AC_MSG_CHECKING(for --with-perldir)`
		`AC_ARG_WITH([perldir],`
		`AS_HELP_STRING([--with-perldir=PATH],`

ldap/admin/src/defaults.inf.in

file modified

+3 -2

		`@@ -1,12 +1,13 @@`
		`; --- BEGIN COPYRIGHT BLOCK ---`
		`; Copyright (C) 2016 Red Hat, Inc.`
		`+ ; Copyright (C) 2019 William Brown <william@blackhats.net.au>`
		`; All rights reserved.`
		`;`
		`; License: GPL (version 3 or any later version).`
		`; See LICENSE for details.`
		`; --- END COPYRIGHT BLOCK ---`

		`- ; Author: firstyear at redhat.com`
		`+ ; Author: wbrown at suse.de`

		`; This is a set of default paths that tools consuming DS should search`
		`; for paths. This is the foundation of the version 2 ds setup inf`
		`@@ -40,7 +41,7 @@`
		`inst_dir = @serverdir@/slapd-{instance_name}`
		`plugin_dir = @serverplugindir@`
		`system_schema_dir = @systemschemadir@`
		`- tmpfiles_d = @with_tmpfiles_d@`
		`+ tmpfiles_d = @tmpfiles_d@`

		`; These values can be altered in an installation of ds`
		`user = dirsrv`

m4/systemd.m4

file modified

+20

		`@@ -114,6 +114,24 @@`
		`fi`
		`AC_SUBST(with_systemdgroupname)`

		`+ if test -z "$with_tmpfiles_d" ; then`
		`+ if test -d $sysconfdir/tmpfiles.d ; then`
		`+ tmpfiles_d='$(sysconfdir)/tmpfiles.d'`
		`+ fi`
		`+ fi`
		`+ AC_MSG_CHECKING(for --with-tmpfiles-d)`
		`+ AC_ARG_WITH(tmpfiles-d,`
		`+ AS_HELP_STRING([--with-tmpfiles-d=PATH],`
		`+ [system uses tmpfiles.d to handle temp files/dirs (default: $tmpfiles_d)])`
		`+ )`
		`+ if test "$with_tmpfiles_d" = yes ; then`
		`+ AC_MSG_ERROR([You must specify --with-tmpfiles-d=/full/path/to/tmpfiles.d directory])`
		`+ elif test "$with_tmpfiles_d" = no ; then`
		`+ tmpfiles_d=`
		`+ else`
		`+ tmpfiles_d=$with_tmpfiles_d`
		`+ AC_MSG_RESULT([$tmpfiles_d])`
		`+ fi`

		`fi`
		`# End of with_systemd`
		`@@ -128,4 +146,6 @@`
		`AC_SUBST(systemd_lib)`
		`AC_SUBST(systemd_defs)`

		`+ AC_SUBST(tmpfiles_d)`
		`+`

src/lib389/lib389/__init__.py

file modified

+7 -5

		`@@ -1222,11 +1222,13 @@`
		`env.update(os.environ)`
		`output = None`
		`try:`
		`- output = subprocess.check_output(["%s/ns-slapd" % self.get_sbin_dir(),`
		`- "-D",`
		`- self.ds_paths.config_dir,`
		`- "-i",`
		`- self.ds_paths.pid_file], env=env, stderr=subprocess.STDOUT)`
		`+ cmd = ["%s/ns-slapd" % self.get_sbin_dir(),`
		`+ "-D",`
		`+ self.ds_paths.config_dir,`
		`+ "-i",`
		`+ self.ds_paths.pid_file],`
		`+ self.log.debug("DEBUG: starting with %s" % cmd)`
		`+ output = subprocess.check_output(*cmd, env=env, stderr=subprocess.STDOUT)`
		`except subprocess.CalledProcessError:`
		`self.log.error(output)`
		`count = timeout`

src/lib389/lib389/instance/setup.py

file modified

+8 -7

		`@@ -718,15 +718,16 @@`
		`"enable",`
		`"dirsrv@%s" % slapd['instance_name']])`

		`- # Setup tmpfiles_d`
		`- tmpfile_d = ds_paths.tmpfiles_d + "/dirsrv-" + slapd['instance_name'] + ".conf"`
		`- with open(tmpfile_d, "w") as TMPFILE_D:`
		`- TMPFILE_D.write("d {} 0770 {} {}\n".format(slapd['run_dir'], slapd['user'], slapd['group']))`
		`- TMPFILE_D.write("d {} 0770 {} {}\n".format(slapd['lock_dir'].replace("slapd-" + slapd['instance_name'], ""),`
		`- slapd['user'], slapd['group']))`
		`- TMPFILE_D.write("d {} 0770 {} {}\n".format(slapd['lock_dir'], slapd['user'], slapd['group']))`
		`+ # Setup tmpfiles_d`
		`+ tmpfile_d = ds_paths.tmpfiles_d + "/dirsrv-" + slapd['instance_name'] + ".conf"`
		`+ with open(tmpfile_d, "w") as TMPFILE_D:`
		`+ TMPFILE_D.write("d {} 0770 {} {}\n".format(slapd['run_dir'], slapd['user'], slapd['group']))`
		`+ TMPFILE_D.write("d {} 0770 {} {}\n".format(slapd['lock_dir'].replace("slapd-" + slapd['instance_name'], ""),`
		`+ slapd['user'], slapd['group']))`
		`+ TMPFILE_D.write("d {} 0770 {} {}\n".format(slapd['lock_dir'], slapd['user'], slapd['group']))`

		`# Else we need to detect other init scripts?`
		`+ # WB: No, we just install and assume that docker will start us ...`

		`# Bind sockets to our type?`

firstyear commented 5 years ago

Description: with_tmpfiles_d does not operate unless it's
part of a systemd install. This moves the configure check
and operation to within the "with_systemd" code.

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

Reviewed by: ???

rebased onto 0d1592128c2e2c2c4ce8a32c38c7db37208e972b

5 years ago

vashirov commented 5 years ago

It seems there are unrelated changes to nunc stans stress test and lib389 init.py. Are these really required here?

firstyear commented 5 years ago

They are not required here :) I'll fix this up

rebased onto d5e3bdd915cb2814c5122ed9e205c862ec5514d5

5 years ago

firstyear commented 5 years ago

@vashirov Would you mind checking this again?

rebased onto dc2423bc8acb5e387d6f8792ddc8749ef28b946b

5 years ago

vashirov commented 5 years ago

It's not working as expected. After building rpm, defaults.inf contains non-replaced value:

tmpfiles_d = @tmpfiles_d@

You also need to update Makefile.am where this replacement happens. Or revert back this chunk:

- tmpfiles_d = @with_tmpfiles_d@
+ tmpfiles_d = @tmpfiles_d@

Edited 5 years ago by vashirov

rebased onto 21c18c11db06c275becddd19ceeeae0529c09583

5 years ago

firstyear commented 5 years ago

I don't have a systemd capable machine to test with, but I can confirm the value looks like it's getting through now. I think the issue is the lack of the AC_SUBST outside of the systemd check which needed to be present to set empty/value when defined. I've also fixed the naming.

firstyear commented 5 years ago

@vashirov Can you check this please :)

vashirov commented on line 20 of m4/systemd.m4 5 years ago

You have spaces around =
Because of that during the build I see the following error:

checking for --with-tmpfiles-d... ./configure: line 22831: tmpfiles_d: command not found

And as a consequence, tmpfiles_d is set to empty value in defaults.inf.

firstyear commented 5 years ago

Removed spaces :)

rebased onto 81bf46790d6e41b8929dbfe62616290d7e109005

5 years ago

vashirov commented 5 years ago

Thanks, ack!

rebased onto 0666b52

5 years ago

Pull-Request has been merged by firstyear

5 years ago

vashirov commented 5 years ago

I forgot that perl utils also use with_tmpfiles_d:

# setup-ds.pl 
Global symbol "@with_tmpfiles_d" requires explicit package name (did you forget to declare "my @with_tmpfiles_d"?) at /usr/lib64/dirsrv/perl/DSCreate.pm line 1069.
Global symbol "@with_tmpfiles_d" requires explicit package name (did you forget to declare "my @with_tmpfiles_d"?) at /usr/lib64/dirsrv/perl/DSCreate.pm line 1433.
Compilation failed in require at /usr/sbin/setup-ds.pl line 21.
BEGIN failed--compilation aborted at /usr/sbin/setup-ds.pl line 21.

mreynolds commented 5 years ago

This broke dscreate:

[mareynol@localhost cli]$ sudo ./dscreate from-file /data/dssetup.inf

Starting installation...
Created symlink /etc/systemd/system/multi-user.target.wants/dirsrv@localhost.service → /usr/lib/systemd/system/dirsrv@.service.
Error: [Errno 2] No such file or directory: '@tmpfiles_d@/dirsrv-localhost.conf'

firstyear commented 5 years ago

:( Did you want me to fix this up as well? This is in 1.4.x? I know we support pl tools for 1.3.x, but if this is 1.4.x, is there a reason we still check them?

vashirov commented 5 years ago

Yes, please. 1.4 still ships with legacy tools in Fedora and RHEL8. We need to provide an upgrade path for existing users. That means they should work as before, although should not be used interchangeably.

firstyear commented 5 years ago

I'll do this as a priority now then (I think it's blocking @mreynolds in another patch).

firstyear commented 5 years ago

https://pagure.io/389-ds-base/pull-request/50174