Skip to content

Commit adfa032

Browse files
sclevinezmb3
andauthored
[v17] [teleport-update] Support for CentOS 7 (#53017)
* support systemd down to 219 * comments * Apply suggestions from code review Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com> * Missed check on additional use of IsPresent * adjustments from testing various versions of centos7 * Typo * Use dedicated error for version incompat --------- Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
1 parent 96138d6 commit adfa032

File tree

5 files changed

+139
-56
lines changed

5 files changed

+139
-56
lines changed

lib/autoupdate/agent/process.go

+53-12
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"os"
2828
"os/exec"
2929
"strconv"
30+
"strings"
3031
"syscall"
3132
"time"
3233

@@ -371,15 +372,19 @@ func (s SystemdService) Enable(ctx context.Context, now bool) error {
371372
if err := s.checkSystem(ctx); err != nil {
372373
return trace.Wrap(err)
373374
}
374-
args := []string{"enable", s.ServiceName}
375-
if now {
376-
args = append(args, "--now")
377-
}
378-
code := s.systemctl(ctx, slog.LevelInfo, args...)
375+
// The --now flag is not supported in systemd versions older than 220,
376+
// so perform enable + start commands instead.
377+
code := s.systemctl(ctx, slog.LevelInfo, "enable", s.ServiceName)
379378
if code != 0 {
380379
return trace.Errorf("unable to enable systemd service")
381380
}
382-
s.Log.InfoContext(ctx, "Service enabled.", unitKey, s.ServiceName)
381+
if now {
382+
code := s.systemctl(ctx, slog.LevelInfo, "start", s.ServiceName)
383+
if code != 0 {
384+
return trace.Errorf("unable to start systemd service")
385+
}
386+
}
387+
s.Log.InfoContext(ctx, "Systemd service enabled.", unitKey, s.ServiceName, "now", now)
383388
return nil
384389
}
385390

@@ -388,15 +393,19 @@ func (s SystemdService) Disable(ctx context.Context, now bool) error {
388393
if err := s.checkSystem(ctx); err != nil {
389394
return trace.Wrap(err)
390395
}
391-
args := []string{"disable", s.ServiceName}
392-
if now {
393-
args = append(args, "--now")
394-
}
395-
code := s.systemctl(ctx, slog.LevelInfo, args...)
396+
// The --now flag is not supported in systemd versions older than 220,
397+
// so perform disable + stop commands instead.
398+
code := s.systemctl(ctx, slog.LevelInfo, "disable", s.ServiceName)
396399
if code != 0 {
397400
return trace.Errorf("unable to disable systemd service")
398401
}
399-
s.Log.InfoContext(ctx, "Systemd service disabled.", unitKey, s.ServiceName)
402+
if now {
403+
code := s.systemctl(ctx, slog.LevelInfo, "stop", s.ServiceName)
404+
if code != 0 {
405+
return trace.Errorf("unable to stop systemd service")
406+
}
407+
}
408+
s.Log.InfoContext(ctx, "Systemd service disabled.", unitKey, s.ServiceName, "now", now)
400409
return nil
401410
}
402411

@@ -405,6 +414,9 @@ func (s SystemdService) IsEnabled(ctx context.Context) (bool, error) {
405414
if err := s.checkSystem(ctx); err != nil {
406415
return false, trace.Wrap(err)
407416
}
417+
if hasSystemDBelow(ctx, 238) {
418+
return false, trace.Wrap(ErrNotAvailable)
419+
}
408420
code := s.systemctl(ctx, slog.LevelDebug, "is-enabled", "--quiet", s.ServiceName)
409421
switch {
410422
case code < 0:
@@ -435,6 +447,9 @@ func (s SystemdService) IsPresent(ctx context.Context) (bool, error) {
435447
if err := s.checkSystem(ctx); err != nil {
436448
return false, trace.Wrap(err)
437449
}
450+
if hasSystemDBelow(ctx, 246) {
451+
return false, trace.Wrap(ErrNotAvailable)
452+
}
438453
code := s.systemctl(ctx, slog.LevelDebug, "list-unit-files", "--quiet", s.ServiceName)
439454
if code < 0 {
440455
return false, trace.Errorf("unable to determine if systemd service %s is present", s.ServiceName)
@@ -466,6 +481,32 @@ func hasSystemD() (bool, error) {
466481
return true, nil
467482
}
468483

484+
// hasSystemDBelow returns true the version of systemd can be determined, and it
485+
// is below the provided version.
486+
func hasSystemDBelow(ctx context.Context, i int) bool {
487+
cmd := exec.CommandContext(ctx, "systemctl", "--version")
488+
out, err := cmd.Output()
489+
if err != nil {
490+
return false
491+
}
492+
v, ok := parseSystemDVersion(out)
493+
return ok && v < i
494+
}
495+
496+
// parseSystemDVersion parses the SystemD version from systemctl command output.
497+
func parseSystemDVersion(out []byte) (int, bool) {
498+
first, _, _ := strings.Cut(string(out), "\n")
499+
parts := strings.SplitN(first, " ", 3)
500+
if len(parts) < 2 || parts[0] != "systemd" {
501+
return 0, false
502+
}
503+
version, err := strconv.Atoi(parts[1])
504+
if err != nil {
505+
return 0, false
506+
}
507+
return version, true
508+
}
509+
469510
// systemctl returns a systemctl subcommand, converting the output to logs.
470511
// Output sent to stdout is logged at debug level.
471512
// Output sent to stderr is logged at the level specified by errLevel.

lib/autoupdate/agent/process_test.go

+44
Original file line numberDiff line numberDiff line change
@@ -311,3 +311,47 @@ func TestTickFile(t *testing.T) {
311311
})
312312
}
313313
}
314+
315+
func TestParseSystemdVersion(t *testing.T) {
316+
t.Parallel()
317+
for _, tt := range []struct {
318+
name string
319+
output string
320+
version int
321+
}{
322+
{
323+
name: "valid",
324+
output: "systemd 249 (249.4-1ubuntu1.1)\n+PAM +AUDIT\n",
325+
version: 249,
326+
},
327+
{
328+
name: "short",
329+
output: "systemd 249\n",
330+
version: 249,
331+
},
332+
{
333+
name: "stripped",
334+
output: "systemd 249",
335+
version: 249,
336+
},
337+
{
338+
name: "missing",
339+
output: "systemd",
340+
},
341+
{
342+
name: "bad",
343+
output: "not found",
344+
},
345+
{
346+
name: "empty",
347+
},
348+
} {
349+
t.Run(tt.name, func(t *testing.T) {
350+
v, ok := parseSystemDVersion([]byte(tt.output))
351+
if tt.version == 0 {
352+
require.False(t, ok)
353+
}
354+
require.Equal(t, tt.version, v)
355+
})
356+
}
357+
}

lib/autoupdate/agent/setup.go

+15-3
Original file line numberDiff line numberDiff line change
@@ -233,11 +233,17 @@ func (ns *Namespace) Setup(ctx context.Context, path string) error {
233233
}
234234
// If the old teleport-upgrade script is detected, disable it to ensure they do not interfere.
235235
// Note that the schedule is also set to nop by the Teleport agent -- this just prevents restarts.
236-
enabled, err := isActiveOrEnabled(ctx, oldTimer)
236+
present, err := oldTimer.IsPresent(ctx)
237+
if errors.Is(err, ErrNotAvailable) { // systemd too old
238+
if err := oldTimer.Disable(ctx, true); err != nil {
239+
ns.log.DebugContext(ctx, "The deprecated teleport-ent-updater package is either missing, or could not be disabled.", errorKey, err)
240+
}
241+
return nil
242+
}
237243
if err != nil {
238-
return trace.Wrap(err, "failed to determine if deprecated teleport-upgrade systemd timer is enabled")
244+
return trace.Wrap(err, "failed to determine if deprecated teleport-upgrade systemd timer is present")
239245
}
240-
if enabled {
246+
if present {
241247
if err := oldTimer.Disable(ctx, true); err != nil {
242248
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)
243249
} else {
@@ -288,6 +294,12 @@ func (ns *Namespace) Teardown(ctx context.Context) error {
288294
}
289295
// If the old upgrader exists, attempt to re-enable it automatically
290296
present, err := oldTimer.IsPresent(ctx)
297+
if errors.Is(err, ErrNotAvailable) { // systemd too old
298+
if err := oldTimer.Enable(ctx, true); err != nil {
299+
ns.log.DebugContext(ctx, "The deprecated teleport-ent-updater package is either missing, or could not be enabled.", errorKey, err)
300+
}
301+
return nil
302+
}
291303
if err != nil {
292304
return trace.Wrap(err, "failed to determine if deprecated teleport-upgrade systemd timer is present")
293305
}

lib/autoupdate/agent/updater.go

+15-26
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,8 @@ var (
268268
ErrNotNeeded = errors.New("not needed")
269269
// ErrNotSupported is returned when the operation is not supported on the platform.
270270
ErrNotSupported = errors.New("not supported on this platform")
271+
// ErrNotAvailable is returned when the operation is not available at the current version of the platform.
272+
ErrNotAvailable = errors.New("not available at this version")
271273
// ErrNoBinaries is returned when no binaries are available to be linked.
272274
ErrNoBinaries = errors.New("no binaries available to link")
273275
// ErrFilePresent is returned when a file is present.
@@ -525,7 +527,7 @@ func (u *Updater) removeWithoutSystem(ctx context.Context, cfg *UpdateConfig, fo
525527
u.Log.WarnContext(ctx, "No packaged installation of Teleport was found, and --force was passed. Teleport will be removed from this system.")
526528
}
527529
u.Log.InfoContext(ctx, "Updater-managed installation of Teleport detected. Attempting to unlink and remove.")
528-
ok, err := isActiveOrEnabled(ctx, u.Process)
530+
ok, err := u.Process.IsActive(ctx)
529531
if err != nil && !errors.Is(err, ErrNotSupported) {
530532
return trace.Wrap(err)
531533
}
@@ -543,25 +545,6 @@ func (u *Updater) removeWithoutSystem(ctx context.Context, cfg *UpdateConfig, fo
543545
return nil
544546
}
545547

546-
// isActiveOrEnabled returns true if the service is active or enabled.
547-
func isActiveOrEnabled(ctx context.Context, s Process) (bool, error) {
548-
enabled, err := s.IsEnabled(ctx)
549-
if err != nil {
550-
return false, trace.Wrap(err)
551-
}
552-
if enabled {
553-
return true, nil
554-
}
555-
active, err := s.IsActive(ctx)
556-
if err != nil {
557-
return false, trace.Wrap(err)
558-
}
559-
if active {
560-
return true, nil
561-
}
562-
return false, nil
563-
}
564-
565548
// Status returns all available local and remote fields related to agent auto-updates.
566549
// Status is safe to run concurrently with other Updater commands.
567550
// Status does not write files, and therefore does not require SetRequiredUmask.
@@ -914,10 +897,11 @@ func (u *Updater) Setup(ctx context.Context, path string, restart bool) error {
914897
u.Log.WarnContext(ctx, "Skipping all systemd setup because systemd is not running.")
915898
return nil
916899
}
917-
if err != nil {
900+
if errors.Is(err, ErrNotAvailable) {
901+
u.Log.DebugContext(ctx, "Systemd version is outdated. Skipping SELinux verification.")
902+
} else if err != nil {
918903
return trace.Wrap(err, "failed to determine if new version of Teleport has an installed systemd service")
919-
}
920-
if !present {
904+
} else if !present {
921905
return trace.Errorf("cannot find systemd service for new version of Teleport, check SELinux settings")
922906
}
923907

@@ -944,6 +928,10 @@ func (u *Updater) notices(ctx context.Context) error {
944928
u.Log.WarnContext(ctx, "After configuring teleport.yaml, your system must also be configured to start Teleport.")
945929
return nil
946930
}
931+
if errors.Is(err, ErrNotAvailable) {
932+
u.Log.WarnContext(ctx, "Remember to use systemctl to enable and start Teleport.")
933+
return nil
934+
}
947935
if err != nil {
948936
return trace.Wrap(err, "failed to query Teleport systemd enabled status")
949937
}
@@ -1035,10 +1023,11 @@ func (u *Updater) LinkPackage(ctx context.Context) error {
10351023
return trace.Wrap(err, "failed to sync systemd configuration")
10361024
} else {
10371025
present, err := u.Process.IsPresent(ctx)
1038-
if err != nil {
1026+
if errors.Is(err, ErrNotAvailable) {
1027+
u.Log.DebugContext(ctx, "Systemd version is outdated. Skipping SELinux verification.")
1028+
} else if err != nil {
10391029
return trace.Wrap(err, "failed to determine if Teleport has an installed systemd service")
1040-
}
1041-
if !present {
1030+
} else if !present {
10421031
return trace.Errorf("cannot find systemd service for Teleport, check SELinux settings")
10431032
}
10441033
}

lib/autoupdate/agent/updater_test.go

+12-15
Original file line numberDiff line numberDiff line change
@@ -1050,14 +1050,14 @@ func TestUpdater_Remove(t *testing.T) {
10501050
const version = "active-version"
10511051

10521052
tests := []struct {
1053-
name string
1054-
cfg *UpdateConfig // nil -> file not present
1055-
linkSystemErr error
1056-
isEnabledErr error
1057-
syncErr error
1058-
reloadErr error
1059-
processEnabled bool
1060-
force bool
1053+
name string
1054+
cfg *UpdateConfig // nil -> file not present
1055+
linkSystemErr error
1056+
isActiveErr error
1057+
syncErr error
1058+
reloadErr error
1059+
processActive bool
1060+
force bool
10611061

10621062
unlinkedVersion string
10631063
teardownCalls int
@@ -1093,7 +1093,7 @@ func TestUpdater_Remove(t *testing.T) {
10931093
force: true,
10941094
},
10951095
{
1096-
name: "no system links, process enabled, force",
1096+
name: "no system links, process active, force",
10971097
cfg: &UpdateConfig{
10981098
Version: updateConfigVersion,
10991099
Kind: updateConfigKind,
@@ -1106,7 +1106,7 @@ func TestUpdater_Remove(t *testing.T) {
11061106
},
11071107
linkSystemErr: ErrNoBinaries,
11081108
linkSystemCalls: 1,
1109-
processEnabled: true,
1109+
processActive: true,
11101110
force: true,
11111111
errMatch: "refusing to remove",
11121112
},
@@ -1158,7 +1158,7 @@ func TestUpdater_Remove(t *testing.T) {
11581158
},
11591159
linkSystemErr: ErrNoBinaries,
11601160
linkSystemCalls: 1,
1161-
isEnabledErr: ErrNotSupported,
1161+
isActiveErr: ErrNotSupported,
11621162
unlinkedVersion: version,
11631163
teardownCalls: 1,
11641164
force: true,
@@ -1307,11 +1307,8 @@ func TestUpdater_Remove(t *testing.T) {
13071307
reloadCalls++
13081308
return tt.reloadErr
13091309
},
1310-
FuncIsEnabled: func(_ context.Context) (bool, error) {
1311-
return tt.processEnabled, tt.isEnabledErr
1312-
},
13131310
FuncIsActive: func(_ context.Context) (bool, error) {
1314-
return false, nil
1311+
return tt.processActive, tt.isActiveErr
13151312
},
13161313
}
13171314
updater.TeardownNamespace = func(_ context.Context) error {

0 commit comments

Comments
 (0)