Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 53 additions & 12 deletions lib/autoupdate/agent/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"os"
"os/exec"
"strconv"
"strings"
"syscall"
"time"

Expand Down Expand Up @@ -371,15 +372,19 @@ func (s SystemdService) Enable(ctx context.Context, now bool) error {
if err := s.checkSystem(ctx); err != nil {
return trace.Wrap(err)
}
args := []string{"enable", s.ServiceName}
if now {
args = append(args, "--now")
}
code := s.systemctl(ctx, slog.LevelInfo, args...)
// The --now flag is not supported in systemd versions older than 220,
// so perform enable + start commands instead.
code := s.systemctl(ctx, slog.LevelInfo, "enable", s.ServiceName)
Comment on lines +375 to +377
Copy link
Copy Markdown
Contributor

@hugoShaka hugoShaka Mar 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like both RHEL7 and CentOS7 have systemctl enable --now:

$ docker run -it --rm centos:7 bash -c "systemctl enable --help | grep now"
     --now            Start or stop unit in addition to enabling or disabling it
$ [root@hugo-autoupdate-test-rhel7 ec2-user]# cat /etc/redhat-release 
Red Hat Enterprise Linux Server release 7.9 (Maipo)

$ [root@hugo-autoupdate-test-rhel7 ec2-user]# systemctl enable --help | grep "now"
     --now            Start or stop unit in addition to enabling or disabling it

Note: they do both run systemd 219, so I suppose we either misunderstood when --now was introduced, or they did fun backports (likely as we're talking about RedHat).

Do we still want to do this change>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--now was added in 220: https://www.freedesktop.org/software/systemd/man/latest/systemctl.html#--now

Since we're aiming to support a wide range of distros, and older versions of rhel7/centos7 will not have the backport, it seems safer to stick to the spec and check the version for any APIs that have version requirements.

if code != 0 {
return trace.Errorf("unable to enable systemd service")
}
s.Log.InfoContext(ctx, "Service enabled.", unitKey, s.ServiceName)
if now {
code := s.systemctl(ctx, slog.LevelInfo, "start", s.ServiceName)
if code != 0 {
return trace.Errorf("unable to start systemd service")
}
}
s.Log.InfoContext(ctx, "Systemd service enabled.", unitKey, s.ServiceName, "now", now)
return nil
}

Expand All @@ -388,15 +393,19 @@ func (s SystemdService) Disable(ctx context.Context, now bool) error {
if err := s.checkSystem(ctx); err != nil {
return trace.Wrap(err)
}
args := []string{"disable", s.ServiceName}
if now {
args = append(args, "--now")
}
code := s.systemctl(ctx, slog.LevelInfo, args...)
// The --now flag is not supported in systemd versions older than 220,
// so perform disable + stop commands instead.
code := s.systemctl(ctx, slog.LevelInfo, "disable", s.ServiceName)
if code != 0 {
return trace.Errorf("unable to disable systemd service")
}
s.Log.InfoContext(ctx, "Systemd service disabled.", unitKey, s.ServiceName)
if now {
code := s.systemctl(ctx, slog.LevelInfo, "stop", s.ServiceName)
if code != 0 {
return trace.Errorf("unable to stop systemd service")
}
}
s.Log.InfoContext(ctx, "Systemd service disabled.", unitKey, s.ServiceName, "now", now)
return nil
}

Expand All @@ -405,6 +414,9 @@ func (s SystemdService) IsEnabled(ctx context.Context) (bool, error) {
if err := s.checkSystem(ctx); err != nil {
return false, trace.Wrap(err)
}
if hasSystemDBelow(ctx, 238) {
return false, trace.Wrap(ErrNotAvailable)
}
code := s.systemctl(ctx, slog.LevelDebug, "is-enabled", "--quiet", s.ServiceName)
switch {
case code < 0:
Expand Down Expand Up @@ -435,6 +447,9 @@ func (s SystemdService) IsPresent(ctx context.Context) (bool, error) {
if err := s.checkSystem(ctx); err != nil {
return false, trace.Wrap(err)
}
if hasSystemDBelow(ctx, 246) {
return false, trace.Wrap(ErrNotAvailable)
}
code := s.systemctl(ctx, slog.LevelDebug, "list-unit-files", "--quiet", s.ServiceName)
if code < 0 {
return false, trace.Errorf("unable to determine if systemd service %s is present", s.ServiceName)
Expand Down Expand Up @@ -466,6 +481,32 @@ func hasSystemD() (bool, error) {
return true, nil
}

// hasSystemDBelow returns true the version of systemd can be determined, and it
// is below the provided version.
func hasSystemDBelow(ctx context.Context, i int) bool {
cmd := exec.CommandContext(ctx, "systemctl", "--version")
out, err := cmd.Output()
if err != nil {
return false
}
v, ok := parseSystemDVersion(out)
return ok && v < i
}

// parseSystemDVersion parses the SystemD version from systemctl command output.
func parseSystemDVersion(out []byte) (int, bool) {
first, _, _ := strings.Cut(string(out), "\n")
Comment thread
zmb3 marked this conversation as resolved.
parts := strings.SplitN(first, " ", 3)
if len(parts) < 2 || parts[0] != "systemd" {
return 0, false
}
version, err := strconv.Atoi(parts[1])
if err != nil {
return 0, false
}
return version, true
}

// systemctl returns a systemctl subcommand, converting the output to logs.
// Output sent to stdout is logged at debug level.
// Output sent to stderr is logged at the level specified by errLevel.
Expand Down
44 changes: 44 additions & 0 deletions lib/autoupdate/agent/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,3 +311,47 @@ func TestTickFile(t *testing.T) {
})
}
}

func TestParseSystemdVersion(t *testing.T) {
t.Parallel()
for _, tt := range []struct {
name string
output string
version int
}{
{
name: "valid",
output: "systemd 249 (249.4-1ubuntu1.1)\n+PAM +AUDIT\n",
version: 249,
},
{
name: "short",
output: "systemd 249\n",
version: 249,
},
{
name: "stripped",
output: "systemd 249",
version: 249,
},
{
name: "missing",
output: "systemd",
},
{
name: "bad",
output: "not found",
},
{
name: "empty",
},
} {
t.Run(tt.name, func(t *testing.T) {
v, ok := parseSystemDVersion([]byte(tt.output))
if tt.version == 0 {
require.False(t, ok)
}
require.Equal(t, tt.version, v)
})
}
}
18 changes: 15 additions & 3 deletions lib/autoupdate/agent/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,17 @@ func (ns *Namespace) Setup(ctx context.Context, path string) error {
}
// If the old teleport-upgrade script is detected, disable it to ensure they do not interfere.
// Note that the schedule is also set to nop by the Teleport agent -- this just prevents restarts.
enabled, err := isActiveOrEnabled(ctx, oldTimer)
present, err := oldTimer.IsPresent(ctx)
if errors.Is(err, ErrNotAvailable) { // systemd too old
if err := oldTimer.Disable(ctx, true); err != nil {
ns.log.DebugContext(ctx, "The deprecated teleport-ent-updater package is either missing, or could not be disabled.", errorKey, err)
}
return nil
}
if err != nil {
return trace.Wrap(err, "failed to determine if deprecated teleport-upgrade systemd timer is enabled")
return trace.Wrap(err, "failed to determine if deprecated teleport-upgrade systemd timer is present")
}
if enabled {
if present {
if err := oldTimer.Disable(ctx, true); err != nil {
ns.log.ErrorContext(ctx, "The deprecated teleport-ent-updater package is installed on this server, and it cannot be disabled due to an error. You must remove the teleport-ent-updater package after verifying that teleport-update is working.", errorKey, err)
} else {
Expand Down Expand Up @@ -288,6 +294,12 @@ func (ns *Namespace) Teardown(ctx context.Context) error {
}
// If the old upgrader exists, attempt to re-enable it automatically
present, err := oldTimer.IsPresent(ctx)
if errors.Is(err, ErrNotAvailable) { // systemd too old
if err := oldTimer.Enable(ctx, true); err != nil {
ns.log.DebugContext(ctx, "The deprecated teleport-ent-updater package is either missing, or could not be enabled.", errorKey, err)
}
return nil
}
if err != nil {
return trace.Wrap(err, "failed to determine if deprecated teleport-upgrade systemd timer is present")
}
Expand Down
41 changes: 15 additions & 26 deletions lib/autoupdate/agent/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ var (
ErrNotNeeded = errors.New("not needed")
// ErrNotSupported is returned when the operation is not supported on the platform.
ErrNotSupported = errors.New("not supported on this platform")
// ErrNotAvailable is returned when the operation is not available at the current version of the platform.
ErrNotAvailable = errors.New("not available at this version")
// ErrNoBinaries is returned when no binaries are available to be linked.
ErrNoBinaries = errors.New("no binaries available to link")
// ErrFilePresent is returned when a file is present.
Expand Down Expand Up @@ -525,7 +527,7 @@ func (u *Updater) removeWithoutSystem(ctx context.Context, cfg *UpdateConfig, fo
u.Log.WarnContext(ctx, "No packaged installation of Teleport was found, and --force was passed. Teleport will be removed from this system.")
}
u.Log.InfoContext(ctx, "Updater-managed installation of Teleport detected. Attempting to unlink and remove.")
ok, err := isActiveOrEnabled(ctx, u.Process)
ok, err := u.Process.IsActive(ctx)
if err != nil && !errors.Is(err, ErrNotSupported) {
return trace.Wrap(err)
}
Expand All @@ -543,25 +545,6 @@ func (u *Updater) removeWithoutSystem(ctx context.Context, cfg *UpdateConfig, fo
return nil
}

// isActiveOrEnabled returns true if the service is active or enabled.
func isActiveOrEnabled(ctx context.Context, s Process) (bool, error) {
enabled, err := s.IsEnabled(ctx)
if err != nil {
return false, trace.Wrap(err)
}
if enabled {
return true, nil
}
active, err := s.IsActive(ctx)
if err != nil {
return false, trace.Wrap(err)
}
if active {
return true, nil
}
return false, nil
}

// Status returns all available local and remote fields related to agent auto-updates.
// Status is safe to run concurrently with other Updater commands.
// Status does not write files, and therefore does not require SetRequiredUmask.
Expand Down Expand Up @@ -914,10 +897,11 @@ func (u *Updater) Setup(ctx context.Context, path string, restart bool) error {
u.Log.WarnContext(ctx, "Skipping all systemd setup because systemd is not running.")
return nil
}
if err != nil {
if errors.Is(err, ErrNotAvailable) {
u.Log.DebugContext(ctx, "Systemd version is outdated. Skipping SELinux verification.")
} else if err != nil {
return trace.Wrap(err, "failed to determine if new version of Teleport has an installed systemd service")
}
if !present {
} else if !present {
return trace.Errorf("cannot find systemd service for new version of Teleport, check SELinux settings")
}

Expand All @@ -944,6 +928,10 @@ func (u *Updater) notices(ctx context.Context) error {
u.Log.WarnContext(ctx, "After configuring teleport.yaml, your system must also be configured to start Teleport.")
return nil
}
if errors.Is(err, ErrNotAvailable) {
u.Log.WarnContext(ctx, "Remember to use systemctl to enable and start Teleport.")
return nil
}
if err != nil {
return trace.Wrap(err, "failed to query Teleport systemd enabled status")
}
Expand Down Expand Up @@ -1035,10 +1023,11 @@ func (u *Updater) LinkPackage(ctx context.Context) error {
return trace.Wrap(err, "failed to sync systemd configuration")
} else {
present, err := u.Process.IsPresent(ctx)
if err != nil {
if errors.Is(err, ErrNotAvailable) {
u.Log.DebugContext(ctx, "Systemd version is outdated. Skipping SELinux verification.")
} else if err != nil {
return trace.Wrap(err, "failed to determine if Teleport has an installed systemd service")
}
if !present {
} else if !present {
return trace.Errorf("cannot find systemd service for Teleport, check SELinux settings")
}
}
Expand Down
27 changes: 12 additions & 15 deletions lib/autoupdate/agent/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1050,14 +1050,14 @@ func TestUpdater_Remove(t *testing.T) {
const version = "active-version"

tests := []struct {
name string
cfg *UpdateConfig // nil -> file not present
linkSystemErr error
isEnabledErr error
syncErr error
reloadErr error
processEnabled bool
force bool
name string
cfg *UpdateConfig // nil -> file not present
linkSystemErr error
isActiveErr error
syncErr error
reloadErr error
processActive bool
force bool

unlinkedVersion string
teardownCalls int
Expand Down Expand Up @@ -1093,7 +1093,7 @@ func TestUpdater_Remove(t *testing.T) {
force: true,
},
{
name: "no system links, process enabled, force",
name: "no system links, process active, force",
cfg: &UpdateConfig{
Version: updateConfigVersion,
Kind: updateConfigKind,
Expand All @@ -1106,7 +1106,7 @@ func TestUpdater_Remove(t *testing.T) {
},
linkSystemErr: ErrNoBinaries,
linkSystemCalls: 1,
processEnabled: true,
processActive: true,
force: true,
errMatch: "refusing to remove",
},
Expand Down Expand Up @@ -1158,7 +1158,7 @@ func TestUpdater_Remove(t *testing.T) {
},
linkSystemErr: ErrNoBinaries,
linkSystemCalls: 1,
isEnabledErr: ErrNotSupported,
isActiveErr: ErrNotSupported,
unlinkedVersion: version,
teardownCalls: 1,
force: true,
Expand Down Expand Up @@ -1307,11 +1307,8 @@ func TestUpdater_Remove(t *testing.T) {
reloadCalls++
return tt.reloadErr
},
FuncIsEnabled: func(_ context.Context) (bool, error) {
return tt.processEnabled, tt.isEnabledErr
},
FuncIsActive: func(_ context.Context) (bool, error) {
return false, nil
return tt.processActive, tt.isActiveErr
},
}
updater.TeardownNamespace = func(_ context.Context) error {
Expand Down