Bug 15468 - in detect_init_style() setting wrong CTDB_INIT_STYLE on redhat when "killproc-2.21-1.2.x86_64" is installed.
Summary: in detect_init_style() setting wrong CTDB_INIT_STYLE on redhat when "killproc...
Status: RESOLVED WONTFIX
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: CTDB (show other bugs)
Version: 4.18.0
Hardware: All Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Martin Schwenke
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-09-04 11:10 UTC by saurabh.singh
Modified: 2023-10-30 11:30 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description saurabh.singh 2023-09-04 11:10:25 UTC
There is an issue in detect_init_style(), where there is a check for “/sbin/startproc” and if is exists and is executable then CTDB_INIT_STYLE is set to “suse”.
Now on redhat, this file “/sbin/startproc”  gets installed as part of the rpm “killproc-2.21-1.2.x86_64” and hence while running on redhat, we see CTDB_INIT_STYLE set to “suse” instead of redhat when “killproc-2.21-1.2.x86_64” is installed.

Can we simply check for /etc/redhat-release to see if the system is RHEL?

Below is the code -
File - ./ctdb/config/functions

# determine on what type of system (init style) we are running
detect_init_style()
{
        # only do detection if not already set:
        if [ -n "$CTDB_INIT_STYLE" ] ; then
                return
        fi

 

        if [ -x /sbin/startproc ]; then
                CTDB_INIT_STYLE="suse"
        elif [ -x /sbin/start-stop-daemon ]; then
                CTDB_INIT_STYLE="debian"
        else
                CTDB_INIT_STYLE="redhat"
        fi
}
Comment 1 Martin Schwenke 2023-09-19 07:34:11 UTC
Hi Saurabh,

Thanks for reporting this!

The problem with relying on /etc/redhat-release is that we still have no way to 
detect SUSE, and it probably shouldn't be the default.

What if we do this instead?

detect_init_style()
{
	_init_style_file="${CTDB_SCRIPT_VARDIR}/init-style"

	if [ ! -f "$_init_style_file" ]; then
		if [ -n "$CTDB_INIT_STYLE" ]; then
			echo "$CTDB_INIT_STYLE" >"$_init_style_file"
			return
		fi

		# Subshell to contain variables in os-release file
		(
			_os_release="${CTDB_SYS_ETCDIR}/os-release"
			if [ -f "$_os_release" ]; then
				. "$_os_release"
				case "$ID" in
				centos | fedora | rhel)
					echo "redhat"
					;;
				debian | ubuntu)
					echo "debian"
					;;
				sles | suse)
					echo "suse"
					;;
				*)
					case "$ID_LIKE" in
					*centos* | *rhel*)
						echo "redhat"
						;;
					*)
						echo "unknown"
					esac
				esac
			else
				echo "WARNING: unknown distribution ${ID}" >&2
				echo "unknown"
			fi
		) >"$_init_style_file"
	fi

	read -r CTDB_INIT_STYLE <"$_init_style_file"
}

* This caches the init style, so we only ever check it once
* /etc/os-release seems very standard, so we might as well use it
* The default is "unknown", but a warning is logged

If you think it is OK, can you please test it out and make sure it
works for you?

Should the commit include:

Reported-by: Saurabh Singh <saurabh.singh@veritas.com>

to give you credit?

Thanks again...
Comment 2 Martin Schwenke 2023-09-19 07:40:41 UTC
I don't have an actual RHEL distro, but I don't see a killproc package
on any of the related distros (CentOS, AlmaLinux, Rocky Linux).

Can you please let me know where the killproc package comes from?

Thanks....
Comment 3 Martin Schwenke 2023-09-19 07:42:51 UTC
In my suggested code, we could just make the default "$ID".
Comment 4 saurabh.singh 2023-10-17 08:32:03 UTC
Hi Martin,

Please find the details if killproc rpm below

node-01:~ # rpm -qi killproc-2.21-1.2.x86_64
Name        : killproc
Version     : 2.21
Release     : 1.2
Architecture: x86_64
Install Date: Thu 21 Sep 2023 10:46:34 PM PDT
Group       : Applications/System
Size        : 153297
License     : GPL v2+
Signature   : DSA/SHA1, Sat 29 Sep 2012 01:58:16 AM PDT, Key ID 5f3704290b8f71fe
Source RPM  : killproc-2.21-1.2.src.rpm
Build Date  : Sat 29 Sep 2012 01:57:54 AM PDT
Build Host  : build12
Relocations : (not relocatable)
Vendor      : obs://build.opensuse.org/home:viliampucik
Summary     : killproc and assorted tools for boot scripts
Description :
Some useful programs for a replacment of the shell functions daemon
and killproc found in the Linux System V init suite. killproc(8) for
signaling or terminating, checkproc(8) for checking and startproc(8)
for starting processes. Each program has its own manual page.

node-01:~ # rpm -qf /sbin/startproc
killproc-2.21-1.2.x86_64
Comment 5 saurabh.singh 2023-10-17 08:52:05 UTC
(In reply to Martin Schwenke from comment #1)
Hi Martin,

I tried this on rhel and its working as expected. Are you going to raise a commit request for this fix?
Comment 6 Martin Schwenke 2023-10-17 11:32:41 UTC
(In reply to saurabh.singh from comment #4)

Hi Saurabh,

I'm a little confused.  You say that killproc is now installed on redhat.  However, the package you have given details for is an OpenSUSE package!

Can you please check our assumptions?

I'm happy to improve this code, but I just want to make sure I'm doing for the right reason... and I have the correct reason in my commit message.  ;-)

Thanks...
Comment 7 saurabh.singh 2023-10-20 11:03:28 UTC
Hi Martin,

Some functionality from the killproc rpm was being used by one of the team's, we are reviewing it to see if we can remove dependency on the killproc rpm.
I think the solution suggested by you is a better approach than the one being used currently in detect_init_style().
Comment 8 Martin Schwenke 2023-10-21 06:55:26 UTC
(In reply to saurabh.singh from comment #7)
Hi Saurabh,

I can certainly improve the logic, but I don't think I can
backport this as a bug fix.  The premise of the bug is that
/sbin/startproc gets installed on redhat, but this is only
true in very obscure circumstances (i.e. in your team :-).

So, I'll push a fix for this into the master branch.  After
doing this I'll update this bug with the commit ID, and then
resolve this bug as will not fix.

If you need the patch in an earlier version and you're building
your own Samba packages then you can easily cherry-pick the
improved logic.

Does that sound fair?
Comment 9 saurabh.singh 2023-10-25 07:20:35 UTC
(In reply to Martin Schwenke from comment #8)
Hi Martin,

Yes, that will work for me. I will cherry pick once you merge it to master.

Thanks,
Saurabh
Comment 10 Martin Schwenke 2023-10-30 11:30:25 UTC
Hi Saurabh,

(In reply to saurabh.singh from comment #9)

This just landed in Samba master as 9313731e96c29ac2fa41f5ca4f5ccd2a75d17dc9.

As discussed, won't backport because Red Hat doesn't actually have startproc.  Therefore, resolving this as "won't fix".