From 043b1e41e3923890d7836919e49b2258f3cd465d Mon Sep 17 00:00:00 2001 From: Branden Clark Date: Thu, 26 Sep 2024 21:41:28 -0400 Subject: [PATCH] Return success from `RemoveAgent` when the Agent MSI/Product is not installed (#29577) --- .../service/datadog_agent_windows.go | 30 ++++++++------- .../tests/installer/windows/base_suite.go | 13 +++++-- .../suites/agent-package/install_test.go | 37 ++++++++++++++++++- 3 files changed, 61 insertions(+), 19 deletions(-) diff --git a/pkg/fleet/installer/service/datadog_agent_windows.go b/pkg/fleet/installer/service/datadog_agent_windows.go index 5b5b9097cb56f..35284e0bb2351 100644 --- a/pkg/fleet/installer/service/datadog_agent_windows.go +++ b/pkg/fleet/installer/service/datadog_agent_windows.go @@ -31,7 +31,7 @@ func SetupAgent(ctx context.Context, args []string) (err error) { span.Finish(tracer.WithError(err)) }() // Make sure there are no Agent already installed - _ = removeProduct("Datadog Agent") + _ = removeAgentIfInstalled(ctx) err = installAgentPackage("stable", args) return err } @@ -91,17 +91,10 @@ func PromoteAgentExperiment(_ context.Context) error { // RemoveAgent stops and removes the agent func RemoveAgent(ctx context.Context) (err error) { - span, _ := tracer.StartSpanFromContext(ctx, "remove_agent") - defer func() { - if err != nil { - // removal failed, this should rarely happen. - // Rollback might have restored the Agent, but we can't be sure. - log.Errorf("Failed to remove agent: %s", err) - } - span.Finish(tracer.WithError(err)) - }() - err = removeProduct("Datadog Agent") - return err + // Don't return an error if the Agent is already not installed. + // returning an error here will prevent the package from being removed + // from the local repository. + return removeAgentIfInstalled(ctx) } // ConfigureAgent noop @@ -118,9 +111,18 @@ func installAgentPackage(target string, args []string) error { return nil } -func removeAgentIfInstalled(ctx context.Context) error { +func removeAgentIfInstalled(ctx context.Context) (err error) { if isProductInstalled("Datadog Agent") { - err := RemoveAgent(ctx) + span, _ := tracer.StartSpanFromContext(ctx, "remove_agent") + defer func() { + if err != nil { + // removal failed, this should rarely happen. + // Rollback might have restored the Agent, but we can't be sure. + log.Errorf("Failed to remove agent: %s", err) + } + span.Finish(tracer.WithError(err)) + }() + err := removeProduct("Datadog Agent") if err != nil { return err } diff --git a/test/new-e2e/tests/installer/windows/base_suite.go b/test/new-e2e/tests/installer/windows/base_suite.go index 5a3fd22833af5..cde5d2be78610 100644 --- a/test/new-e2e/tests/installer/windows/base_suite.go +++ b/test/new-e2e/tests/installer/windows/base_suite.go @@ -61,6 +61,7 @@ type BaseInstallerSuite struct { currentAgentVersion agentVersion.Version stableInstallerVersion PackageVersion stableAgentVersion PackageVersion + outputDir string } // Installer the Datadog Installer for testing. @@ -115,10 +116,11 @@ func (s *BaseInstallerSuite) SetupSuite() { func (s *BaseInstallerSuite) BeforeTest(suiteName, testName string) { s.BaseSuite.BeforeTest(suiteName, testName) - outputDir, err := runner.GetTestOutputDir(runner.GetProfile(), s.T()) + var err error + s.outputDir, err = runner.GetTestOutputDir(runner.GetProfile(), s.T()) s.Require().NoError(err, "should get output dir") - s.T().Logf("Output dir: %s", outputDir) - s.installer = NewDatadogInstaller(s.Env(), outputDir) + s.T().Logf("Output dir: %s", s.outputDir) + s.installer = NewDatadogInstaller(s.Env(), s.outputDir) } // Require instantiates a suiteAssertions for the current suite. @@ -132,3 +134,8 @@ func (s *BaseInstallerSuite) BeforeTest(suiteName, testName string) { func (s *BaseInstallerSuite) Require() *suiteasserts.SuiteAssertions { return suiteasserts.New(s.BaseSuite.Require(), s) } + +// OutputDir returns the output directory for the test +func (s *BaseInstallerSuite) OutputDir() string { + return s.outputDir +} diff --git a/test/new-e2e/tests/installer/windows/suites/agent-package/install_test.go b/test/new-e2e/tests/installer/windows/suites/agent-package/install_test.go index b318186b53f72..38fd858783940 100644 --- a/test/new-e2e/tests/installer/windows/suites/agent-package/install_test.go +++ b/test/new-e2e/tests/installer/windows/suites/agent-package/install_test.go @@ -7,9 +7,13 @@ package agenttests import ( + "path/filepath" + "github.com/DataDog/datadog-agent/test/new-e2e/pkg/e2e" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/environments/aws/host/windows" installerwindows "github.com/DataDog/datadog-agent/test/new-e2e/tests/installer/windows" + windowsAgent "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common/agent" + "testing" ) @@ -30,7 +34,7 @@ func TestAgentInstalls(t *testing.T) { func (s *testAgentInstallSuite) TestInstallAgentPackage() { s.Run("Install", func() { s.installAgent() - s.Run("Uninstall", s.uninstallAgent) + s.Run("Uninstall", s.removeAgentPackage) }) } @@ -45,7 +49,7 @@ func (s *testAgentInstallSuite) installAgent() { s.Require().Host(s.Env().RemoteHost).HasARunningDatadogAgentService() } -func (s *testAgentInstallSuite) uninstallAgent() { +func (s *testAgentInstallSuite) removeAgentPackage() { // Arrange // Act @@ -54,4 +58,33 @@ func (s *testAgentInstallSuite) uninstallAgent() { // Assert s.Require().NoErrorf(err, "failed to remove the Datadog Agent package: %s", output) s.Require().Host(s.Env().RemoteHost).HasNoDatadogAgentService() + s.Require().Host(s.Env().RemoteHost). + NoDirExists(installerwindows.GetStableDirFor(installerwindows.AgentPackage), + "the package directory should be removed") +} + +func (s *testAgentInstallSuite) TestRemoveAgentAfterMSIUninstall() { + // Arrange + s.installAgent() + s.uninstallAgentWithMSI() + + // Act + + // Assert + s.removeAgentPackage() +} + +func (s *testAgentInstallSuite) uninstallAgentWithMSI() { + // Arrange + + // Act + err := windowsAgent.UninstallAgent(s.Env().RemoteHost, + filepath.Join(s.OutputDir(), "uninstall.log"), + ) + + // Assert + s.Require().NoErrorf(err, "failed to uninstall the Datadog Agent package") + s.Require().Host(s.Env().RemoteHost). + DirExists(installerwindows.GetStableDirFor(installerwindows.AgentPackage), + "the package directory should still exist after manually uninstalling the Agent with the MSI") }