Refactor and fix all upgrade integration tests (#3477)

* Fix all upgrade tests.

* Fix imports and headers.

* Update notice.

* Exclude testing/** from sonar.

* Fix comments from code review.

* Add extra error information in the artifact fetcher.

* Fixes from code review.

* Add upgrade uninstall kill watcher test.

* Remove go replace. Regenerate notice. Fix lint.

* Import WithSourceURI logic. Fix fleet test to not skip if the versions are different and the commits are the same.

* More test fixes.

* Fix imports.

* Re-add TestStandaloneUpgradeFailsWhenUpgradeIsInProgress. Fix code review.

(cherry picked from commit 6201e19)

# Conflicts:
#	sonar-project.properties
#	testing/integration/upgrade_test.go

magefile.go

@@ -1495,7 +1495,11 @@ func (Integration) Local(ctx context.Context, testName string) error {
 	params.Tags = append(params.Tags, "local")
 	params.Packages = []string{"github.com/elastic/elastic-agent/testing/integration"}
 
-	goTestFlags := strings.SplitN(os.Getenv("GOTEST_FLAGS"), " ", -1)
+	var goTestFlags []string
+	rawTestFlags := os.Getenv("GOTEST_FLAGS")
+	if rawTestFlags != "" {
+		goTestFlags = strings.Split(rawTestFlags, " ")
+	}
 	params.ExtraFlags = goTestFlags
 
 	if testName == "all" {

pkg/testing/fetcher_artifact.go

@@ -63,15 +63,19 @@ func (f *artifactFetcher) Fetch(ctx context.Context, operatingSystem string, arc
 	}
 
 	var uri string
+	var prevErr error
 	if !f.snapshotOnly {
-		uri, _ = findURI(ctx, f.doer, version)
+		uri, prevErr = findURI(ctx, f.doer, version)
 	}
 	preVersion := version
+	version, _ = splitBuildID(version)
 	if uri == "" {
-		version = fmt.Sprintf("%s-SNAPSHOT", version)
+		if !strings.HasSuffix(version, "-SNAPSHOT") {
+			version += "-SNAPSHOT"
+		}
 		uri, err = findURI(ctx, f.doer, version)
 		if err != nil {
-			return nil, fmt.Errorf("failed to find snapshot URI for version %s: %w", preVersion, err)
+			return nil, fmt.Errorf("failed to find snapshot URI for version %s: %w (previous error: %w)", preVersion, err, prevErr)
 		}
 	}
 
@@ -112,6 +116,7 @@ func (r *artifactResult) Fetch(ctx context.Context, l Logger, dir string) error
 }
 
 func findURI(ctx context.Context, doer httpDoer, version string) (string, error) {
+	version, buildID := splitBuildID(version)
 	artifactsURI := fmt.Sprintf("https://artifacts-api.elastic.co/v1/search/%s/elastic-agent", version)
 	req, err := http.NewRequestWithContext(ctx, "GET", artifactsURI, nil)
 	if err != nil {
@@ -165,11 +170,36 @@ func findURI(ctx context.Context, doer httpDoer, version string) (string, error)
 		// https://snapshots.elastic.co/8.7.0-d050210c/downloads/elastic-agent-shipper/elastic-agent-shipper-8.7.0-SNAPSHOT-linux-x86_64.tar.gz
 		index := strings.Index(uri, "/beats/elastic-agent/")
 		if index != -1 {
-			return fmt.Sprintf("%s/beats/elastic-agent/", uri[:index]), nil
+			if buildID == "" {
+				// no build id, first is selected
+				return fmt.Sprintf("%s/beats/elastic-agent/", uri[:index]), nil
+			}
+			if strings.Contains(uri, fmt.Sprintf("%s-%s", stripSnapshot(version), buildID)) {
+				return fmt.Sprintf("%s/beats/elastic-agent/", uri[:index]), nil
+			}
 		}
 	}
 
-	return "", fmt.Errorf("uri not detected")
+	if buildID == "" {
+		return "", fmt.Errorf("uri not detected")
+	}
+	return "", fmt.Errorf("uri not detected with specific buildid %s", buildID)
+}
+
+func splitBuildID(version string) (string, string) {
+	split := strings.SplitN(version, "+", 2)
+	if len(split) == 1 {
+		// no build ID
+		return split[0], ""
+	}
+	return split[0], split[1]
+}
+
+func stripSnapshot(version string) string {
+	if strings.HasSuffix(version, "-SNAPSHOT") {
+		return strings.TrimSuffix(version, "-SNAPSHOT")
+	}
+	return version
 }
 
 func DownloadPackage(ctx context.Context, l Logger, doer httpDoer, downloadPath string, packageFile string) error {

pkg/testing/fixture.go

@@ -183,7 +183,7 @@ func (f *Fixture) Prepare(ctx context.Context, components ...UsableComponent) er
 // configuration. This must be called after `Prepare` is called but before `Run`
 // or `Install` can be called.
 func (f *Fixture) Configure(ctx context.Context, yamlConfig []byte) error {
-	err := f.ensurePrepared(ctx)
+	err := f.EnsurePrepared(ctx)
 	if err != nil {
 		return err
 	}
@@ -205,7 +205,7 @@ func (f *Fixture) WorkDir() string {
 
 // SrcPackage returns the location on disk of the elastic agent package used by this fixture.
 func (f *Fixture) SrcPackage(ctx context.Context) (string, error) {
-	err := f.ensurePrepared(ctx)
+	err := f.EnsurePrepared(ctx)
 	if err != nil {
 		return "", err
 	}
@@ -248,7 +248,7 @@ func (f *Fixture) Run(ctx context.Context, states ...State) error {
 	}
 
 	var err error
-	err = f.ensurePrepared(ctx)
+	err = f.EnsurePrepared(ctx)
 	if err != nil {
 		return err
 	}
@@ -350,7 +350,7 @@ func (f *Fixture) Run(ctx context.Context, states ...State) error {
 
 // Exec provides a way of performing subcommand on the prepared Elastic Agent binary.
 func (f *Fixture) Exec(ctx context.Context, args []string, opts ...process.CmdOption) ([]byte, error) {
-	err := f.ensurePrepared(ctx)
+	err := f.EnsurePrepared(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to prepare before exec: %w", err)
 	}
@@ -369,7 +369,7 @@ func (f *Fixture) Exec(ctx context.Context, args []string, opts ...process.CmdOp
 
 // PrepareAgentCommand creates an exec.Cmd ready to execute an elastic-agent command.
 func (f *Fixture) PrepareAgentCommand(ctx context.Context, args []string, opts ...process.CmdOption) (*exec.Cmd, error) {
-	err := f.ensurePrepared(ctx)
+	err := f.EnsurePrepared(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to prepare before exec: %w", err)
 	}
@@ -456,6 +456,23 @@ func (f *Fixture) ExecInspect(ctx context.Context, opts ...process.CmdOption) (A
 	return inspect, err
 }
 
+// ExecVersion executes the version subcommand on the prepared Elastic Agent binary
+// with '--binary-only'. It returns the parsed YAML output.
+func (f *Fixture) ExecVersion(ctx context.Context, opts ...process.CmdOption) (AgentVersionOutput, error) {
+	out, err := f.Exec(ctx, []string{"version", "--binary-only", "--yaml"}, opts...)
+	version := AgentVersionOutput{}
+	if uerr := yaml.Unmarshal(out, &version); uerr != nil {
+		return AgentVersionOutput{},
+			fmt.Errorf("could not unmarshal agent version output: %w",
+				errors.Join(&ExecErr{
+					err:    err,
+					Output: out,
+				}, uerr))
+	}
+
+	return version, err
+}
+
 // IsHealthy returns if the prepared Elastic Agent reports itself as healthy.
 // It returns false, err if it cannot determine the state of the agent.
 // It should work with any 8.6+ agent
@@ -468,7 +485,8 @@ func (f *Fixture) IsHealthy(ctx context.Context, opts ...process.CmdOption) (boo
 	return status.State == int(cproto.State_HEALTHY), nil
 }
 
-func (f *Fixture) ensurePrepared(ctx context.Context) error {
+// EnsurePrepared ensures that the fixture has been prepared.
+func (f *Fixture) EnsurePrepared(ctx context.Context) error {
 	if f.workDir == "" {
 		return f.Prepare(ctx)
 	}
@@ -916,3 +934,23 @@ type AgentInspectOutput struct {
 		Data string `yaml:"data"`
 	} `yaml:"signed"`
 }
+
+type AgentBinaryVersion struct {
+	Version   string `yaml:"version"`
+	Commit    string `yaml:"commit"`
+	BuildTime string `yaml:"build_time"`
+	Snapshot  bool   `yaml:"snapshot"`
+}
+
+// String returns the version string.
+func (v *AgentBinaryVersion) String() string {
+	s := v.Version
+	if v.Snapshot {
+		s += "-SNAPSHOT"
+	}
+	return s
+}
+
+type AgentVersionOutput struct {
+	Binary AgentBinaryVersion `yaml:"binary"`
+}

pkg/testing/fixture_install.go

@@ -141,7 +141,8 @@ func (f *Fixture) Install(ctx context.Context, installOpts *InstallOpts, opts ..
 				"keeping the agent installed will jeopardise other tests")
 		}
 
-		out, err := f.Uninstall(ctx, &UninstallOpts{Force: true, UninstallToken: f.uninstallToken})
+		// don't use current `ctx` as it could be cancelled
+		out, err := f.Uninstall(context.Background(), &UninstallOpts{Force: true, UninstallToken: f.uninstallToken})
 		f.setClient(nil)
 		if err != nil &&
 			(errors.Is(err, ErrNotInstalled) ||

pkg/testing/tools/agents.go

@@ -105,7 +105,7 @@ func GetAgentIDByHostname(client *kibana.Client, policyID, hostname string) (str
 	return agent.Agent.ID, nil
 }
 
-func UpgradeAgent(client *kibana.Client, policyID, version string) error {
+func UpgradeAgent(client *kibana.Client, policyID, version string, force bool) error {
 	hostname, err := os.Hostname()
 	if err != nil {
 		return err
@@ -118,6 +118,7 @@ func UpgradeAgent(client *kibana.Client, policyID, version string) error {
 	upgradeAgentReq := kibana.UpgradeAgentRequest{
 		ID:      agentID,
 		Version: version,
+		Force:   force,
 	}
 	_, err = client.UpgradeAgent(context.Background(), upgradeAgentReq)
 	if err != nil {

sonar-project.properties

@@ -3,7 +3,11 @@ sonar.projectKey=elastic_elastic-agent
 sonar.host.url=https://sonarcloud.io
 
 sonar.sources=.
+<<<<<<< HEAD
 sonar.exclusions=**/*_test.go, .git/**, dev-tools/**, /magefile.go, changelog/**, _meta/**, deploy/**, docs/**, img/**, specs/**, pkg/testing/**
+=======
+sonar.exclusions=**/*_test.go, .git/**, dev-tools/**, /magefile.go, changelog/**, _meta/**, deploy/**, docs/**, img/**, specs/**, pkg/testing/**, pkg/component/fake/**, testing/**, **/mocks/*.go
+>>>>>>> 6201e19a5b (Refactor and fix all upgrade integration tests (#3477))
 sonar.tests=.
 sonar.test.inclusions=**/*_test.go
 

testing/integration/upgrade_broken_package_test.go

@@ -0,0 +1,92 @@
+// 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.
+
+//go:build integration
+
+package integration
+
+import (
+	"context"
+	"fmt"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	atesting "github.com/elastic/elastic-agent/pkg/testing"
+	"github.com/elastic/elastic-agent/pkg/testing/define"
+	"github.com/elastic/elastic-agent/testing/upgradetest"
+	agtversion "github.com/elastic/elastic-agent/version"
+)
+
+func TestUpgradeBrokenPackageVersion(t *testing.T) {
+	define.Require(t, define.Requirements{
+		Local: false, // requires Agent installation
+		Sudo:  true,  // requires Agent installation
+	})
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// Start at the build version as we want to test the retry
+	// logic that is in the build.
+	startFixture, err := define.NewFixture(t, define.Version())
+	require.NoError(t, err)
+
+	// Upgrade to an old build.
+	upgradeToVersion, err := upgradetest.PreviousMinor(ctx, define.Version())
+	require.NoError(t, err)
+	endFixture, err := atesting.NewFixture(
+		t,
+		upgradeToVersion,
+		atesting.WithFetcher(atesting.ArtifactFetcher()),
+	)
+	require.NoError(t, err)
+
+	// Pre-upgrade remove the package version files.
+	preUpgradeHook := func() error {
+		// get rid of the package version files in the installed directory
+		return removePackageVersionFiles(t, startFixture)
+	}
+
+	t.Logf("Testing Elastic Agent upgrade from %s to %s...", define.Version(), upgradeToVersion)
+
+	err = upgradetest.PerformUpgrade(ctx, startFixture, endFixture, t, upgradetest.WithPreUpgradeHook(preUpgradeHook))
+	assert.NoError(t, err)
+}
+
+func removePackageVersionFiles(t *testing.T, f *atesting.Fixture) error {
+	installFS := os.DirFS(f.WorkDir())
+	matches := []string{}
+
+	err := fs.WalkDir(installFS, ".", func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if d.Name() == agtversion.PackageVersionFileName {
+			matches = append(matches, path)
+		}
+		return nil
+	})
+	if err != nil {
+		return err
+	}
+
+	t.Logf("package version files found: %v", matches)
+
+	// the version files should have been removed from the other test, we just make sure
+	for _, m := range matches {
+		vFile := filepath.Join(f.WorkDir(), m)
+		t.Logf("removing package version file %q", vFile)
+		err = os.Remove(vFile)
+		if err != nil {
+			return fmt.Errorf("error removing package version file %q: %w", vFile, err)
+		}
+	}
+	return nil
+}