From 3e877825b91cdde39f2ea94eabdd6275176b7b82 Mon Sep 17 00:00:00 2001 From: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> Date: Tue, 26 Jul 2022 13:17:34 -0700 Subject: [PATCH] Update will cleanup unneeded artifacts. (#752) * Update will cleanup unneeded artifacts. The update process will cleanup unneeded artifacts. When an update starts all artifacts that do not have the current version number in it's name will be removed. If artifact retrieval fails, downloaded artifacts are removed. On a successful upgrade, all contents of the downloads dir will be removed. * Clean up linter warnings * Wrap errors * cleanup tests * Fix passed version * Use os.RemoveAll --- CHANGELOG.next.asciidoc | 1 + .../pkg/agent/application/upgrade/cleanup.go | 36 +++++++++++++++ .../agent/application/upgrade/cleanup_test.go | 44 +++++++++++++++++++ .../pkg/agent/application/upgrade/upgrade.go | 20 +++++++++ 4 files changed, 101 insertions(+) create mode 100644 internal/pkg/agent/application/upgrade/cleanup.go create mode 100644 internal/pkg/agent/application/upgrade/cleanup_test.go diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 75d574a23b5..2361baf73f5 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -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 diff --git a/internal/pkg/agent/application/upgrade/cleanup.go b/internal/pkg/agent/application/upgrade/cleanup.go new file mode 100644 index 00000000000..5e0618dfe78 --- /dev/null +++ b/internal/pkg/agent/application/upgrade/cleanup.go @@ -0,0 +1,36 @@ +// 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 +} diff --git a/internal/pkg/agent/application/upgrade/cleanup_test.go b/internal/pkg/agent/application/upgrade/cleanup_test.go new file mode 100644 index 00000000000..736a9c42b3d --- /dev/null +++ b/internal/pkg/agent/application/upgrade/cleanup_test.go @@ -0,0 +1,44 @@ +// 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) +} + +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) +} diff --git a/internal/pkg/agent/application/upgrade/upgrade.go b/internal/pkg/agent/application/upgrade/upgrade.go index 1d370cb5301..ce811036176 100644 --- a/internal/pkg/agent/application/upgrade/upgrade.go +++ b/internal/pkg/agent/application/upgrade/upgrade.go @@ -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(u.agentInfo.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 @@ -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(u.agentInfo.Version()); dErr != nil { + u.log.Errorf("Unable to remove file after verification failure: %v", dErr) + } return nil, err } @@ -180,10 +190,20 @@ func (u *Upgrader) Upgrade(ctx context.Context, a Action, reexecNow bool) (_ ree cb := shutdownCallback(u.log, paths.Home(), release.Version(), a.Version(), release.TrimCommit(newHash)) if reexecNow { + err = os.RemoveAll(paths.Downloads()) + if err != nil { + u.log.Errorf("Unable to clean downloads dir %q after update: %v", paths.Downloads(), err) + } u.reexec.ReExec(cb) return nil, nil } + // Clean everything from the downloads dir + err = os.RemoveAll(paths.Downloads()) + if err != nil { + u.log.Errorf("Unable to clean downloads dir %q after update: %v", paths.Downloads(), err) + } + return cb, nil }