diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index f03401063a3b8..11646570e3a60 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -102,7 +102,7 @@ func (li *LocalInstaller) Remove(ctx context.Context, version string) error { return trace.Wrap(err) } - linked, err := li.isLinked(versionDir) + linked, err := li.isLinked(filepath.Join(versionDir, "bin")) if err != nil && !errors.Is(err, os.ErrNotExist) { return trace.Errorf("failed to determine if linked: %w", err) } @@ -766,10 +766,22 @@ func (li *LocalInstaller) versionDir(version string) (string, error) { return versionDir, nil } -// isLinked returns true if any binaries or services in versionDir are linked. -// Returns os.ErrNotExist error if the versionDir does not exist. -func (li *LocalInstaller) isLinked(versionDir string) (bool, error) { - binDir := filepath.Join(versionDir, "bin") +// IsLinked returns true if the version is linked or partially linked. +// Returns os.ErrNotExist error if the version does not exist. +// See Installer interface for additional specs. +func (li *LocalInstaller) IsLinked(ctx context.Context, version string) (bool, error) { + versionDir, err := li.versionDir(version) + if err != nil { + return false, trace.Wrap(err) + } + b, err := li.isLinked(filepath.Join(versionDir, "bin")) + return b, trace.Wrap(err) + +} + +// isLinked returns true if any binaries in binDir are linked. +// Returns os.ErrNotExist error if the binDir does not exist. +func (li *LocalInstaller) isLinked(binDir string) (bool, error) { entries, err := os.ReadDir(binDir) if err != nil { return false, trace.Wrap(err) diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 8fa5d34c246c2..0687dcf011e59 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -509,17 +509,18 @@ func (u *Updater) Update(ctx context.Context) error { func (u *Updater) update(ctx context.Context, cfg *UpdateConfig, targetVersion string, flags InstallFlags) error { activeVersion := cfg.Status.ActiveVersion - switch v := cfg.Status.BackupVersion; v { + backupVersion := cfg.Status.BackupVersion + switch backupVersion { case "", targetVersion, activeVersion: default: if targetVersion == activeVersion { // Keep backup version if we are only verifying active version break } - err := u.Installer.Remove(ctx, v) + err := u.Installer.Remove(ctx, backupVersion) if err != nil { // this could happen if it was already removed due to a failed installation - u.Log.WarnContext(ctx, "Failed to remove backup version of Teleport before new install.", errorKey, err, backupVersionKey, v) + u.Log.WarnContext(ctx, "Failed to remove backup version of Teleport before new install.", errorKey, err, backupVersionKey, backupVersion) } } @@ -607,14 +608,31 @@ func (u *Updater) update(ctx context.Context, cfg *UpdateConfig, targetVersion s u.Log.InfoContext(ctx, "Backup version set.", backupVersionKey, v) } - // Check if manual cleanup might be needed. + // Cleanup orphans. versions, err := u.Installer.List(ctx) if err != nil { - return trace.Errorf("failed to list installed versions: %w", err) + u.Log.ErrorContext(ctx, "Failed to read installed versions.", errorKey, err) + return nil } - if n := len(versions); n > 2 { - u.Log.WarnContext(ctx, "More than 2 versions of Teleport installed. Version directory may need cleanup to save space.", "count", n) + if len(versions) < 3 { + return nil + } + u.Log.WarnContext(ctx, "More than two versions of Teleport are installed. Removing unused versions.", "count", len(versions)) + for _, v := range versions { + switch v { + case "", targetVersion, activeVersion, backupVersion: + continue + } + err := u.Installer.Remove(ctx, v) + if errors.Is(err, ErrLinked) { + u.Log.WarnContext(ctx, "Refusing to remove version with orphan links.", "version", v) + continue + } + if err != nil { + u.Log.WarnContext(ctx, "Failed to remove unused version of Teleport.", errorKey, err, "version", v) + } + u.Log.WarnContext(ctx, "Deleted unused version of Teleport.", "version", v) } return nil } diff --git a/lib/autoupdate/agent/updater_test.go b/lib/autoupdate/agent/updater_test.go index 01e6e4980f5de..8e582554f3fe6 100644 --- a/lib/autoupdate/agent/updater_test.go +++ b/lib/autoupdate/agent/updater_test.go @@ -27,6 +27,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "strings" "testing" @@ -136,7 +137,7 @@ func TestUpdater_Update(t *testing.T) { setupErr error reloadErr error - removedVersion string + removedVersions []string installedVersion string installedTemplate string linkedVersion string @@ -162,6 +163,7 @@ func TestUpdater_Update(t *testing.T) { }, inWindow: true, + removedVersions: []string{"unknown-version"}, installedVersion: "16.3.0", installedTemplate: "https://example.com", linkedVersion: "16.3.0", @@ -294,7 +296,7 @@ func TestUpdater_Update(t *testing.T) { installedVersion: "16.3.0", installedTemplate: "https://example.com", linkedVersion: "16.3.0", - removedVersion: "backup-version", + removedVersions: []string{"backup-version", "unknown-version"}, reloadCalls: 1, setupCalls: 1, }, @@ -337,7 +339,7 @@ func TestUpdater_Update(t *testing.T) { installedVersion: "16.3.0", installedTemplate: "https://example.com", linkedVersion: "16.3.0", - removedVersion: "backup-version", + removedVersions: []string{"backup-version", "unknown-version"}, reloadCalls: 1, setupCalls: 1, }, @@ -366,7 +368,7 @@ func TestUpdater_Update(t *testing.T) { installedVersion: "16.3.0", installedTemplate: "https://example.com", linkedVersion: "16.3.0", - removedVersion: "backup-version", + removedVersions: []string{"backup-version"}, reloadCalls: 0, revertCalls: 1, setupCalls: 1, @@ -392,7 +394,7 @@ func TestUpdater_Update(t *testing.T) { installedVersion: "16.3.0", installedTemplate: "https://example.com", linkedVersion: "16.3.0", - removedVersion: "backup-version", + removedVersions: []string{"backup-version"}, reloadCalls: 2, revertCalls: 1, setupCalls: 1, @@ -442,7 +444,7 @@ func TestUpdater_Update(t *testing.T) { installedVersion string installedTemplate string linkedVersion string - removedVersion string + removedVersions []string installedFlags InstallFlags revertFuncCalls int setupCalls int @@ -464,10 +466,14 @@ func TestUpdater_Update(t *testing.T) { }, nil }, FuncList: func(_ context.Context) (versions []string, err error) { - return []string{"old"}, nil + return slices.Compact([]string{ + installedVersion, + tt.cfg.Status.ActiveVersion, + "unknown-version", + }), nil }, FuncRemove: func(_ context.Context, version string) error { - removedVersion = version + removedVersions = append(removedVersions, version) return nil }, } @@ -497,7 +503,7 @@ func TestUpdater_Update(t *testing.T) { require.Equal(t, tt.installedVersion, installedVersion) require.Equal(t, tt.installedTemplate, installedTemplate) require.Equal(t, tt.linkedVersion, linkedVersion) - require.Equal(t, tt.removedVersion, removedVersion) + require.Equal(t, tt.removedVersions, removedVersions) require.Equal(t, tt.flags, installedFlags) require.Equal(t, tt.requestGroup, requestedGroup) require.Equal(t, tt.reloadCalls, reloadCalls) @@ -924,7 +930,7 @@ func TestUpdater_Enable(t *testing.T) { }, nil }, FuncList: func(_ context.Context) (versions []string, err error) { - return []string{"old"}, nil + return []string{}, nil }, FuncRemove: func(_ context.Context, version string) error { removedVersion = version