Remove the agent version override in tests. (#3563)

* Remove the agent version override in tests.

We now have working 8.12.0 and 8.11.0 snapshots.
* Add failing unit test for upgradeable minor during FF.
* Return 8.11.0-SNAPSHOT as the previous minor.
* Fix broken condition in upgrade rollback test. Use the agent core version in the condition in the policy because the
variable evaluation strips the snapshot suffix.
* Don't auto overwrite sourceURI if it isn't the default.

(cherry picked from commit 9adbac2)

.buildkite/scripts/steps/integration_tests.sh

@@ -5,8 +5,8 @@ source .buildkite/scripts/common.sh
 
 # Override the agent package version using a string with format <major>.<minor>.<patch>
 # NOTE: use only after version bump when the new version is not yet available, for example:
-# OVERRIDE_AGENT_PACKAGE_VERSION="8.10.3"
-OVERRIDE_AGENT_PACKAGE_VERSION="8.10.2"
+# OVERRIDE_AGENT_PACKAGE_VERSION="8.10.3" otherwise OVERRIDE_AGENT_PACKAGE_VERSION="".
+OVERRIDE_AGENT_PACKAGE_VERSION=""
 
 if [[ -n "$OVERRIDE_AGENT_PACKAGE_VERSION" ]]; then
   OVERRIDE_TEST_AGENT_VERSION=${OVERRIDE_AGENT_PACKAGE_VERSION}"-SNAPSHOT"

internal/pkg/agent/application/upgrade/artifact/download/snapshot/downloader.go

@@ -88,6 +88,11 @@ func snapshotConfig(config *artifact.Config, versionOverride *agtversion.ParsedS
 }
 
 func snapshotURI(versionOverride *agtversion.ParsedSemVer, config *artifact.Config) (string, error) {
+	// Respect a non-default source URI even if the version is a snapshot.
+	if config.SourceURI != artifact.DefaultSourceURI {
+		return config.SourceURI, nil
+	}
+
 	// snapshot downloader is used also by the 'localremote' impl in case of agent currently running off a snapshot build:
 	// the 'localremote' downloader does not pass a specific version, implying that we should update to the latest snapshot
 	// build of the same <major>.<minor>.<patch>-SNAPSHOT version

internal/pkg/agent/application/upgrade/artifact/download/snapshot/downloader_test.go

@@ -0,0 +1,27 @@
+// 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.
+
+package snapshot
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact"
+	"github.com/elastic/elastic-agent/pkg/version"
+)
+
+func TestNonDefaultSourceURI(t *testing.T) {
+	version, err := version.ParseVersion("8.12.0-SNAPSHOT")
+	require.NoError(t, err)
+
+	config := artifact.Config{
+		SourceURI: "localhost:1234",
+	}
+	sourceURI, err := snapshotURI(version, &config)
+	require.NoError(t, err)
+	require.Equal(t, config.SourceURI, sourceURI)
+
+}

pkg/testing/tools/artifacts_api.go

@@ -34,21 +34,20 @@ var (
 	ErrBadHTTPStatusCode = errors.New("bad http status code")
 )
 
+type Manifests struct {
+	LastUpdateTime         string `json:"last-update-time"`
+	SecondsSinceLastUpdate int    `json:"seconds-since-last-update"`
+}
+
 type VersionList struct {
-	Versions  []string `json:"versions"`
-	Aliases   []string `json:"aliases"`
-	Manifests struct {
-		LastUpdateTime         string `json:"last-update-time"`
-		SecondsSinceLastUpdate int    `json:"seconds-since-last-update"`
-	} `json:"manifests"`
+	Versions  []string  `json:"versions"`
+	Aliases   []string  `json:"aliases"`
+	Manifests Manifests `json:"manifests"`
 }
 
 type VersionBuilds struct {
-	Builds    []string `json:"builds"`
-	Manifests struct {
-		LastUpdateTime         string `json:"last-update-time"`
-		SecondsSinceLastUpdate int    `json:"seconds-since-last-update"`
-	} `json:"manifests"`
+	Builds    []string  `json:"builds"`
+	Manifests Manifests `json:"manifests"`
 }
 
 type Package struct {
@@ -99,18 +98,12 @@ type Build struct {
 
 type BuildDetails struct {
 	Build     Build
-	Manifests struct {
-		LastUpdateTime         string `json:"last-update-time"`
-		SecondsSinceLastUpdate int    `json:"seconds-since-last-update"`
-	} `json:"manifests"`
+	Manifests Manifests `json:"manifests"`
 }
 
 type SearchPackageResult struct {
 	Packages  map[string]Package `json:"packages"`
-	Manifests struct {
-		LastUpdateTime         string `json:"last-update-time"`
-		SecondsSinceLastUpdate int    `json:"seconds-since-last-update"`
-	} `json:"manifests"`
+	Manifests Manifests          `json:"manifests"`
 }
 
 type httpDoer interface {

testing/integration/upgrade_rollback_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/elastic/elastic-agent/internal/pkg/agent/install"
 	atesting "github.com/elastic/elastic-agent/pkg/testing"
 	"github.com/elastic/elastic-agent/pkg/testing/define"
+	"github.com/elastic/elastic-agent/pkg/version"
 	"github.com/elastic/elastic-agent/testing/upgradetest"
 )
 
@@ -54,6 +55,11 @@ func TestStandaloneUpgradeRollback(t *testing.T) {
 
 	t.Logf("Testing Elastic Agent upgrade from %s to %s...", define.Version(), upgradeToVersion)
 
+	// We need to use the core version in the condition below because -SNAPSHOT is
+	// stripped from the ${agent.version.version} evaluation below.
+	parsedUpgradeToVersion, err := version.ParseVersion(upgradeToVersion)
+	require.NoError(t, err)
+
 	// Configure Agent with fast watcher configuration and also an invalid
 	// input when the Agent version matches the upgraded Agent version. This way
 	// the pre-upgrade version of the Agent runs healthy, but the post-upgrade
@@ -69,7 +75,7 @@ inputs:
   - condition: '${agent.version.version} == "%s"'
     type: invalid
     id: invalid-input
-`, upgradeToVersion)
+`, parsedUpgradeToVersion.CoreVersion())
 		return startFixture.Configure(ctx, []byte(invalidInputPolicy))
 	}
 

testing/upgradetest/versions.go

@@ -42,10 +42,16 @@ func GetUpgradableVersions(ctx context.Context, upgradeToVersion string, current
 		return nil, errors.New("retrieved versions list from Artifact API is empty")
 	}
 
+	return getUpgradableVersions(ctx, vList, upgradeToVersion, currentMajorVersions, previousMajorVersions)
+}
+
+// Internal version of GetUpgradableVersions() with the artifacts API dependency removed for testing.
+func getUpgradableVersions(ctx context.Context, vList *tools.VersionList, upgradeToVersion string, currentMajorVersions int, previousMajorVersions int) ([]*version.ParsedSemVer, error) {
 	parsedUpgradeToVersion, err := version.ParseVersion(upgradeToVersion)
 	if err != nil {
 		return nil, fmt.Errorf("upgradeToVersion %q is not a valid version string: %w", upgradeToVersion, err)
 	}
+
 	currentMajor := parsedUpgradeToVersion.Major()
 	var currentMajorSelected, previousMajorSelected int
 
@@ -66,6 +72,11 @@ func GetUpgradableVersions(ctx context.Context, upgradeToVersion string, current
 	// we want to sort in descending orders, so we sort them
 	sort.Sort(sort.Reverse(sortedParsedVersions))
 
+	// If the only available build of the most recent version is a snapshot it is unreleased.
+	// This is always true on main and true until the first release of each minor version branch.
+	mostRecentVersion := sortedParsedVersions[0]
+	mostRecentIsUnreleased := mostRecentVersion.IsSnapshot()
+
 	var upgradableVersions []*version.ParsedSemVer
 	for _, parsedVersion := range sortedParsedVersions {
 		if currentMajorSelected == currentMajorVersions && previousMajorSelected == previousMajorVersions {
@@ -78,9 +89,21 @@ func GetUpgradableVersions(ctx context.Context, upgradeToVersion string, current
 			continue
 		}
 
+		isPrevMinor := (parsedUpgradeToVersion.Major() == parsedVersion.Major()) &&
+			(parsedUpgradeToVersion.Minor()-parsedVersion.Minor()) == 1
+
 		if parsedVersion.IsSnapshot() {
-			// skip all snapshots
-			continue
+			// Allow returning the snapshot build of the previous minor if the current version is unreleased.
+			// In this situation the previous minor branch may also be unreleased immediately after feature freeze.
+			if !mostRecentIsUnreleased || !isPrevMinor {
+				continue
+			}
+		} else {
+			// Skip the non-snapshot build of the previous minor since it might only be available at
+			// staging.elastic.co which is not a default binary download location.
+			if mostRecentIsUnreleased && isPrevMinor {
+				continue
+			}
 		}
 
 		if parsedVersion.Major() == currentMajor && currentMajorSelected < currentMajorVersions {

testing/upgradetest/versions_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.
+
+package upgradetest
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/elastic/elastic-agent/pkg/testing/tools"
+)
+
+// Response from https://artifacts-api.elastic.co/v1/versions shortly after the 8.11 feature freeze.
+var versionListAfter8_11FeatureFreeze = tools.VersionList{
+	Versions: []string{
+		"7.17.10",
+		"7.17.11",
+		"7.17.12",
+		"7.17.13",
+		"7.17.14-SNAPSHOT",
+		"7.17.14",
+		"8.7.1",
+		"8.8.0",
+		"8.8.1",
+		"8.8.2",
+		"8.9.0",
+		"8.9.1",
+		"8.9.2",
+		"8.10.0-SNAPSHOT",
+		"8.10.0",
+		"8.10.1-SNAPSHOT",
+		"8.10.1",
+		"8.10.2-SNAPSHOT",
+		"8.10.2",
+		"8.10.3-SNAPSHOT",
+		"8.10.3",
+		"8.11.0-SNAPSHOT",
+		"8.11.0",
+		"8.12.0-SNAPSHOT",
+	},
+	Aliases: []string{
+		"7.17-SNAPSHOT",
+		"7.17",
+		"8.7",
+		"8.8",
+		"8.9",
+		"8.10-SNAPSHOT",
+		"8.10",
+		"8.11-SNAPSHOT",
+		"8.11",
+		"8.12-SNAPSHOT",
+	},
+	Manifests: tools.Manifests{
+		LastUpdateTime:         "Tue, 10 Oct 2023 19:20:17 UTC",
+		SecondsSinceLastUpdate: 278,
+	},
+}
+
+// Tests that GetUpgradableVersions behaves correctly during the feature freeze period
+// where the both main and the previous minor release branch versions are unreleased.
+// Regression test for the problem described in https://github.com/elastic/elastic-agent/pull/3563#issuecomment-1756007790.
+func TestGetUpgradableVersionsAfterFeatureFreeze(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// Start from 8.12.0 assuming the 8.11.0 feature freeze has just happened.
+	// The 8.11.0 release is upgradable because the first 8.11.0 build candidate exists,
+	// but it is only available from staging.elastic.co which is not a binary download
+	// source that is supported by default.
+	currentVersion := "8.12.0"
+
+	// Since the 8.11.0 BC at staging.elastic.co isn't available to the agent by default,
+	// getUpgradableVersions should return 8.11.0-SNAPSHOT as the previous minor so an
+	// upgrade can proceed.
+	expectedUpgradableVersions := []string{
+		"8.11.0-SNAPSHOT", "8.10.3", "8.10.2", "7.17.14", "7.17.13",
+	}
+
+	// Get several of the previous versions to ensure snapshot selection works correctly.
+	versions, err := getUpgradableVersions(ctx, &versionListAfter8_11FeatureFreeze, currentVersion, 3, 2)
+	require.NoError(t, err)
+	require.NotEmpty(t, versions)
+
+	t.Logf("exp: %s", expectedUpgradableVersions)
+	t.Logf("act: %s", versions)
+	for i, exp := range expectedUpgradableVersions {
+		require.Equal(t, exp, versions[i].String())
+	}
+}