Skip to content

Commit

Permalink
test: do not wait afterRestartDelay in TestCleanup (#5446)
Browse files Browse the repository at this point in the history
delay after agent restart is performed to allow agent to tear down all the processes
important mainly for windows, as it prevents removing files which are in use.
In TestCleanup there are no processes and we don't need to wait.
Update Cleanup method to accept delay as an arg and defaulting to afterRestartDelay in prod

TestCleanup speed improves depending on the os:

linux: 8s -> 0.2s
darwin 8s -> 0.2s
windows: 60s -> 0.2s
  • Loading branch information
kruskall authored Sep 6, 2024
1 parent 0d121d5 commit 542db76
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
6 changes: 5 additions & 1 deletion internal/pkg/agent/application/upgrade/rollback.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,12 @@ func Rollback(ctx context.Context, log *logger.Logger, c client.Client, topDirPa

// Cleanup removes all artifacts and files related to a specified version.
func Cleanup(log *logger.Logger, topDirPath, currentVersionedHome, currentHash string, removeMarker, keepLogs bool) error {
return cleanup(log, topDirPath, currentVersionedHome, currentHash, removeMarker, keepLogs, afterRestartDelay)
}

func cleanup(log *logger.Logger, topDirPath, currentVersionedHome, currentHash string, removeMarker, keepLogs bool, delay time.Duration) error {
log.Infow("Cleaning up upgrade", "hash", currentHash, "remove_marker", removeMarker)
<-time.After(afterRestartDelay)
<-time.After(delay)

// data directory path
dataDirPath := paths.DataFrom(topDirPath)
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/agent/application/upgrade/rollback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func TestCleanup(t *testing.T) {
require.NoError(t, err, "error loading update marker")
require.NotNil(t, marker, "loaded marker must not be nil")
t.Logf("Loaded update marker %+v", marker)
tt.wantErr(t, Cleanup(testLogger, testTop, marker.VersionedHome, marker.Hash, tt.args.removeMarker, tt.args.keepLogs), fmt.Sprintf("Cleanup(%v, %v, %v, %v)", marker.VersionedHome, marker.Hash, tt.args.removeMarker, tt.args.keepLogs))
tt.wantErr(t, cleanup(testLogger, testTop, marker.VersionedHome, marker.Hash, tt.args.removeMarker, tt.args.keepLogs, 0), fmt.Sprintf("Cleanup(%v, %v, %v, %v)", marker.VersionedHome, marker.Hash, tt.args.removeMarker, tt.args.keepLogs))
tt.checkAfterCleanup(t, testTop)
})
}
Expand Down

1 comment on commit 542db76

@kruskall
Copy link
Member Author

Choose a reason for hiding this comment

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

for reference: windows is actually 80s -> 0.2s

Please sign in to comment.