Proposal: Improve Debian package quality and compatibility across debian-based distributions by using proper debhelper scripts and respecting the debian packaging policy

[automaticserver]

This will solve most (if not all) issues collected in #4087!

The current problem is that the debian source package mainly contains files for setting up a debian package quick and dirty (https://github.com/opensearch-project/opensearch-build/tree/2.11.0/scripts/pkg/build_templates/opensearch/deb/debian). There only exists the control file, the rules file and manually written maintainer scripts.

The quality and compatibility of the debian package can be improved drasticly if this debian source package provides files and references recognized by debhelper/debian packaging. There is no need to change the assemble workflow - we just need to provide proper setup so debhelper can do its thing.

I tried to reflect the state of the assemble workflow https://github.com/opensearch-project/opensearch-build/blob/2.11.0/src/assemble_workflow/bundle_linux_deb.py#L112 as close as possible.

source.tar.gz
result.tar.gz
(I have left the original maintainer scripts, they obviously should be removed in favour of the generated parts. Look for lines starting with # Automatically added by ...)

The command to build the source above (is exactly the same as in the assemble workflow): debmake --fullname "OpenSearch Team" --email "[email protected]" --invoke debuild --native --revision 1 --package opensearch --upstreamversion 1.3.0

But, there might be differences on how debhelper will do or place things that needs to be compared.

As I've seen somewhere the runner configured to assemble the deb seems to be ubuntu 20.04, so all links provided below are to manpages for those debhelpers for this ubuntu release.

systemd units

Use *.service (and other systemd extension) files and invoke dh_installsystemd with options (if required). The example above reflects exactly the current implementation: don't start on installation, stop on upgrade (Whether that is the intended good behaviour or not). Additionally the generated maintainer scripts use the correct abstraction to enable and start/stop/restart the service(s).

sysv inits

Use *.init files and invoke dh_installinit with options if required. Here as well, the generated maintainer scripts use the correct abstraction to enable/start/stop/restart the service(s).

default files

Use *.default files to place that file automatically to /etc/default/*.

system user opensarch

Use *.sysusr files to define the user instad of manually creating it. dh_sysusr will generate appropriate maintainer scripts.

Other options

There are more options that can be used, I'm mainly showing where to start (and where a lot of issues seem to be)

Overall, have a look at debhelper's manpage for all integrated options and their usage. They are all available to be used. Also consult the debian policy manual.

I'm happy to be a helping hand in this but I couldn't figure out how to properly run the assemble process locally. There seem to be also a lot of tests checking the resulting package.

Bonus: Lintian

Warnings of lintian are usually a good indication of bad practices and we should rethink the approaches:

W: opensearch: command-with-path-in-maintainer-script /usr/bin/getconf (in backticks) [postinst:31]
W: opensearch source: custom-compression-in-debian-rules dh_builddeb -- -Zgzip [debian/rules:20]
W: opensearch: description-starts-with-leading-spaces line 1
W: opensearch: description-synopsis-starts-with-article
W: opensearch: init.d-script-does-not-source-init-functions [etc/init.d/opensearch]
W: opensearch: init.d-script-missing-lsb-section [etc/init.d/opensearch]
W: opensearch: maintainer-script-calls-chown-improperly "chown -R opensearch.opensearch" [postinst:41]
W: opensearch: maintainer-script-calls-chown-improperly "chown -R opensearch.opensearch" [postinst:42]
W: opensearch: maintainer-script-calls-chown-improperly "chown -R opensearch.opensearch" [postinst:43]
W: opensearch: maintainer-script-calls-chown-improperly "chown -R opensearch.opensearch" [postinst:44]
W: opensearch: maintainer-script-calls-chown-improperly "chown -R opensearch.opensearch" [postinst:45]
W: opensearch: maintainer-script-calls-systemctl [postinst:54]
W: opensearch: maintainer-script-calls-systemctl [preinst:19]
W: opensearch: maintainer-script-calls-systemctl [preinst:23]
W: opensearch: maintainer-script-calls-systemctl [prerm:19]
W: opensearch: maintainer-script-calls-systemctl [prerm:23]
W: opensearch: possibly-insecure-handling-of-tmp-files-in-maintainer-script /tmp [postinst:28]
W: opensearch: recursive-privilege-change "chown -R" [postinst:41]
W: opensearch: recursive-privilege-change "chown -R" [postinst:42]
W: opensearch: recursive-privilege-change "chown -R" [postinst:43]
W: opensearch: recursive-privilege-change "chown -R" [postinst:44]
W: opensearch: recursive-privilege-change "chown -R" [postinst:45]

E.g.:

• do really all folders need write access by opensearch user? Usually files belong root unless its a data/working dir of the service.
• direct invocation of systemctl is bad, use dh_* above for the correct abstractions
• etc.

Bonus: Maintainer scripts lifecycle

It is also good to know which maintainer script is called how in the various installation/upgrade/remove operations. Have a read of this post.

What about RPM?

Good question, they seem similiar but I have no experience building rpm packages. Maybe many/most of the improvements can be taken over there too.

/dm

[automaticserver]

While above was to show the general idea, lets go here into some issue details. The exact output depends on the version of debhelper installed, so my output might be a tad bit improved than what's available on ubuntu 20.04

In #3891 the issue is that the package default behaviour is not to enable and start the service, but it should be possible that an upgrade of a package also restarts the service if it was running.

rules:

override_dh_installinit:
	dh_installinit --no-enable --no-start --restart-after-upgrade
# where is this?
#	dh_installinit --no-enable --no-start --restart-after-upgrade --name opensearch-performance-analyzer 

override_dh_installsystemd:
	dh_installsystemd --no-enable --no-start --restart-after-upgrade
	dh_installsystemd --no-enable --no-start --restart-after-upgrade --name opensearch-performance-analyzer 

produces:

#preinst

# Automatically added by dh_installinit/13.11.4
if [ "$1" = "install" ] && [ -n "$2" ] && [ -e "/etc/init.d/opensearch" ] ; then
	chmod +x "/etc/init.d/opensearch" >/dev/null || true
fi
# End automatically added section

#postinst

# Automatically added by dh_installinit/13.11.4
if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
	if [ -x "/etc/init.d/opensearch" ]; then
		update-rc.d opensearch defaults-disabled >/dev/null || exit 1
	fi
fi
# End automatically added section
# Automatically added by dh_installsystemd/13.11.4
if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
	if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then
		systemctl --system daemon-reload >/dev/null || true
		if [ -n "$2" ]; then
			deb-systemd-invoke try-restart 'opensearch.service' >/dev/null || true
		fi
	fi
fi
# End automatically added section
# Automatically added by dh_installsystemd/13.11.4
if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
	if [ -z "${DPKG_ROOT:-}" ] && [ -d /run/systemd/system ]; then
		systemctl --system daemon-reload >/dev/null || true
		if [ -n "$2" ]; then
			deb-systemd-invoke try-restart 'opensearch-performance-analyzer.service' >/dev/null || true
		fi
	fi
fi
# End automatically added section

#postrm

# Automatically added by dh_installinit/13.11.4
if [ "$1" = "remove" ] && [ -x "/etc/init.d/opensearch" ] ; then
	chmod -x "/etc/init.d/opensearch" >/dev/null || true
fi
if [ -z "${DPKG_ROOT:-}" ] && [ "$1" = "purge" ] ; then
	update-rc.d opensearch remove >/dev/null
fi
# End automatically added section

So we gain a deb-systemd-invoke try-restart ... during postinst. There is a catch though (from dh_installsystemd:

But you should make sure that the daemon will not get confused by the package being upgraded while it's running before using this option.

Because before it is invoked all the files have been upgraded already while the service was still running. You only can know, if this is the desired action.

There's also the possibility to use --no-restart-after-upgrade which by default does a stop on preinst and a start in postinst. But combined with --no-start will not start it again and will lead again in the issue referenced above.

If on upgrade stop to start is desired instead of restart while still shouldn't start on installation then it would need to be manually written in those maintainer scripts as this situation seem to be not expressible by the current options. Maybe raise a bug to debian for your use case and in some point in the future might be available.

Depending on the wished behaviour, see also deb-systemd-helper and deb-systemd-invoke

---

#4084 is not affected by deb. As far I can see all files installed in /etc are listed in DEBIAN/conffiles (by debuild or a debhelper automatically) and thus dpkg is careful and relays detected changes to the admin interactively (if not actively suppressed) on upgrade.

---

#4240 is covered implicitly by using dh_installsystemd and dh_installinit

/dm

[automaticserver]

Empty dirs can be expressed in *.dirs (see dh_installdirs), links can be expressed in *.links (see dh_links) and ignoring lintian warnings (if correctly wished so) can be expressed in *.lintian-overrides (see dh_lintian)

Settings permissions would still be part of manual maintainer scripts (like it is already), as there's no debhelper for that as far as I know. Restarting sysctl probably as well unless there's a way the opensearch.service file can depend on sysctl.service so it restarts it on installation/upgrade - that will be a systemd's service dependency question.

/dm

[jordarlu]

Thanks for the issue and comments @automaticserver , let the community have a look and comment here....