Skip to content

Commit

Permalink
fix: remove temp dir if installer fails to download (#25392)
Browse files Browse the repository at this point in the history
> For #25373

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Added/updated automated tests
- [x] A detailed QA plan exists on the associated ticket (if it isn't
there, work with the product group's QA engineer to add it)
- [x] Manual QA for all new/changed functionality
- For Orbit and Fleet Desktop changes:
- [x] Orbit runs on macOS, Linux and Windows. Check if the orbit
feature/bugfix should only apply to one platform (`runtime.GOOS`).
- [x] Manual QA must be performed in the three main OSs, macOS, Windows
and Linux.
- [ ] Auto-update manual QA, from released version of component to new
version (see [tools/tuf/test](../tools/tuf/test/README.md)).
  • Loading branch information
jahzielv authored Jan 29, 2025
1 parent e0682ea commit 91fe3e2
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 13 deletions.
2 changes: 2 additions & 0 deletions orbit/changes/25373-tmp-dir-fix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Fixed a bug where fleetd was not properly cleaning up temporary directories during software
installation.
24 changes: 12 additions & 12 deletions orbit/pkg/installer/installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,18 @@ func (r *Runner) installSoftware(ctx context.Context, installID string) (*fleet.
return payload, fmt.Errorf("creating temporary directory: %w", err)
}

// remove tmp directory and installer
defer func() {
removeAllFn := r.removeAllFn
if removeAllFn == nil {
removeAllFn = os.RemoveAll
}
err := removeAllFn(tmpDir)
if err != nil {
log.Err(err)
}
}()

var installerPath string
if installer.SoftwareInstallerURL != nil && installer.SoftwareInstallerURL.URL != "" {
log.Debug().Str("install_id", installID).Msgf("about to download software installer from URL")
Expand All @@ -255,18 +267,6 @@ func (r *Runner) installSoftware(ctx context.Context, installID string) (*fleet.
}
}

// remove tmp directory and installer
defer func() {
removeAllFn := r.removeAllFn
if removeAllFn == nil {
removeAllFn = os.RemoveAll
}
err := removeAllFn(tmpDir)
if err != nil {
log.Err(err)
}
}()

scriptExtension := ".sh"
if runtime.GOOS == "windows" {
scriptExtension = ".ps1"
Expand Down
15 changes: 14 additions & 1 deletion orbit/pkg/installer/installer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,20 @@ func TestInstallerRun(t *testing.T) {
assert.Equal(t, 2, numPostInstallMatches)
})

t.Run("failed installer download", func(t *testing.T) {
resetAll()

oc.downloadInstallerFn = func(installerID uint, downloadDir string) (string, error) {
return "", errors.New("failed to download installer")
}

err := r.run(context.Background(), &config)
require.Error(t, err)

require.True(t, removeAllFnCalled)
require.True(t, tmpDirFnCalled)
require.Equal(t, tmpDir, removedDir)
})
t.Run("failed results upload", func(t *testing.T) {
var retries int
// set a shorter interval to speed up tests
Expand Down Expand Up @@ -669,7 +683,6 @@ func TestInstallerRunWithInstallerFromURL(t *testing.T) {
assert.True(t, getInstallerDetailsFnCalled)
assert.Equal(t, installDetails.ExecutionID, installIdRequested)
})

}

func TestScriptsDisabled(t *testing.T) {
Expand Down

0 comments on commit 91fe3e2

Please sign in to comment.