Skip to content

Commit

Permalink
fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@sclevine
sclevine committed Jan 31, 2025
1 parent f9da51f commit f9b1f10
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 85 deletions.
2 changes: 1 addition & 1 deletion ...ata/TestUpdater_Install/no_systemd.golden → ...tall/setup_fails_already_installed.golden
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
version: v1
kind: update_config
spec:
proxy: localhost
proxy: ""
enabled: false
pinned: false
status:
Expand Down
15 changes: 8 additions & 7 deletions lib/autoupdate/agent/updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func NewLocalUpdater(cfg LocalUpdaterConfig, ns *Namespace) (*Updater, error) {
PIDPath: ns.pidFile,
Log: cfg.Log,
},
ExecCheck: func(ctx context.Context, restart bool) error {
ExecSetup: func(ctx context.Context, restart bool) error {
name := ns.updaterBinFile
if cfg.SelfSetup && runtime.GOOS == constants.LinuxOS {
name = "/proc/self/exe"
Expand Down Expand Up @@ -174,8 +174,8 @@ type Updater struct {
Installer Installer
// Process manages a running instance of Teleport.
Process Process
// ExecCheck execs teleport-update to verify and reload the installation.
ExecCheck func(ctx context.Context, restart bool) error
// ExecSetup execs teleport-update to verify and reload the installation.
ExecSetup func(ctx context.Context, restart bool) error
// SetupConfig installs the Teleport updater service using the running installation.
SetupConfig func(ctx context.Context) error
// TeardownConfig removes all traces of the updater and all managed installations.
Expand Down Expand Up @@ -406,10 +406,11 @@ func (u *Updater) Remove(ctx context.Context) error {
// Sync systemd.

err = u.Process.Sync(ctx)
if errors.Is(err, context.Canceled) {
return trace.Errorf("sync canceled")
}
if errors.Is(err, ErrNotSupported) {
u.Log.WarnContext(ctx, "Not syncing systemd configuration because systemd is not running.")
} else if errors.Is(err, context.Canceled) {
return trace.Errorf("sync canceled")
} else if err != nil {
// If sync fails, we may have left the host in a bad state, so we revert linking and re-Sync.
u.Log.ErrorContext(ctx, "Reverting symlinks due to invalid configuration.")
Expand Down Expand Up @@ -718,7 +719,7 @@ func (u *Updater) update(ctx context.Context, cfg *UpdateConfig, target Revision
}

if cfg.Status.Active != target {
err := u.ExecCheck(ctx, true)
err := u.ExecSetup(ctx, true)
if errors.Is(err, context.Canceled) {
return trace.Errorf("check canceled")
}
Expand All @@ -741,7 +742,7 @@ func (u *Updater) update(ctx context.Context, cfg *UpdateConfig, target Revision
}
cfg.Status.Active = target
} else {
err := u.ExecCheck(ctx, false)
err := u.ExecSetup(ctx, false)
if errors.Is(err, context.Canceled) {
return trace.Errorf("check canceled")
}
Expand Down
118 changes: 41 additions & 77 deletions lib/autoupdate/agent/updater_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ func TestUpdater_Update(t *testing.T) {
reloadCalls int
revertCalls int
setupCalls int
restarted bool
errMatch string
}{
{
Expand All @@ -262,8 +263,8 @@ func TestUpdater_Update(t *testing.T) {
installedBaseURL: "https://example.com",
linkedRevision: NewRevision("16.3.0", 0),
requestGroup: "group",
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "updates enabled now",
Expand All @@ -286,8 +287,8 @@ func TestUpdater_Update(t *testing.T) {
installedBaseURL: "https://example.com",
linkedRevision: NewRevision("16.3.0", 0),
requestGroup: "group",
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "updates enabled now, not started or enabled",
Expand All @@ -312,8 +313,8 @@ func TestUpdater_Update(t *testing.T) {
installedBaseURL: "https://example.com",
linkedRevision: NewRevision("16.3.0", 0),
requestGroup: "group",
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "updates disabled during window",
Expand Down Expand Up @@ -444,8 +445,8 @@ func TestUpdater_Update(t *testing.T) {
NewRevision("backup-version", 0),
NewRevision("unknown-version", 0),
},
reloadCalls: 1,
setupCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "backup version kept when no change",
Expand Down Expand Up @@ -490,8 +491,8 @@ func TestUpdater_Update(t *testing.T) {
NewRevision("backup-version", autoupdate.FlagEnterprise|autoupdate.FlagFIPS),
NewRevision("unknown-version", 0),
},
reloadCalls: 1,
setupCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "invalid metadata",
Expand Down Expand Up @@ -521,39 +522,12 @@ func TestUpdater_Update(t *testing.T) {
removedRevisions: []Revision{
NewRevision("backup-version", 0),
},
reloadCalls: 0,
reloadCalls: 1,
revertCalls: 1,
setupCalls: 1,
restarted: true,
errMatch: "setup error",
},
{
name: "reload fails",
cfg: &UpdateConfig{
Version: updateConfigVersion,
Kind: updateConfigKind,
Spec: UpdateSpec{
BaseURL: "https://example.com",
Enabled: true,
},
Status: UpdateStatus{
Active: NewRevision("old-version", 0),
Backup: toPtr(NewRevision("backup-version", 0)),
},
},
inWindow: true,
reloadErr: errors.New("reload error"),

installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: "https://example.com",
linkedRevision: NewRevision("16.3.0", 0),
removedRevisions: []Revision{
NewRevision("backup-version", 0),
},
reloadCalls: 2,
revertCalls: 1,
setupCalls: 1,
errMatch: "reload error",
},
{
name: "skip version",
cfg: &UpdateConfig{
Expand Down Expand Up @@ -680,7 +654,9 @@ func TestUpdater_Update(t *testing.T) {
return !tt.notActive, nil
},
}
updater.ExecCheck = func(_ context.Context) error {
var restarted bool
updater.ExecSetup = func(_ context.Context, restart bool) error {
restarted = restart
setupCalls++
return tt.setupErr
}
Expand All @@ -707,6 +683,7 @@ func TestUpdater_Update(t *testing.T) {
require.Equal(t, tt.revertCalls, revertSetupCalls)
require.Equal(t, tt.revertCalls, revertFuncCalls)
require.Equal(t, tt.setupCalls, setupCalls)
require.Equal(t, tt.restarted, restarted)

if tt.cfg == nil {
_, err := os.Stat(cfgPath)
Expand Down Expand Up @@ -1160,6 +1137,7 @@ func TestUpdater_Install(t *testing.T) {
reloadCalls int
revertCalls int
setupCalls int
restarted bool
errMatch string
}{
{
Expand All @@ -1181,8 +1159,8 @@ func TestUpdater_Install(t *testing.T) {
installedBaseURL: "https://example.com",
linkedRevision: NewRevision("16.3.0", 0),
requestGroup: "group",
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "config from user",
Expand Down Expand Up @@ -1210,8 +1188,8 @@ func TestUpdater_Install(t *testing.T) {
installedBaseURL: "https://example.com/new",
linkedRevision: NewRevision("new-version", 0),
requestGroup: "new-group",
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "defaults",
Expand All @@ -1226,8 +1204,8 @@ func TestUpdater_Install(t *testing.T) {
installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "override skip",
Expand All @@ -1243,8 +1221,8 @@ func TestUpdater_Install(t *testing.T) {
installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "insecure URL",
Expand Down Expand Up @@ -1283,8 +1261,8 @@ func TestUpdater_Install(t *testing.T) {
installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 0,
setupCalls: 1,
restarted: false,
},
{
name: "backup version removed on install",
Expand All @@ -1301,8 +1279,8 @@ func TestUpdater_Install(t *testing.T) {
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
removedRevision: NewRevision("backup-version", 0),
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "backup version kept for validation",
Expand All @@ -1318,7 +1296,6 @@ func TestUpdater_Install(t *testing.T) {
installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 0,
setupCalls: 1,
},
{
Expand All @@ -1327,17 +1304,17 @@ func TestUpdater_Install(t *testing.T) {
installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "FIPS and Enterprise flags",
flags: autoupdate.FlagEnterprise | autoupdate.FlagFIPS,
installedRevision: NewRevision("16.3.0", autoupdate.FlagEnterprise|autoupdate.FlagFIPS),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", autoupdate.FlagEnterprise|autoupdate.FlagFIPS),
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
{
name: "invalid metadata",
Expand All @@ -1351,33 +1328,29 @@ func TestUpdater_Install(t *testing.T) {
installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 0,
revertCalls: 1,
setupCalls: 1,
reloadCalls: 1,
restarted: true,
errMatch: "setup error",
},
{
name: "reload fails",
reloadErr: errors.New("reload error"),
name: "setup fails already installed",
cfg: &UpdateConfig{
Version: updateConfigVersion,
Kind: updateConfigKind,
Status: UpdateStatus{
Active: NewRevision("16.3.0", 0),
},
},
setupErr: errors.New("setup error"),

installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 2,
revertCalls: 1,
setupCalls: 1,
errMatch: "reload error",
},
{
name: "no systemd",
reloadErr: ErrNotSupported,
setupErr: ErrNotSupported,

installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 1,
setupCalls: 1,
errMatch: "setup error",
},
{
name: "no need to reload",
Expand All @@ -1386,20 +1359,8 @@ func TestUpdater_Install(t *testing.T) {
installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 1,
setupCalls: 1,
},
{
name: "not present after install",
notPresent: true,

installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 0,
revertCalls: 1,
setupCalls: 1,
errMatch: "cannot find systemd service",
restarted: true,
},
{
name: "not started or enabled",
Expand All @@ -1409,8 +1370,8 @@ func TestUpdater_Install(t *testing.T) {
installedRevision: NewRevision("16.3.0", 0),
installedBaseURL: autoupdate.DefaultBaseURL,
linkedRevision: NewRevision("16.3.0", 0),
reloadCalls: 1,
setupCalls: 1,
restarted: true,
},
}

Expand Down Expand Up @@ -1502,8 +1463,10 @@ func TestUpdater_Install(t *testing.T) {
return !tt.notActive, nil
},
}
updater.ExecCheck = func(_ context.Context) error {
var restarted bool
updater.ExecSetup = func(_ context.Context, restart bool) error {
setupCalls++
restarted = restart
return tt.setupErr
}
updater.SetupConfig = func(_ context.Context) error {
Expand All @@ -1529,6 +1492,7 @@ func TestUpdater_Install(t *testing.T) {
require.Equal(t, tt.revertCalls, revertSetupCalls)
require.Equal(t, tt.revertCalls, revertFuncCalls)
require.Equal(t, tt.setupCalls, setupCalls)
require.Equal(t, tt.restarted, restarted)

if tt.cfg == nil && err != nil {
_, err := os.Stat(cfgPath)
Expand Down

0 comments on commit f9b1f10

Please sign in to comment.