Preserve prerelease and metadata in GetPrevMinor. (#3289) (#3326)

* Preserve prerelease and metadata in GetPrevMinor.

The GetPrevMinor function would discard build metadata, in particular it
will drop the SNAPSHOT prerelease signifier which can cause us to
upgrade to non-existent or incorrect versions.

* Skip TestAddToBucket on Windows.

* Log error when reload fails.

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit a5c9d3a)

Co-authored-by: Craig MacKenzie <[email protected]>

internal/pkg/agent/cmd/enroll_cmd.go

@@ -271,8 +271,8 @@ func (c *enrollCmd) Execute(ctx context.Context, streams *cli.IOStreams) error {
 	}()
 
 	if c.agentProc == nil {
-		if c.daemonReload(ctx) != nil {
-			c.log.Info("Elastic Agent might not be running; unable to trigger restart")
+		if err := c.daemonReload(ctx); err != nil {
+			c.log.Infow("Elastic Agent might not be running; unable to trigger restart", "error", err)
 		} else {
 			c.log.Info("Successfully triggered restart on running Elastic Agent.")
 		}

pkg/component/runtime/command_test.go

@@ -5,6 +5,7 @@
 package runtime
 
 import (
+	"runtime"
 	"testing"
 	"time"
 
@@ -31,6 +32,9 @@ func TestAddToBucket(t *testing.T) {
 
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
+			if runtime.GOOS == "windows" {
+				t.Skip("https://github.com/elastic/elastic-agent/issues/3290: Flaky timing on Windows")
+			}
 			b := newRateLimiter(tc.dropRate, tc.bucketSize)
 
 			blocked := false

pkg/version/version_parser.go

@@ -113,16 +113,17 @@ func (psv ParsedSemVer) GetPreviousMinor() (*ParsedSemVer, error) {
 
 	if minor > 0 {
 		// We have at least one previous minor version in the current
-		// major version series
-		return NewParsedSemVer(major, minor-1, 0, "", ""), nil
+		// major version series. Set the patch to zero to guarnatee the
+		// version exists, the number of patch releases varies.
+		return NewParsedSemVer(major, minor-1, 0, psv.Prerelease(), psv.BuildMetadata()), nil
 	}
 
 	// We are at the first minor of the current major version series. To
 	// figure out the previous minor, we need to rely on knowledge of
 	// the release versions from the past major series'.
 	switch major {
 	case 8:
-		return NewParsedSemVer(7, 17, 10, "", ""), nil
+		return NewParsedSemVer(7, 17, 10, psv.Prerelease(), psv.BuildMetadata()), nil
 	}
 
 	return nil, fmt.Errorf("unable to determine previous minor version for [%s]", psv.String())

pkg/version/version_parser_test.go

@@ -360,3 +360,54 @@ func TestLess(t *testing.T) {
 		})
 	}
 }
+
+func TestPreviousMinor(t *testing.T) {
+	testcases := []struct {
+		name             string
+		version          string
+		prevMinorVersion string
+	}{
+		{
+			name:             "basic release version",
+			version:          "8.7.0",
+			prevMinorVersion: "8.6.0",
+		},
+		{
+			name:             "snapshot release version",
+			version:          "8.9.3-SNAPSHOT",
+			prevMinorVersion: "8.8.0-SNAPSHOT",
+		},
+		{
+			name:             "emergency release version",
+			version:          "8.9.0-er1",
+			prevMinorVersion: "8.8.0-er1",
+		},
+		{
+			name:             "previous major version",
+			version:          "8.0.0",
+			prevMinorVersion: "7.17.10",
+		},
+		{
+			name:             "previous major snapshot",
+			version:          "8.0.0-SNAPSHOT",
+			prevMinorVersion: "7.17.10-SNAPSHOT",
+		},
+		{
+			name:             "snapshot version with metadata",
+			version:          "8.9.1-SNAPSHOT+aaaaaa",
+			prevMinorVersion: "8.8.0-SNAPSHOT+aaaaaa",
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			parsed, err := ParseVersion(tc.version)
+			require.NoError(t, err)
+			require.NotNil(t, parsed)
+
+			prev, err := parsed.GetPreviousMinor()
+			require.NoError(t, err)
+			require.Equal(t, tc.prevMinorVersion, prev.String())
+		})
+	}
+}