4 Commits

Author SHA1 Message Date
da8db0d3a3 Add unit reload support 2026-03-09 16:43:24 -04:00
038fbe1a17 fix(errors): prioritize permission errors over 'does not exist' warnings (#11)
filterErr checked 'does not exist' before 'Interactive authentication
required', so when systemd printed both (common for mask/unmask on
non-installed units as a non-root user), the wrong error was returned.

Reorder checks so permission, bus, and masked errors take priority over
existence warnings. Add tests covering mixed-stderr scenarios.

Also:
- CI: install and start nginx so user + root tests pass
- CI: run tests as both user and root for full coverage
- Bump Go 1.26 → 1.26.1
2026-03-06 11:38:03 -05:00
7253c912ca ci: add test pipeline with codecov, staticcheck, and race detection (#10)
* ci: add test pipeline with codecov, staticcheck, and race detection

- Build/test matrix: Go 1.25 + 1.26
- Race detection enabled for all tests
- Coverage uploaded to Codecov (latest Go only)
- staticcheck lint step
- Go module caching via setup-go

* ci: drop Go 1.25 from matrix (go.mod requires 1.26)
2026-03-05 17:39:38 -05:00
adf3c36632 fix(helpers): return ErrValueNotSet for nonexistent units in GetNumRestarts (#9)
systemd returns NRestarts=0 for units with LoadState=not-found, making
it indistinguishable from a genuinely loaded unit with zero restarts.
GetNumRestarts now checks LoadState when NRestarts is 0 and returns
ErrValueNotSet for units that don't exist, matching GetMemoryUsage
behavior.

Also adds unit tests for filterErr (all stderr error mapping cases)
and HasValidUnitSuffix (all valid unit types + negative cases).

Updates syncthing test expectation from ErrValueNotSet to nil since
loaded-but-inactive units legitimately have NRestarts=0.
2026-03-05 12:26:12 -05:00
12 changed files with 320 additions and 14 deletions

65
.github/workflows/ci.yml vendored Normal file
View File

@@ -0,0 +1,65 @@
name: CI
on:
push:
branches: [master]
pull_request:
branches: [master]
permissions:
contents: read
jobs:
test:
name: Test (Go ${{ matrix.go-version }})
runs-on: ubuntu-latest
strategy:
matrix:
go-version: ["1.26"]
steps:
- uses: actions/checkout@v4
- name: Set up Go ${{ matrix.go-version }}
uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go-version }}
cache: true
- name: Install and start nginx
run: |
sudo apt-get update -qq
sudo apt-get install -y -qq nginx
sudo systemctl start nginx
sudo systemctl enable nginx
- name: Run tests (user)
run: go test -race -coverprofile=coverage-user.out -covermode=atomic ./...
- name: Run tests (root)
run: sudo go test -race -coverprofile=coverage-root.out -covermode=atomic ./...
- name: Upload coverage to Codecov
if: matrix.go-version == '1.26'
uses: codecov/codecov-action@v5
with:
files: coverage-user.out,coverage-root.out
token: ${{ secrets.CODECOV_TOKEN }}
fail_ci_if_error: false
lint:
name: Lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: "1.26"
cache: true
- name: Run staticcheck
uses: dominikh/staticcheck-action@v1
with:
version: latest
install-go: false

View File

@@ -22,6 +22,7 @@ If your system isn't running (or targeting another system running) `systemctl`,
- [x] `systemctl is-enabled` - [x] `systemctl is-enabled`
- [x] `systemctl is-failed` - [x] `systemctl is-failed`
- [x] `systemctl mask` - [x] `systemctl mask`
- [x] `systemctl reload`
- [x] `systemctl restart` - [x] `systemctl restart`
- [x] `systemctl show` - [x] `systemctl show`
- [x] `systemctl start` - [x] `systemctl start`

134
filtererr_test.go Normal file
View File

@@ -0,0 +1,134 @@
package systemctl
import (
"errors"
"testing"
)
func TestFilterErr(t *testing.T) {
tests := []struct {
name string
stderr string
want error
}{
{
name: "empty stderr",
stderr: "",
want: nil,
},
{
name: "unit does not exist",
stderr: "Unit foo.service does not exist, proceeding anyway.",
want: ErrDoesNotExist,
},
{
name: "unit not found",
stderr: "Unit foo.service not found.",
want: ErrDoesNotExist,
},
{
name: "unit not loaded",
stderr: "Unit foo.service not loaded.",
want: ErrUnitNotLoaded,
},
{
name: "no such file or directory",
stderr: "No such file or directory",
want: ErrDoesNotExist,
},
{
name: "interactive authentication required",
stderr: "Interactive authentication required.",
want: ErrInsufficientPermissions,
},
{
name: "access denied",
stderr: "Access denied",
want: ErrInsufficientPermissions,
},
{
name: "dbus session bus address",
stderr: "Failed to connect to bus: $DBUS_SESSION_BUS_ADDRESS not set",
want: ErrBusFailure,
},
{
name: "unit is masked",
stderr: "Unit foo.service is masked.",
want: ErrMasked,
},
{
name: "generic failed",
stderr: "Failed to do something unknown",
want: ErrUnspecified,
},
{
name: "does not exist with auth required prioritizes permission error",
stderr: "Unit nginx.service does not exist, proceeding anyway.\nFailed to mask unit: Interactive authentication required.",
want: ErrInsufficientPermissions,
},
{
name: "does not exist with access denied prioritizes permission error",
stderr: "Unit foo.service does not exist, proceeding anyway.\nAccess denied",
want: ErrInsufficientPermissions,
},
{
name: "does not exist with bus failure prioritizes bus error",
stderr: "Unit foo.service does not exist, proceeding anyway.\n$DBUS_SESSION_BUS_ADDRESS not set",
want: ErrBusFailure,
},
{
name: "unrecognized warning",
stderr: "Warning: something benign happened",
want: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := filterErr(tt.stderr)
if tt.want == nil {
if got != nil {
t.Errorf("filterErr(%q) = %v, want nil", tt.stderr, got)
}
return
}
if !errors.Is(got, tt.want) {
t.Errorf("filterErr(%q) = %v, want error wrapping %v", tt.stderr, got, tt.want)
}
})
}
}
func TestHasValidUnitSuffix(t *testing.T) {
tests := []struct {
unit string
want bool
}{
{"nginx.service", true},
{"sshd.socket", true},
{"backup.timer", true},
{"dev-sda1.device", true},
{"home.mount", true},
{"dev-sda1.swap", true},
{"user.slice", true},
{"multi-user.target", true},
{"session-1.scope", true},
{"foo.automount", true},
{"backup.path", true},
{"foo.snapshot", true},
{"nginx", false},
{"", false},
{"foo.bar", false},
{"foo.services", false},
{".service", true},
}
for _, tt := range tests {
t.Run(tt.unit, func(t *testing.T) {
got := HasValidUnitSuffix(tt.unit)
if got != tt.want {
t.Errorf("HasValidUnitSuffix(%q) = %v, want %v", tt.unit, got, tt.want)
}
})
}
}

2
go.mod
View File

@@ -1,3 +1,3 @@
module github.com/taigrr/systemctl module github.com/taigrr/systemctl
go 1.26 go 1.26.1

View File

@@ -32,7 +32,23 @@ func GetNumRestarts(ctx context.Context, unit string, opts Options) (int, error)
if err != nil { if err != nil {
return -1, err return -1, err
} }
return strconv.Atoi(value) if value == "[not set]" {
return -1, ErrValueNotSet
}
restarts, err := strconv.Atoi(value)
if err != nil {
return -1, err
}
// systemd returns NRestarts=0 for both genuinely zero-restart units and
// nonexistent/unloaded units. Disambiguate by checking LoadState: if the
// unit isn't loaded, the value is meaningless.
if restarts == 0 {
loadState, loadErr := Show(ctx, unit, properties.LoadState, opts)
if loadErr == nil && loadState == "not-found" {
return -1, ErrValueNotSet
}
}
return restarts, nil
} }
// Get current memory in bytes (`systemctl show [unit] --property MemoryCurrent`) as an int // Get current memory in bytes (`systemctl show [unit] --property MemoryCurrent`) as an int

View File

@@ -105,8 +105,8 @@ func TestGetNumRestarts(t *testing.T) {
// try nonexistant unit in user mode as user // try nonexistant unit in user mode as user
{"nonexistant", ErrValueNotSet, Options{UserMode: false}, true}, {"nonexistant", ErrValueNotSet, Options{UserMode: false}, true},
// try existing unit in user mode as user // try existing unit in user mode as user (loaded, so NRestarts=0 is valid)
{"syncthing", ErrValueNotSet, Options{UserMode: true}, true}, {"syncthing", nil, Options{UserMode: true}, true},
// try existing unit in system mode as user // try existing unit in system mode as user
{"nginx", nil, Options{UserMode: false}, true}, {"nginx", nil, Options{UserMode: false}, true},

67
reload_linux_test.go Normal file
View File

@@ -0,0 +1,67 @@
//go:build linux
package systemctl
import (
"context"
"os"
"path/filepath"
"reflect"
"strings"
"testing"
"time"
)
func TestReloadBuildsExpectedCommand(t *testing.T) {
tests := []struct {
name string
opts Options
want []string
}{
{
name: "system mode",
opts: Options{},
want: []string{"reload", "--system", "nginx.service"},
},
{
name: "user mode",
opts: Options{UserMode: true},
want: []string{"reload", "--user", "nginx.service"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tempDir := t.TempDir()
logFile := filepath.Join(tempDir, "args.log")
fakeSystemctl := filepath.Join(tempDir, "systemctl")
script := "#!/bin/sh\nprintf '%s\\n' \"$@\" > '" + logFile + "'\n"
if err := os.WriteFile(fakeSystemctl, []byte(script), 0o755); err != nil {
t.Fatalf("write fake systemctl: %v", err)
}
original := systemctl
systemctl = fakeSystemctl
t.Cleanup(func() {
systemctl = original
})
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
if err := Reload(ctx, "nginx.service", tt.opts); err != nil {
t.Fatalf("Reload returned error: %v", err)
}
gotBytes, err := os.ReadFile(logFile)
if err != nil {
t.Fatalf("read captured args: %v", err)
}
got := strings.Fields(strings.TrimSpace(string(gotBytes)))
if !reflect.DeepEqual(got, tt.want) {
t.Fatalf("Reload args = %v, want %v", got, tt.want)
}
})
}
}

View File

@@ -104,6 +104,13 @@ func Restart(ctx context.Context, unit string, opts Options, args ...string) err
return restart(ctx, unit, opts, args...) return restart(ctx, unit, opts, args...)
} }
// Reload one or more units if they support reload.
//
// Any additional arguments are passed directly to the systemctl command.
func Reload(ctx context.Context, unit string, opts Options, args ...string) error {
return reload(ctx, unit, opts, args...)
}
// Show a selected property of a unit. Accepted properties are predefined in the // Show a selected property of a unit. Accepted properties are predefined in the
// properties subpackage to guarantee properties are valid and assist code-completion. // properties subpackage to guarantee properties are valid and assist code-completion.
// //

View File

@@ -44,6 +44,10 @@ func restart(_ context.Context, _ string, _ Options, _ ...string) error {
return nil return nil
} }
func reload(_ context.Context, _ string, _ Options, _ ...string) error {
return nil
}
func show(_ context.Context, _ string, _ properties.Property, _ Options, _ ...string) (string, error) { func show(_ context.Context, _ string, _ properties.Property, _ Options, _ ...string) (string, error) {
return "", nil return "", nil
} }

View File

@@ -115,6 +115,12 @@ func restart(ctx context.Context, unit string, opts Options, args ...string) err
return err return err
} }
func reload(ctx context.Context, unit string, opts Options, args ...string) error {
a := prepareArgs("reload", opts, append([]string{unit}, args...)...)
_, _, _, err := execute(ctx, a)
return err
}
func show(ctx context.Context, unit string, property properties.Property, opts Options, args ...string) (string, error) { func show(ctx context.Context, unit string, property properties.Property, opts Options, args ...string) (string, error) {
extra := append([]string{unit, "--property", string(property)}, args...) extra := append([]string{unit, "--property", string(property)}, args...)
a := prepareArgs("show", opts, extra...) a := prepareArgs("show", opts, extra...)

View File

@@ -290,7 +290,7 @@ func TestMask(t *testing.T) {
// try existing unit in user mode as user // try existing unit in user mode as user
{"syncthing", nil, Options{UserMode: true}, true}, {"syncthing", nil, Options{UserMode: true}, true},
// try nonexisting unit in system mode as user // try nonexisting unit in system mode as user
{"nonexistant", ErrDoesNotExist, Options{UserMode: false}, true}, {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true},
// try existing unit in system mode as user // try existing unit in system mode as user
{"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true}, {"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true},
@@ -521,7 +521,7 @@ func TestUnmask(t *testing.T) {
// try existing unit in user mode as user // try existing unit in user mode as user
{"syncthing", nil, Options{UserMode: true}, true}, {"syncthing", nil, Options{UserMode: true}, true},
// try nonexisting unit in system mode as user // try nonexisting unit in system mode as user
{"nonexistant", ErrDoesNotExist, Options{UserMode: false}, true}, {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true},
// try existing unit in system mode as user // try existing unit in system mode as user
{"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true}, {"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true},

22
util.go
View File

@@ -70,15 +70,13 @@ func prepareArgs(base string, opts Options, extra ...string) []string {
} }
func filterErr(stderr string) error { func filterErr(stderr string) error {
// Order matters: check higher-priority errors first.
// For example, `systemctl mask nginx` as a non-root user on a system
// without nginx prints both "does not exist, proceeding anyway" (a
// warning) and "Interactive authentication required" (the real error).
// Permission and bus errors must be checked before "does not exist" so
// the actual failure reason is returned.
switch { switch {
case strings.Contains(stderr, `does not exist`):
return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `not found.`):
return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `not loaded.`):
return errors.Join(ErrUnitNotLoaded, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `No such file or directory`):
return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `Interactive authentication required`): case strings.Contains(stderr, `Interactive authentication required`):
return errors.Join(ErrInsufficientPermissions, fmt.Errorf("stderr: %s", stderr)) return errors.Join(ErrInsufficientPermissions, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `Access denied`): case strings.Contains(stderr, `Access denied`):
@@ -87,6 +85,14 @@ func filterErr(stderr string) error {
return errors.Join(ErrBusFailure, fmt.Errorf("stderr: %s", stderr)) return errors.Join(ErrBusFailure, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `is masked`): case strings.Contains(stderr, `is masked`):
return errors.Join(ErrMasked, fmt.Errorf("stderr: %s", stderr)) return errors.Join(ErrMasked, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `does not exist`):
return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `not found.`):
return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `not loaded.`):
return errors.Join(ErrUnitNotLoaded, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `No such file or directory`):
return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr))
case strings.Contains(stderr, `Failed`): case strings.Contains(stderr, `Failed`):
return errors.Join(ErrUnspecified, fmt.Errorf("stderr: %s", stderr)) return errors.Join(ErrUnspecified, fmt.Errorf("stderr: %s", stderr))
default: default: