[8.18](backport #6559) Fix/5595 force uninstall correct installation (#…

…6702)

* Fix/5595 force uninstall correct installation (#6559)

* fix(5595): using topPath and binary name to execute uninstall, using service display name for messages

* fix(5595): added unit tests for exeUninstall

* fix(5595): updated comment

* fix(5595): added --force flag to integration tests

* fix(5595): added changelog

* fix(5595): fixed typo

* fix(5595): remove ErrNotExist check

* fix(5595): fix test agent path

(cherry picked from commit 6b9ff26)

# Conflicts:
#	testing/integration/install_test.go

* fix(5595): resolve conflict

---------

Co-authored-by: Kaan Yalti <[email protected]>

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

internal/pkg/agent/cmd/install.go

@@ -245,9 +245,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
@@ -261,6 +261,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)
@@ -309,7 +310,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 {
@@ -324,7 +325,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")
@@ -344,21 +345,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 {

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")
+	})
+}

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

testing/integration/install_test.go

@@ -156,6 +156,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}))
+	})
 
 	// Make sure uninstall from within the topPath fails on Windows
 	if runtime.GOOS == "windows" {
@@ -209,6 +213,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) {
@@ -257,6 +265,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) {
@@ -288,6 +300,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()
@@ -303,6 +320,11 @@ func testInstallWithoutBasePathWithCustomUser(ctx context.Context, t *testing.T,
 	}
 }
 
+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) {
@@ -541,7 +563,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 {