Fix upgrade tests, so they use correct snapshots (#4191)

TestStandaloneUpgradeRetryDownload,
TestStandaloneUpgradeUninstallKillWatcher were using snapshots with a
matching hash instead of filtering builds by a commit hash.

Author: rdner

pkg/testing/tools/artifacts_api.go

@@ -24,12 +24,15 @@ const (
 	artifactsAPIV1VersionBuildsEndpoint = "v1/versions/%s/builds/"
 	artifactAPIV1BuildDetailsEndpoint   = "v1/versions/%s/builds/%s"
 	// artifactAPIV1SearchVersionPackage = "v1/search/%s/%s"
+
+	artifactElasticAgentProject = "elastic-agent-package"
 )
 
 var (
 	ErrLatestVersionNil        = errors.New("latest version is nil")
 	ErrSnapshotVersionsEmpty   = errors.New("snapshot list is nil")
 	ErrInvalidVersionRetrieved = errors.New("invalid version retrieved from artifact API")
+	ErrBuildNotFound           = errors.New("there are no build that satisfy given conditions")
 
 	ErrBadHTTPStatusCode = errors.New("bad http status code")
 )
@@ -146,13 +149,12 @@ func NewArtifactAPIClient(opts ...ArtifactAPIClientOpt) *ArtifactAPIClient {
 func (aac ArtifactAPIClient) GetVersions(ctx context.Context) (list *VersionList, err error) {
 	joinedURL, err := aac.composeURL(artifactsAPIV1VersionsEndpoint)
 	if err != nil {
-		return
+		return nil, err
 	}
 
 	resp, err := aac.createAndPerformRequest(ctx, joinedURL)
 	if err != nil {
-		err = fmt.Errorf("getting versions: %w", err)
-		return
+		return nil, fmt.Errorf("getting versions: %w", err)
 	}
 
 	defer resp.Body.Close()
@@ -165,32 +167,56 @@ func (aac ArtifactAPIClient) GetVersions(ctx context.Context) (list *VersionList
 func (aac ArtifactAPIClient) GetBuildsForVersion(ctx context.Context, version string) (builds *VersionBuilds, err error) {
 	joinedURL, err := aac.composeURL(fmt.Sprintf(artifactsAPIV1VersionBuildsEndpoint, version))
 	if err != nil {
-		return
+		return nil, err
 	}
 
 	resp, err := aac.createAndPerformRequest(ctx, joinedURL)
 	if err != nil {
-		err = fmt.Errorf("getting builds for version %s: %w", version, err)
-		return
+		return nil, fmt.Errorf("getting builds for version %s: %w", version, err)
 	}
 
 	defer resp.Body.Close()
 	return checkResponseAndUnmarshal[VersionBuilds](resp)
 }
 
+// FindBuild returns a build of the given `version` that does not match the
+// `excludeHash` commit hash. It searches for a matching build from the latest
+// to the oldest, starting the search at the `offset` index in the list of builds.
+// Setting `offset` to 0 includes all builds, 1 skips the latest, and so forth.
+// If there are no builds matching these conditions, returns `ErrBuildNotFound`.
+func (aac ArtifactAPIClient) FindBuild(ctx context.Context, version, excludeHash string, offset int) (buildDetails *BuildDetails, err error) {
+	resp, err := aac.GetBuildsForVersion(ctx, version)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get a list of builds: %w", err)
+	}
+	if len(resp.Builds) < offset+1 {
+		return nil, ErrBuildNotFound
+	}
+	for _, buildID := range resp.Builds[offset:] {
+		details, err := aac.GetBuildDetails(ctx, version, buildID)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get build information for %q: %w", buildID, err)
+		}
+		if details.Build.Projects[artifactElasticAgentProject].CommitHash != excludeHash {
+			return details, nil
+		}
+	}
+
+	return nil, ErrBuildNotFound
+}
+
 // GetBuildDetails returns the list of project and artifacts related to a specific build.
 // Version parameter format follows semver (without build metadata) and buildID format is <major>.<minor>.<patch>-<buildhash> as returned by
 // GetBuildsForVersion()
 func (aac ArtifactAPIClient) GetBuildDetails(ctx context.Context, version string, buildID string) (buildDetails *BuildDetails, err error) {
 	joinedURL, err := aac.composeURL(fmt.Sprintf(artifactAPIV1BuildDetailsEndpoint, version, buildID))
 	if err != nil {
-		return
+		return nil, err
 	}
 
 	resp, err := aac.createAndPerformRequest(ctx, joinedURL)
 	if err != nil {
-		err = fmt.Errorf("getting build details for version %s buildID %s: %w", version, buildID, err)
-		return
+		return nil, fmt.Errorf("getting build details for version %s buildID %s: %w", version, buildID, err)
 	}
 
 	defer resp.Body.Close()

testing/integration/upgrade_downgrade_test.go

@@ -8,6 +8,7 @@ package integration
 
 import (
 	"context"
+	"errors"
 	"strings"
 	"testing"
 	"time"
@@ -23,10 +24,6 @@ import (
 	"github.com/elastic/elastic-agent/testing/upgradetest"
 )
 
-const (
-	artifactElasticAgentProject = "elastic-agent-package"
-)
-
 func TestStandaloneDowngradeToSpecificSnapshotBuild(t *testing.T) {
 	define.Require(t, define.Requirements{
 		Group: Upgrade,
@@ -61,54 +58,24 @@ func TestStandaloneDowngradeToSpecificSnapshotBuild(t *testing.T) {
 	// as the currently running binary (so, we don't have a file system collision).
 	// Multiple builds can have different IDs but the same commit hash.
 	preReleaseVersion := latestSnapshotVersion.VersionWithPrerelease()
-	resp, err := aac.GetBuildsForVersion(ctx, preReleaseVersion)
-	require.NoError(t, err)
-
-	if len(resp.Builds) < 2 {
-		t.Skipf("need at least 2 builds in the version %s", latestSnapshotVersion.VersionWithPrerelease())
-		return
-	}
-
-	t.Logf("found %d builds for version %q", len(resp.Builds), preReleaseVersion)
-
-	t.Logf("looking for a build that does not match the current commit hash %q", startVersion.Binary.Commit)
-	var upgradeVersionString string
-	for _, buildID := range resp.Builds[1:] {
-		details, err := aac.GetBuildDetails(ctx, preReleaseVersion, buildID)
-		require.NoError(t, err)
-		if details.Build.Projects[artifactElasticAgentProject].CommitHash != startVersion.Binary.Commit {
-			upgradeVersionString = buildID
-			break
-		}
-		t.Logf("build %q matches the current commit hash %q, skipping...", buildID, startVersion.Binary.Commit)
-	}
-
-	if upgradeVersionString == "" {
+	buildInfo, err := aac.FindBuild(ctx, preReleaseVersion, startVersion.Binary.Commit, 1)
+	if errors.Is(err, tools.ErrBuildNotFound) {
 		t.Skipf("there is no other build with a non-matching commit hash in the given version %s", latestSnapshotVersion.VersionWithPrerelease())
 		return
 	}
-
-	t.Logf("found build %q available for testing", upgradeVersionString)
-
-	buildFragments := strings.Split(upgradeVersionString, "-")
-	require.Lenf(t, buildFragments, 2, "version %q returned by artifact api is not in format <version>-<buildID>", upgradeVersionString)
-	endParsedVersion := version.NewParsedSemVer(
-		latestSnapshotVersion.Major(),
-		latestSnapshotVersion.Minor(),
-		latestSnapshotVersion.Patch(),
-		latestSnapshotVersion.Prerelease(),
-		buildFragments[1],
-	)
+	require.NoError(t, err)
 
 	// Upgrade to the specific build.
+	t.Logf("found build %q available for testing", buildInfo.Build.BuildID)
+	endVersion := versionWithBuildID(t, latestSnapshotVersion, buildInfo.Build.BuildID)
 	endFixture, err := atesting.NewFixture(
 		t,
-		endParsedVersion.String(),
+		endVersion,
 		atesting.WithFetcher(atesting.ArtifactFetcher()),
 	)
 	require.NoError(t, err)
 
-	t.Logf("Testing Elastic Agent upgrade from %s to %s...", define.Version(), endParsedVersion.String())
+	t.Logf("Testing Elastic Agent upgrade from %s to %s...", define.Version(), endVersion)
 
 	// We pass the upgradetest.WithDisableUpgradeWatcherUpgradeDetailsCheck option here because the endFixture
 	// is fetched from the artifacts API and it may not contain changes in the Upgrade Watcher whose effects are
@@ -117,3 +84,19 @@ func TestStandaloneDowngradeToSpecificSnapshotBuild(t *testing.T) {
 	err = upgradetest.PerformUpgrade(ctx, startFixture, endFixture, t, upgradetest.WithDisableUpgradeWatcherUpgradeDetailsCheck())
 	assert.NoError(t, err)
 }
+
+// versionWithBuildID creates a new parsed version created from the given `initialVersion` with the given `buildID` as build metadata.
+func versionWithBuildID(t *testing.T, initialVersion *version.ParsedSemVer, buildID string) string {
+	buildFragments := strings.Split(buildID, "-")
+	require.Lenf(t, buildFragments, 2, "version %q returned by artifact api is not in format <version>-<buildID>", buildID)
+	result := version.NewParsedSemVer(
+		initialVersion.Major(),
+		initialVersion.Minor(),
+		initialVersion.Patch(),
+		initialVersion.Prerelease(),
+		buildFragments[1],
+	)
+
+	return result.String()
+
+}

testing/integration/upgrade_standalone_retry_test.go

@@ -8,6 +8,7 @@ package integration
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -22,7 +23,9 @@ import (
 
 	atesting "github.com/elastic/elastic-agent/pkg/testing"
 	"github.com/elastic/elastic-agent/pkg/testing/define"
+	"github.com/elastic/elastic-agent/pkg/testing/tools"
 	"github.com/elastic/elastic-agent/pkg/testing/tools/testcontext"
+	"github.com/elastic/elastic-agent/pkg/version"
 	"github.com/elastic/elastic-agent/testing/upgradetest"
 )
 
@@ -38,13 +41,27 @@ func TestStandaloneUpgradeRetryDownload(t *testing.T) {
 
 	// Start at the build version as we want to test the retry
 	// logic that is in the build.
+	startVersion, err := version.ParseVersion(define.Version())
+	require.NoError(t, err)
 	startFixture, err := define.NewFixture(t, define.Version())
 	require.NoError(t, err)
+	startVersionInfo, err := startFixture.ExecVersion(ctx)
+	require.NoError(t, err, "failed to get end agent build version info")
+
+	// Upgrade to an older snapshot build of same version but with a different commit hash
+	aac := tools.NewArtifactAPIClient()
+	buildInfo, err := aac.FindBuild(ctx, startVersion.VersionWithPrerelease(), startVersionInfo.Binary.Commit, 0)
+	if errors.Is(err, tools.ErrBuildNotFound) {
+		t.Skipf("there is no other build with a non-matching commit hash in the given version %s", define.Version())
+		return
+	}
+	require.NoError(t, err)
 
-	// Upgrade to an older snapshot build of same version.
+	t.Logf("found build %q available for testing", buildInfo.Build.BuildID)
+	endVersion := versionWithBuildID(t, startVersion, buildInfo.Build.BuildID)
 	endFixture, err := atesting.NewFixture(
 		t,
-		define.Version(),
+		endVersion,
 		atesting.WithFetcher(atesting.ArtifactFetcher()),
 	)
 	require.NoError(t, err)

testing/integration/upgrade_uninstall_test.go

@@ -20,6 +20,7 @@ import (
 
 	atesting "github.com/elastic/elastic-agent/pkg/testing"
 	"github.com/elastic/elastic-agent/pkg/testing/define"
+	"github.com/elastic/elastic-agent/pkg/testing/tools"
 	"github.com/elastic/elastic-agent/testing/upgradetest"
 )
 
@@ -39,21 +40,34 @@ func TestStandaloneUpgradeUninstallKillWatcher(t *testing.T) {
 	ctx, cancel := testcontext.WithDeadline(t, context.Background(), time.Now().Add(10*time.Minute))
 	defer cancel()
 
+	// Upgrades to build under test.
+	endVersion, err := version.ParseVersion(define.Version())
+	require.NoError(t, err)
+	endFixture, err := define.NewFixture(t, define.Version())
+	require.NoError(t, err)
+	endVersionInfo, err := endFixture.ExecVersion(ctx)
+	require.NoError(t, err, "failed to get end agent build version info")
+
 	// Start on a snapshot build, we want this test to upgrade to our
 	// build to ensure that the uninstall will kill the watcher.
+	// We need a snapshot with a non-matching commit hash to perform the upgrade
+	aac := tools.NewArtifactAPIClient()
+	buildInfo, err := aac.FindBuild(ctx, endVersion.VersionWithPrerelease(), endVersionInfo.Binary.Commit, 0)
+	if errors.Is(err, tools.ErrBuildNotFound) {
+		t.Skipf("there is no other build with a non-matching commit hash in the given version %s", endVersion.VersionWithPrerelease())
+		return
+	}
+	require.NoError(t, err)
+
+	t.Logf("found build %q available for testing", buildInfo.Build.BuildID)
+	startVersion := versionWithBuildID(t, endVersion, buildInfo.Build.BuildID)
 	startFixture, err := atesting.NewFixture(
 		t,
-		define.Version(),
+		startVersion,
 		atesting.WithFetcher(atesting.ArtifactFetcher()),
 	)
 	require.NoError(t, err)
 
-	// Upgrades to build under test.
-	endFixture, err := define.NewFixture(t, define.Version())
-	require.NoError(t, err)
-	endVersionInfo, err := endFixture.ExecVersion(ctx)
-	require.NoError(t, err, "failed to get end agent build version info")
-
 	// Use the post-upgrade hook to bypass the remainder of the PerformUpgrade
 	// because we want to do our own checks for the rollback.
 	var ErrPostExit = errors.New("post exit")