From b97766ee8c870b49de9472eefef6f91fdea5441c Mon Sep 17 00:00:00 2001 From: Craig MacKenzie Date: Tue, 10 Oct 2023 16:55:32 -0400 Subject: [PATCH] Correctly detect versions during FF. --- testing/upgradetest/versions.go | 23 ++++++++++++++++++++--- testing/upgradetest/versions_test.go | 25 ++++++++++++++----------- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/testing/upgradetest/versions.go b/testing/upgradetest/versions.go index 46d689abc39..331fc5f3fea 100644 --- a/testing/upgradetest/versions.go +++ b/testing/upgradetest/versions.go @@ -47,11 +47,11 @@ func GetUpgradableVersions(ctx context.Context, upgradeToVersion string, current // 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) { - fmt.Println(vList) 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 @@ -72,6 +72,11 @@ func getUpgradableVersions(ctx context.Context, vList *tools.VersionList, upgrad // 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 { @@ -84,9 +89,21 @@ func getUpgradableVersions(ctx context.Context, vList *tools.VersionList, upgrad continue } + isCurrentOrPrevMinor := (parsedUpgradeToVersion.Major() == parsedVersion.Major()) && + (parsedUpgradeToVersion.Minor()-parsedVersion.Minor()) <= 1 + if parsedVersion.IsSnapshot() { - // skip all snapshots - continue + // Skip snapshot builds if the most recent version isn't unreleased and this isn't the current + // or previous possibly unreleased minor version. + if !mostRecentIsUnreleased || !isCurrentOrPrevMinor { + continue + } + } else { + // Skip non-snapshot builds if the most recent is unrelesed and this version is the current or + // previous minor since they are not released yet. + if mostRecentIsUnreleased && isCurrentOrPrevMinor { + continue + } } if parsedVersion.Major() == currentMajor && currentMajorSelected < currentMajorVersions { diff --git a/testing/upgradetest/versions_test.go b/testing/upgradetest/versions_test.go index 53e96d4c8f9..bdfb0c2cc09 100644 --- a/testing/upgradetest/versions_test.go +++ b/testing/upgradetest/versions_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/elastic/elastic-agent/pkg/testing/tools" - "github.com/elastic/elastic-agent/pkg/version" "github.com/stretchr/testify/require" ) @@ -55,7 +54,7 @@ var versionListAfter8_11FeatureFreeze = tools.VersionList{ }, } -// Tests that versiontest.PreviousMinor behaves correctly during the feature freeze period +// 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) { @@ -69,17 +68,21 @@ func TestGetUpgradableVersionsAfterFeatureFreeze(t *testing.T) { 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. - expectedPrevVersion := "8.11.0-SNAPSHOT" - expectedPrevParsed, err := version.ParseVersion(expectedPrevVersion) - require.NoError(t, err) + // getUpgradableVersions should return 8.12.0-SNAPSHOT as the previous minor so an + // upgrade can proceed. It should also allow upgrading to 8.11.0-SNAPSHOT instead of + // 8.11.0. + expectedUpgradableVersions := []string{ + "8.12.0-SNAPSHOT", "8.11.0-SNAPSHOT", "8.10.3", "8.10.2", "7.17.14", "7.17.13", + } - // Duplicate the logic from PreviousMinor where only the first version returned is inspected. - versions, err := getUpgradableVersions(ctx, &versionListAfter8_11FeatureFreeze, currentVersion, 1, 0) + // Get several of the previous versions to ensure snapshot selection works correctly. + versions, err := getUpgradableVersions(ctx, &versionListAfter8_11FeatureFreeze, currentVersion, 4, 2) require.NoError(t, err) require.NotEmpty(t, versions) - prevMinor := versions[0] - require.Equal(t, expectedPrevParsed, prevMinor) + t.Logf("exp: %s", expectedUpgradableVersions) + t.Logf("act: %s", versions) + for i, exp := range expectedUpgradableVersions { + require.Equal(t, exp, versions[i].String()) + } }