From a1960692bb76c5a30bf1f8e7d17ecf14b031dd26 Mon Sep 17 00:00:00 2001 From: Craig MacKenzie Date: Mon, 16 Oct 2023 13:45:32 -0400 Subject: [PATCH] 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 9adbac28c880cc73d98f57dc5c0369db31aed3cb) --- .buildkite/scripts/steps/integration_tests.sh | 4 +- .../artifact/download/snapshot/downloader.go | 5 + .../download/snapshot/downloader_test.go | 27 ++++++ pkg/testing/tools/artifacts_api.go | 31 +++---- testing/integration/upgrade_rollback_test.go | 8 +- testing/upgradetest/versions.go | 27 +++++- testing/upgradetest/versions_test.go | 92 +++++++++++++++++++ 7 files changed, 170 insertions(+), 24 deletions(-) create mode 100644 internal/pkg/agent/application/upgrade/artifact/download/snapshot/downloader_test.go create mode 100644 testing/upgradetest/versions_test.go diff --git a/.buildkite/scripts/steps/integration_tests.sh b/.buildkite/scripts/steps/integration_tests.sh index fe916cbe50e..12f1f780ff6 100755 --- a/.buildkite/scripts/steps/integration_tests.sh +++ b/.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 .. # 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" diff --git a/internal/pkg/agent/application/upgrade/artifact/download/snapshot/downloader.go b/internal/pkg/agent/application/upgrade/artifact/download/snapshot/downloader.go index 0cad22dbe09..51b16ee4372 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/snapshot/downloader.go +++ b/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 ..-SNAPSHOT version diff --git a/internal/pkg/agent/application/upgrade/artifact/download/snapshot/downloader_test.go b/internal/pkg/agent/application/upgrade/artifact/download/snapshot/downloader_test.go new file mode 100644 index 00000000000..18ed58b0d65 --- /dev/null +++ b/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) + +} diff --git a/pkg/testing/tools/artifacts_api.go b/pkg/testing/tools/artifacts_api.go index 84fdf226df1..660f2c61574 100644 --- a/pkg/testing/tools/artifacts_api.go +++ b/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 { diff --git a/testing/integration/upgrade_rollback_test.go b/testing/integration/upgrade_rollback_test.go index fa9477de585..9bc19fc832b 100644 --- a/testing/integration/upgrade_rollback_test.go +++ b/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)) } diff --git a/testing/upgradetest/versions.go b/testing/upgradetest/versions.go index a1f046bb6aa..ea77b6f0ba6 100644 --- a/testing/upgradetest/versions.go +++ b/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 { diff --git a/testing/upgradetest/versions_test.go b/testing/upgradetest/versions_test.go new file mode 100644 index 00000000000..14fe58128cc --- /dev/null +++ b/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()) + } +}