Skip to content

Commit

Permalink
cleanup unused
Browse files Browse the repository at this point in the history
  • Loading branch information
@sclevine
sclevine committed Nov 21, 2024
1 parent 9317c0e commit f5ccbfd
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 22 deletions.
22 changes: 17 additions & 5 deletions lib/autoupdate/agent/installer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down
32 changes: 25 additions & 7 deletions lib/autoupdate/agent/updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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
}
Expand Down
26 changes: 16 additions & 10 deletions lib/autoupdate/agent/updater_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"os"
"path/filepath"
"regexp"
"slices"
"strings"
"testing"

Expand Down Expand Up @@ -136,7 +137,7 @@ func TestUpdater_Update(t *testing.T) {
setupErr error
reloadErr error

removedVersion string
removedVersions []string
installedVersion string
installedTemplate string
linkedVersion string
Expand All @@ -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",
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
},
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit f5ccbfd

Please sign in to comment.