diff --git a/changelog/fragments/1737473147-force-install-replaces-correct-agent.yaml b/changelog/fragments/1737473147-force-install-replaces-correct-agent.yaml new file mode 100644 index 00000000000..108e73cda44 --- /dev/null +++ b/changelog/fragments/1737473147-force-install-replaces-correct-agent.yaml @@ -0,0 +1,30 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: bug-fix + +# Change summary; a 80ish characters long description of the change. +summary: The install command is updated so that if a user installs an agent, while there is already an agent, using the `--force` flag replaces the correct one. + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +#description: + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: "elastic-agent" +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +pr: https://github.com/elastic/elastic-agent/pull/6559 +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +issue: https://github.com/elastic/elastic-agent/issues/5595 diff --git a/internal/pkg/agent/cmd/install.go b/internal/pkg/agent/cmd/install.go index 34ecbb5c1f4..47102c09ae2 100644 --- a/internal/pkg/agent/cmd/install.go +++ b/internal/pkg/agent/cmd/install.go @@ -247,9 +247,9 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { cfgFile := paths.ConfigFile() if status == install.Installed { // Uninstall the agent - progBar.Describe("Uninstalling current Elastic Agent") + progBar.Describe(fmt.Sprintf("Uninstalling current %s", paths.ServiceDisplayName())) if !runUninstallBinary { - err := execUninstall(streams) + err := execUninstall(streams, topPath, paths.BinaryName) if err != nil { progBar.Describe("Uninstall failed") return err @@ -263,6 +263,7 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { } progBar.Describe("Successfully uninstalled Elastic Agent") } + if status != install.PackageInstall { customUser, _ := cmd.Flags().GetString(flagInstallCustomUser) customGroup, _ := cmd.Flags().GetString(flagInstallCustomGroup) @@ -316,7 +317,7 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { }() } - fmt.Fprintln(streams.Out, "Elastic Agent successfully installed, starting enrollment.") + fmt.Fprintf(streams.Out, "%s successfully installed, starting enrollment.\n", paths.ServiceDisplayName()) } if enroll { @@ -331,7 +332,7 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { return err } - progBar.Describe("Enrolling Elastic Agent with Fleet") + progBar.Describe(fmt.Sprintf("Enrolling %s with Fleet", paths.ServiceDisplayName())) err = enrollCmd.Start() if err != nil { progBar.Describe("Failed to Enroll") @@ -351,21 +352,31 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { progBar.Describe("Done") _ = progBar.Finish() _ = progBar.Exit() - fmt.Fprint(streams.Out, "\nElastic Agent has been successfully installed.\n") + fmt.Fprintf(streams.Out, "\n%s has been successfully installed.\n", paths.ServiceDisplayName()) return nil } // execUninstall execs "elastic-agent uninstall --force" from the elastic agent installed on the system (found in PATH) -func execUninstall(streams *cli.IOStreams) error { +func execUninstall(streams *cli.IOStreams, topPath string, binName string) error { args := []string{ "uninstall", "--force", } - execPath, err := exec.LookPath(paths.BinaryName) + + // Using the topPath with binaryName is feasible only because the shell wrapper (linux) does not + // do anything complicated aside from calling the agent binary. If this were + // to change, the implementation here may need to change as well. + binPath := filepath.Join(topPath, binName) + fi, err := os.Stat(binPath) if err != nil { - return fmt.Errorf("unable to find %s on path: %w", paths.BinaryName, err) + return fmt.Errorf("error checking binary path %s: %w", binPath, err) } - uninstall := exec.Command(execPath, args...) + + if fi.IsDir() { + return fmt.Errorf("expected file, found a directory at %s", binPath) + } + + uninstall := exec.Command(binPath, args...) uninstall.Stdout = streams.Out uninstall.Stderr = streams.Err if err := uninstall.Start(); err != nil { diff --git a/internal/pkg/agent/cmd/install_test.go b/internal/pkg/agent/cmd/install_test.go index f383d593185..f483c71c020 100644 --- a/internal/pkg/agent/cmd/install_test.go +++ b/internal/pkg/agent/cmd/install_test.go @@ -7,11 +7,17 @@ package cmd import ( + "bytes" + "io/fs" + "os" + "path/filepath" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/elastic/elastic-agent/internal/pkg/agent/application/paths" + "github.com/elastic/elastic-agent/internal/pkg/agent/errors" "github.com/elastic/elastic-agent/internal/pkg/cli" ) @@ -65,3 +71,63 @@ func TestInvalidBasePath(t *testing.T) { }) } } + +func TestExecUninstall(t *testing.T) { + tmpDir := t.TempDir() + + t.Run("successful uninstall", func(t *testing.T) { + binPath := filepath.Join(tmpDir, "elastic-agent") + err := os.WriteFile(binPath, []byte("#!/bin/sh\nexit 0"), 0755) + require.NoError(t, err) + + var stdout, stderr bytes.Buffer + streams := &cli.IOStreams{ + Out: &stdout, + Err: &stderr, + } + + err = execUninstall(streams, tmpDir, "elastic-agent") + assert.NoError(t, err) + }) + + t.Run("binary not found", func(t *testing.T) { + streams := &cli.IOStreams{ + Out: &bytes.Buffer{}, + Err: &bytes.Buffer{}, + } + + err := execUninstall(streams, tmpDir, "non-existent-binary") + assert.Error(t, err) + assert.True(t, errors.Is(err, fs.ErrNotExist)) + }) + + t.Run("directory instead of file", func(t *testing.T) { + binPath := filepath.Join(tmpDir, "elastic-agent-dir") + err := os.Mkdir(binPath, 0755) + require.NoError(t, err) + + streams := &cli.IOStreams{ + Out: &bytes.Buffer{}, + Err: &bytes.Buffer{}, + } + + err = execUninstall(streams, tmpDir, "elastic-agent-dir") + assert.Error(t, err) + assert.Contains(t, err.Error(), "expected file, found a directory") + }) + + t.Run("command execution failure", func(t *testing.T) { + binPath := filepath.Join(tmpDir, "failing-agent") + err := os.WriteFile(binPath, []byte("#!/bin/sh\nexit 1"), 0755) + require.NoError(t, err) + + streams := &cli.IOStreams{ + Out: &bytes.Buffer{}, + Err: &bytes.Buffer{}, + } + + err = execUninstall(streams, tmpDir, "failing-agent") + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to uninstall elastic-agent") + }) +} diff --git a/internal/pkg/agent/cmd/uninstall.go b/internal/pkg/agent/cmd/uninstall.go index 26c670f07da..a83df2e6055 100644 --- a/internal/pkg/agent/cmd/uninstall.go +++ b/internal/pkg/agent/cmd/uninstall.go @@ -65,8 +65,8 @@ func uninstallCmd(streams *cli.IOStreams, cmd *cobra.Command) error { skipFleetAudit, _ := cmd.Flags().GetBool("skip-fleet-audit") if status == install.Broken { if !force { - fmt.Fprintf(streams.Out, "Elastic Agent is installed but currently broken: %s\n", reason) - confirm, err := cli.Confirm(fmt.Sprintf("Continuing will uninstall the broken Elastic Agent at %s. Do you want to continue?", paths.Top()), true) + fmt.Fprintf(streams.Out, "%s is installed but currently broken: %s\n", paths.ServiceDisplayName(), reason) + confirm, err := cli.Confirm(fmt.Sprintf("Continuing will uninstall the broken %s at %s. Do you want to continue?", paths.ServiceDisplayName(), paths.Top()), true) if err != nil { return fmt.Errorf("problem reading prompt response") } @@ -76,7 +76,7 @@ func uninstallCmd(streams *cli.IOStreams, cmd *cobra.Command) error { } } else { if !force { - confirm, err := cli.Confirm(fmt.Sprintf("Elastic Agent will be uninstalled from your system at %s. Do you want to continue?", paths.Top()), true) + confirm, err := cli.Confirm(fmt.Sprintf("%s will be uninstalled from your system at %s. Do you want to continue?", paths.ServiceDisplayName(), paths.Top()), true) if err != nil { return fmt.Errorf("problem reading prompt response") } @@ -86,7 +86,7 @@ func uninstallCmd(streams *cli.IOStreams, cmd *cobra.Command) error { } } - progBar := install.CreateAndStartNewSpinner(streams.Out, "Uninstalling Elastic Agent...") + progBar := install.CreateAndStartNewSpinner(streams.Out, fmt.Sprintf("Uninstalling %s...", paths.ServiceDisplayName())) log, logBuff := logger.NewInMemory("uninstall", logp.ConsoleEncoderConfig()) defer func() { @@ -106,7 +106,7 @@ func uninstallCmd(streams *cli.IOStreams, cmd *cobra.Command) error { } _ = progBar.Finish() _ = progBar.Exit() - fmt.Fprintf(streams.Out, "\nElastic Agent has been uninstalled.\n") + fmt.Fprintf(streams.Out, "\n%s has been uninstalled.\n", paths.ServiceDisplayName()) _ = install.RemovePath(paths.Top()) return nil diff --git a/testing/integration/install_test.go b/testing/integration/install_test.go index 7d7da4ad42e..9a446aab132 100644 --- a/testing/integration/install_test.go +++ b/testing/integration/install_test.go @@ -163,6 +163,10 @@ func TestInstallWithBasePath(t *testing.T) { t.Run("check agent package version", testAgentPackageVersion(ctx, fixture, true)) t.Run("check second agent installs with --namespace", testSecondAgentCanInstall(ctx, fixture, basePath, false, opts)) + t.Run("check second agent can be installed again with --namespace --force", testSecondAgentCanInstallWithForce(ctx, fixture, basePath, false, opts)) + t.Run("check the initial agent is still installed and healthy", func(t *testing.T) { + require.NoError(t, installtest.CheckSuccess(ctx, fixture, topPath, &installtest.CheckOpts{Privileged: opts.Privileged})) + }) t.Run("check components set", testComponentsPresence(ctx, fixture, @@ -324,6 +328,10 @@ func TestInstallPrivilegedWithoutBasePath(t *testing.T) { t.Run("check agent package version", testAgentPackageVersion(ctx, fixture, true)) t.Run("check second agent installs with --namespace", testSecondAgentCanInstall(ctx, fixture, "", false, opts)) + t.Run("check second agent can be installed again with --namespace --force", testSecondAgentCanInstallWithForce(ctx, fixture, "", false, opts)) + t.Run("check the initial agent is still installed and healthy", func(t *testing.T) { + require.NoError(t, installtest.CheckSuccess(ctx, fixture, opts.BasePath, &installtest.CheckOpts{Privileged: opts.Privileged})) + }) } func TestInstallPrivilegedWithBasePath(t *testing.T) { @@ -372,6 +380,10 @@ func TestInstallPrivilegedWithBasePath(t *testing.T) { require.NoError(t, installtest.CheckSuccess(ctx, fixture, topPath, &installtest.CheckOpts{Privileged: opts.Privileged})) t.Run("check agent package version", testAgentPackageVersion(ctx, fixture, true)) t.Run("check second agent installs with --develop", testSecondAgentCanInstall(ctx, fixture, randomBasePath, true, opts)) + t.Run("check second agent can be installed again with --develop --force", testSecondAgentCanInstallWithForce(ctx, fixture, randomBasePath, true, opts)) + t.Run("check the initial agent is still installed and healthy", func(t *testing.T) { + require.NoError(t, installtest.CheckSuccess(ctx, fixture, topPath, &installtest.CheckOpts{Privileged: opts.Privileged})) + }) } func testInstallWithoutBasePathWithCustomUser(ctx context.Context, t *testing.T, fixture *atesting.Fixture, customUsername, customGroup string) { @@ -403,6 +415,11 @@ func testInstallWithoutBasePathWithCustomUser(ctx context.Context, t *testing.T, t.Run("check agent package version", testAgentPackageVersion(ctx, fixture, true)) t.Run("check second agent installs with --develop", testSecondAgentCanInstall(ctx, fixture, "", true, opts)) + t.Run("check second agent can be installed again with --develop --force", testSecondAgentCanInstallWithForce(ctx, fixture, "", true, opts)) + t.Run("check the initial agent is still installed and healthy", func(t *testing.T) { + require.NoError(t, installtest.CheckSuccess(ctx, fixture, topPath, checks)) + }) + // Make sure uninstall from within the topPath fails on Windows if runtime.GOOS == "windows" { cwd, err := os.Getwd() @@ -455,6 +472,11 @@ func testComponentsPresence(ctx context.Context, fixture *atesting.Fixture, requ } } +func testSecondAgentCanInstallWithForce(ctx context.Context, fixture *atesting.Fixture, basePath string, develop bool, installOpts atesting.InstallOpts) func(*testing.T) { + installOpts.Force = true + return testSecondAgentCanInstall(ctx, fixture, basePath, develop, installOpts) +} + // Tests that a second agent can be installed in an isolated namespace, using either --develop or --namespace. func testSecondAgentCanInstall(ctx context.Context, fixture *atesting.Fixture, basePath string, develop bool, installOpts atesting.InstallOpts) func(*testing.T) { return func(t *testing.T) { @@ -693,7 +715,7 @@ func TestRepeatedInstallUninstallFleet(t *testing.T) { } func randStr(length int) string { - var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") + letters := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") runes := make([]rune, length) for i := range runes {