Uninstall finds and kills any running elastic-agent watch process (#3384)

* kill watcher on uninstall

* Empty commit.

* Fix killWatcher.

* Empty commit.

* Another fix for killWatcher.

* Empty commit.

* Catch ErrProcessDone.

* Empty commit.

* Empty commit

* Add changelog fragment.

* Make it work on Windows.

* Change killWatcher to be in a loop.

* Add loop to killWatcher.

* Revert "Skip TestStandaloneUpgradeFailsStatus to fix failing integration tests again (#3391)"

This reverts commit bf467e3.

* Revert "Fix integration tests by waiting for the watcher to finish during upgrade tests (#3370)"

This reverts commit 94764be.

* Fix test.

* Revert "Revert "Skip TestStandaloneUpgradeFailsStatus to fix failing integration tests again (#3391)""

This reverts commit 3b0f040.

* Add progress tracking for uninstall like install.

* Log when no watchers our found.

* Improve uninstall.

* Fix data race.

Author: blakerouse

changelog/fragments/1694187216-Uninstall-finds-and-kills-any-running-watcher-process.yaml

@@ -0,0 +1,32 @@
+# 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: Uninstall finds and kills any running watcher process
+
+# 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:
+
+# 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/3384
+
+# 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/3371

internal/pkg/agent/cmd/install.go

@@ -180,37 +180,53 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error {
 	}
 
 	pt := install.NewProgressTracker(streams.Out)
-	pt.Start()
-	defer pt.Stop()
+	s := pt.Start()
+	defer func() {
+		if err != nil {
+			s.Failed()
+		} else {
+			s.Succeeded()
+		}
+	}()
 
 	cfgFile := paths.ConfigFile()
 	if status != install.PackageInstall {
-		err = install.Install(cfgFile, topPath, pt)
+		err = install.Install(cfgFile, topPath, s)
 		if err != nil {
 			return err
 		}
 
 		defer func() {
 			if err != nil {
-				_ = install.Uninstall(cfgFile, topPath, "")
+				uninstallStep := s.StepStart("Uninstalling")
+				innerErr := install.Uninstall(cfgFile, topPath, "", uninstallStep)
+				if innerErr != nil {
+					uninstallStep.Failed()
+				} else {
+					uninstallStep.Succeeded()
+				}
 			}
 		}()
 
 		if !delayEnroll {
-			pt.StepStart("Starting service")
+			startServiceStep := s.StepStart("Starting service")
 			err = install.StartService(topPath)
 			if err != nil {
-				pt.StepFailed()
+				startServiceStep.Failed()
 				fmt.Fprintf(streams.Out, "Installation failed to start Elastic Agent service.\n")
 				return err
 			}
-			pt.StepSucceeded()
+			startServiceStep.Succeeded()
 
 			defer func() {
 				if err != nil {
-					fmt.Fprint(streams.Out, "Stopping service... ")
-					_ = install.StopService(topPath)
-					pt.StepSucceeded()
+					stoppingServiceStep := s.StepStart("Stopping service")
+					innerErr := install.StopService(topPath)
+					if innerErr != nil {
+						stoppingServiceStep.Failed()
+					} else {
+						stoppingServiceStep.Succeeded()
+					}
 				}
 			}()
 		}
@@ -224,25 +240,20 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error {
 		enrollCmd.Stdout = os.Stdout
 		enrollCmd.Stderr = os.Stderr
 
-		fmt.Fprint(streams.Out, "Enrolling Elastic Agent with Fleet... ")
+		enrollStep := s.StepStart("Enrolling Elastic Agent with Fleet")
 		err = enrollCmd.Start()
 		if err != nil {
-			pt.StepFailed()
+			enrollStep.Failed()
 			return fmt.Errorf("failed to execute enroll command: %w", err)
 		}
 		err = enrollCmd.Wait()
 		if err != nil {
-			pt.StepFailed()
-			if status != install.PackageInstall {
-				var exitErr *exec.ExitError
-				_ = install.Uninstall(cfgFile, topPath, "")
-				if err != nil && errors.As(err, &exitErr) {
-					return fmt.Errorf("enroll command failed with exit code: %d", exitErr.ExitCode())
-				}
-			}
+			enrollStep.Failed()
+			// uninstall doesn't need to be performed here the defer above will
+			// catch the error and perform the uninstall
 			return fmt.Errorf("enroll command failed for unknown reason: %w", err)
 		}
-		pt.StepSucceeded()
+		enrollStep.Succeeded()
 	}
 
 	if err := info.CreateInstallMarker(topPath); err != nil {

internal/pkg/agent/cmd/uninstall.go

@@ -80,9 +80,15 @@ func uninstallCmd(streams *cli.IOStreams, cmd *cobra.Command) error {
 		}
 	}
 
-	err = install.Uninstall(paths.ConfigFile(), paths.Top(), uninstallToken)
+	pt := install.NewProgressTracker(streams.Out)
+	s := pt.Start()
+
+	err = install.Uninstall(paths.ConfigFile(), paths.Top(), uninstallToken, s)
 	if err != nil {
+		s.Failed()
 		return err
+	} else {
+		s.Succeeded()
 	}
 	fmt.Fprintf(streams.Out, "Elastic Agent has been uninstalled.\n")
 

internal/pkg/agent/install/install.go

@@ -22,7 +22,7 @@ const (
 )
 
 // Install installs Elastic Agent persistently on the system including creating and starting its service.
-func Install(cfgFile, topPath string, pt *ProgressTracker) error {
+func Install(cfgFile, topPath string, pt ProgressTrackerStep) error {
 	dir, err := findDirectory()
 	if err != nil {
 		return errors.New(err, "failed to discover the source directory for installation", errors.TypeFilesystem)
@@ -33,16 +33,16 @@ func Install(cfgFile, topPath string, pt *ProgressTracker) error {
 	// There is no uninstall token for "install" command.
 	// Uninstall will fail on protected agent.
 	// The protected Agent will need to be uninstalled first before it can be installed.
-	pt.StepStart("Uninstalling current Elastic Agent")
-	err = Uninstall(cfgFile, topPath, "")
+	s := pt.StepStart("Uninstalling current Elastic Agent")
+	err = Uninstall(cfgFile, topPath, "", s)
 	if err != nil {
-		pt.StepFailed()
+		s.Failed()
 		return errors.New(
 			err,
 			fmt.Sprintf("failed to uninstall Agent at (%s)", filepath.Dir(topPath)),
 			errors.M("directory", filepath.Dir(topPath)))
 	}
-	pt.StepSucceeded()
+	s.Succeeded()
 
 	// ensure parent directory exists
 	err = os.MkdirAll(filepath.Dir(topPath), 0755)
@@ -54,21 +54,21 @@ func Install(cfgFile, topPath string, pt *ProgressTracker) error {
 	}
 
 	// copy source into install path
-	pt.StepStart("Copying files")
+	s = pt.StepStart("Copying files")
 	err = copy.Copy(dir, topPath, copy.Options{
 		OnSymlink: func(_ string) copy.SymlinkAction {
 			return copy.Shallow
 		},
 		Sync: true,
 	})
 	if err != nil {
-		pt.StepFailed()
+		s.Failed()
 		return errors.New(
 			err,
 			fmt.Sprintf("failed to copy source directory (%s) to destination (%s)", dir, topPath),
 			errors.M("source", dir), errors.M("destination", topPath))
 	}
-	pt.StepSucceeded()
+	s.Succeeded()
 
 	// place shell wrapper, if present on platform
 	if paths.ShellWrapperPath != "" {
@@ -132,21 +132,21 @@ func Install(cfgFile, topPath string, pt *ProgressTracker) error {
 	}
 
 	// install service
-	pt.StepStart("Installing service")
+	s = pt.StepStart("Installing service")
 	svc, err := newService(topPath)
 	if err != nil {
-		pt.StepFailed()
+		s.Failed()
 		return err
 	}
 	err = svc.Install()
 	if err != nil {
-		pt.StepFailed()
+		s.Failed()
 		return errors.New(
 			err,
 			fmt.Sprintf("failed to install service (%s)", paths.ServiceName),
 			errors.M("service", paths.ServiceName))
 	}
-	pt.StepSucceeded()
+	s.Succeeded()
 
 	return nil
 }