From 6fc26fbe21b5add22f336f47afd1c504fd3586f2 Mon Sep 17 00:00:00 2001 From: Vikram bir Singh Date: Tue, 28 Jul 2026 20:21:51 -0700 Subject: [PATCH 1/2] fix(initsystem): don't let SysVinit match systemd hosts (#409) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test -d /etc/init.d` succeeds on virtually every systemd distro (kept for LSB compatibility), so SysVinit's probe matched systemd hosts. With the shared registry's move-to-front reorder, a Windows host resolving first demoted Systemd behind SysVinit, so a subsequent systemd Linux host in a mixed Linux+Windows cluster resolved to SysVinit — Service.IsRunning() then probed /etc/init.d/ and returned false for a running service. Require systemd to be absent (stat /run/systemd/system fails) in addition to /etc/init.d existing, mirroring RegisterSystemd's probe, so a systemd host never resolves to SysVinit regardless of factory ordering. Fixes #409 Signed-off-by: Vikram bir Singh --- initsystem/service_ops_test.go | 49 ++++++++++++++++++++++++++++++++++ initsystem/sysvinit.go | 15 ++++++++--- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/initsystem/service_ops_test.go b/initsystem/service_ops_test.go index c1f79b6c..7ff24029 100644 --- a/initsystem/service_ops_test.go +++ b/initsystem/service_ops_test.go @@ -590,6 +590,55 @@ func TestRegistryDetection(t *testing.T) { require.ErrorIs(t, err, initsystem.ErrNoInitSystem) require.NoError(t, mr.NotReceived(rigtest.Contains("openrc"))) }) + + t.Run("SysVinit not detected when systemd is present", func(t *testing.T) { + reg := initsystem.NewRegistry() + initsystem.RegisterSysVinit(reg) + + // A systemd host: /etc/init.d exists (LSB compat) but systemd is present. + // SysVinit must decline so it never shadows systemd. + mr := rigtest.NewMockRunner() + mr.ErrDefault = errExec + mr.AddCommandSuccess(rigtest.Contains("/etc/init.d")) + mr.AddCommandSuccess(rigtest.Contains("stat /run/systemd/system")) + + _, err := reg.Get(mr) + require.ErrorIs(t, err, initsystem.ErrNoInitSystem) + }) + + // Regression test for https://github.com/k0sproject/rig/issues/409: in a mixed + // Linux+Windows cluster, a Windows host resolving first swaps WinSCM to the front + // of the shared factory slice, demoting Systemd behind SysVinit. A subsequently + // resolved systemd Linux host must still resolve to Systemd, not SysVinit. + t.Run("Systemd wins on a systemd host after a Windows host resolved first (issue #409)", func(t *testing.T) { + reg := initsystem.NewRegistry() + // Same order as DefaultRegistry. + initsystem.RegisterSystemd(reg) + initsystem.RegisterOpenRC(reg) + initsystem.RegisterUpstart(reg) + initsystem.RegisterSysVinit(reg) + initsystem.RegisterWinSCM(reg) + initsystem.RegisterRunit(reg) + initsystem.RegisterLaunchd(reg) + + // Windows host resolves first -> WinSCM matches and is moved to the front. + win := rigtest.NewMockRunner() + win.Windows = true + mgr, err := reg.Get(win) + require.NoError(t, err) + assert.IsType(t, &initsystem.WinSCM{}, mgr) + + // Ubuntu-like systemd host: both `stat /run/systemd/system` and + // `test -d /etc/init.d` succeed. Must resolve to Systemd. + ubuntu := rigtest.NewMockRunner() + ubuntu.ErrDefault = errExec + ubuntu.AddCommandSuccess(rigtest.Contains("stat /run/systemd/system")) + ubuntu.AddCommandSuccess(rigtest.Contains("/etc/init.d")) + + mgr, err = reg.Get(ubuntu) + require.NoError(t, err) + assert.IsType(t, initsystem.Systemd{}, mgr) + }) } // Compile-time interface checks. diff --git a/initsystem/sysvinit.go b/initsystem/sysvinit.go index b58f8197..0a6e2a2a 100644 --- a/initsystem/sysvinit.go +++ b/initsystem/sysvinit.go @@ -87,11 +87,20 @@ func (i SysVinit) DisableService(ctx context.Context, h cmd.ContextRunner, s str // RegisterSysVinit registers SysVinit in a repository. func RegisterSysVinit(repo *Registry) { - repo.Register(func(c cmd.ContextRunner) (ServiceManager, bool) { - if c.IsWindows() { + repo.Register(func(runner cmd.ContextRunner) (ServiceManager, bool) { + if runner.IsWindows() { return nil, false } - if c.ExecContext(context.Background(), "test -d /etc/init.d") != nil { + if runner.ExecContext(context.Background(), "test -d /etc/init.d") != nil { + return nil, false + } + // /etc/init.d exists on virtually every systemd distro too (kept for LSB + // compatibility), so the probe above is not specific enough on its own: it + // makes SysVinit match systemd hosts. Require systemd to be ABSENT as well, + // mirroring RegisterSystemd's probe, so a systemd host never resolves to + // SysVinit regardless of factory ordering in the shared registry. + // See https://github.com/k0sproject/rig/issues/409. + if runner.ExecContext(context.Background(), "stat /run/systemd/system") == nil { return nil, false } From 840c26efc164383084e843af3348d16e286f09c3 Mon Sep 17 00:00:00 2001 From: Vikram bir Singh Date: Wed, 29 Jul 2026 14:44:20 -0700 Subject: [PATCH 2/2] test(initsystem): assert SysVinit negative-detection probes actually run The "SysVinit not detected when systemd is present" test asserted only the ErrNoInitSystem outcome, not that the detection probes ran. Assert both the /etc/init.d and `stat /run/systemd/system` probes were received so the test stays specific and cannot pass without exercising the intended logic. Signed-off-by: Vikram bir Singh --- initsystem/service_ops_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/initsystem/service_ops_test.go b/initsystem/service_ops_test.go index 7ff24029..22894bb4 100644 --- a/initsystem/service_ops_test.go +++ b/initsystem/service_ops_test.go @@ -604,6 +604,13 @@ func TestRegistryDetection(t *testing.T) { _, err := reg.Get(mr) require.ErrorIs(t, err, initsystem.ErrNoInitSystem) + + // Assert SysVinit actually exercised both probes: it passed the + // /etc/init.d check and then declined on the systemd-present check. This + // keeps the test specific so it cannot pass without running the intended + // detection logic. + require.NoError(t, mr.Received(rigtest.Contains("/etc/init.d"))) + require.NoError(t, mr.Received(rigtest.Contains("stat /run/systemd/system"))) }) // Regression test for https://github.com/k0sproject/rig/issues/409: in a mixed