Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
- Allow the - char to appear as part of variable names in eql expressions. {issue}709[709] {pull}710[710]
- Allow the / char in variable names in eql and transpiler. {issue}715[715] {pull}718[718]
- Fix data duplication for standalone agent on Kubernetes using the default manifest {issue-beats}31512[31512] {pull}742[742]
- Agent updates will clean up unneeded artifacts. {issue}693[693] {issue}694[694] {pull}752[752]

==== New features

Expand Down
54 changes: 54 additions & 0 deletions internal/pkg/agent/application/upgrade/cleanup.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

package upgrade

import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/hashicorp/go-multierror"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
)

// preUpgradeCleanup will remove files that do not have the passed version number from the downloads directory.
func preUpgradeCleanup(version string) error {
files, err := os.ReadDir(paths.Downloads())
if err != nil {
return fmt.Errorf("unable to read directory %q: %w", paths.Downloads(), err)
}
var rErr error
for _, file := range files {
if file.IsDir() {
continue
}
if !strings.Contains(file.Name(), version) {
if err := os.Remove(filepath.Join(paths.Downloads(), file.Name())); err != nil {
rErr = multierror.Append(rErr, fmt.Errorf("unable to remove file %q: %w", filepath.Join(paths.Downloads(), file.Name()), err))
}
}
}
return rErr
}

// cleanAllDownloads will remove all files from the downloads directory
func cleanAllDownloads() error {
files, err := os.ReadDir(paths.Downloads())
if err != nil {
return fmt.Errorf("unable to read directory %q: %w", paths.Downloads(), err)
}
var rErr error
for _, file := range files {
if file.IsDir() {
continue
}
if err := os.Remove(filepath.Join(paths.Downloads(), file.Name())); err != nil {
rErr = multierror.Append(rErr, fmt.Errorf("unable to remove file %q: %w", filepath.Join(paths.Downloads(), file.Name()), err))
}
}
return rErr
}
53 changes: 53 additions & 0 deletions internal/pkg/agent/application/upgrade/cleanup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

package upgrade

import (
"os"
"path/filepath"
"testing"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"

"github.com/stretchr/testify/require"
)

func setupDir(t *testing.T) {
t.Helper()
dir := t.TempDir()
paths.SetDownloads(dir)

err := os.WriteFile(filepath.Join(dir, "test-8.3.0-file"), []byte("hello, world!"), 0600)
require.NoError(t, err)
err = os.WriteFile(filepath.Join(dir, "test-8.4.0-file"), []byte("hello, world!"), 0600)
require.NoError(t, err)
err = os.WriteFile(filepath.Join(dir, "test-8.5.0-file"), []byte("hello, world!"), 0600)
require.NoError(t, err)
err = os.WriteFile(filepath.Join(dir, "test-hash-file"), []byte("hello, world!"), 0600)
require.NoError(t, err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit late to the discussion here, but there is an FS Interface at https://pkg.go.dev/io/fs#FS, this allow you mock everything in memory and not have to deal with the weird syncing issues on drives.


func TestPreUpgradeCleanup(t *testing.T) {
setupDir(t)
err := preUpgradeCleanup("8.4.0")
require.NoError(t, err)

files, err := os.ReadDir(paths.Downloads())
require.NoError(t, err)
require.Len(t, files, 1)
require.Equal(t, "test-8.4.0-file", files[0].Name())
p, err := os.ReadFile(filepath.Join(paths.Downloads(), files[0].Name()))
require.NoError(t, err)
require.Equal(t, []byte("hello, world!"), p)
}

func TestCleanAllDownloads(t *testing.T) {
setupDir(t)
err := cleanAllDownloads()
require.NoError(t, err)
files, err := os.ReadDir(paths.Downloads())
require.NoError(t, err)
require.Len(t, files, 0)
}
17 changes: 17 additions & 0 deletions internal/pkg/agent/application/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ func (u *Upgrader) Upgrade(ctx context.Context, a Action, reexecNow bool) (_ ree
"running under control of the systems supervisor")
}

err = preUpgradeCleanup(a.Version())
if err != nil {
u.log.Errorf("Unable to clean downloads dir %q before update: %v", paths.Downloads(), err)
}

if u.caps != nil {
if _, err := u.caps.Apply(a); errors.Is(err, capabilities.ErrBlocked) {
return nil, nil
Expand All @@ -137,6 +142,11 @@ func (u *Upgrader) Upgrade(ctx context.Context, a Action, reexecNow bool) (_ ree
sourceURI := u.sourceURI(a.SourceURI())
archivePath, err := u.downloadArtifact(ctx, a.Version(), sourceURI)
if err != nil {
// Run the same preUpgradeCleanup task to get rid of any newly downloaded files
// This may have an issue if users are upgrading to the same version number.
if dErr := preUpgradeCleanup(a.Version()); dErr != nil {
u.log.Errorf("Unable to remove file after verification failure: %v", dErr)
}
return nil, err
}

Expand Down Expand Up @@ -184,6 +194,13 @@ func (u *Upgrader) Upgrade(ctx context.Context, a Action, reexecNow bool) (_ ree
return nil, nil
}

// Clean everything from the downloads dir
// If we wanted to be a bit simpler we could call os.RemoveAll to remove the dir + children
Copy link
Member

Choose a reason for hiding this comment

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

That sounds like a plan :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, also added the call before the exec now call

err = cleanAllDownloads()
if err != nil {
u.log.Errorf("Unable to clean downloads dir %q after update: %v", paths.Downloads(), err)
}

return cb, nil
}

Expand Down