Skip to content

Commit

Permalink
Correctly detect versions during FF.
Browse files Browse the repository at this point in the history
  • Loading branch information
cmacknz committed Oct 10, 2023
1 parent 89ad961 commit b97766e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
23 changes: 20 additions & 3 deletions testing/upgradetest/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
25 changes: 14 additions & 11 deletions testing/upgradetest/versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"testing"

"github.com/elastic/elastic-agent/pkg/testing/tools"

Check failure on line 7 in testing/upgradetest/versions_test.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

File is not `goimports`-ed with -local github.com/elastic (goimports)
"github.com/elastic/elastic-agent/pkg/version"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -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) {
Expand All @@ -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())
}
}

0 comments on commit b97766e

Please sign in to comment.