Generalize package version

changelog/fragments/1712737337-always-launch-more-recent-watcher.yaml

@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: feature
+
+# Change summary; a 80ish characters long description of the change.
+summary: Always select the more recent watcher during agent upgrade/downgrade
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+#description:
+
+# Affected component; a word indicating the component this changeset affects.
+component: elastic-agent
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+#pr: https://github.com/owner/repo/1234
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+#issue: https://github.com/owner/repo/1234

dev-tools/mage/settings.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/elastic/elastic-agent/dev-tools/mage/gotool"
 	v1 "github.com/elastic/elastic-agent/pkg/api/v1"
+	"github.com/elastic/elastic-agent/pkg/version"
 )
 
 const (
@@ -50,9 +51,9 @@ const (
 	AgentCommitHashEnvVar
 
 	// Mapped functions
-	agentPackageVersionMappedFunc    = "agent_package_version"
-	agentManifestGeneratorMappedFunc = "manifest"
-	snapshotSuffix                   = "snapshot_suffix"
+	agentPackageVersionMappedFunc             = "agent_package_version"
+	agentManifestGeneratorMappedFunc          = "manifest"
+	agentPackageVersionWithSnapshotMappedFunc = "agent_package_version_with_snapshot"
 )
 
 // Common settings with defaults derived from files, CWD, and environment.
@@ -113,7 +114,7 @@ var (
 		"contains":                       strings.Contains,
 		agentPackageVersionMappedFunc:    AgentPackageVersion,
 		agentManifestGeneratorMappedFunc: PackageManifest,
-		snapshotSuffix:                   SnapshotSuffix,
+		agentPackageVersionWithSnapshotMappedFunc: AgentPackageVersionWithSnapshotFlag,
 	}
 )
 
@@ -259,7 +260,7 @@ repo.RootDir                 = {{ repo.RootDir }}
 repo.ImportPath              = {{ repo.ImportPath }}
 repo.SubDir                  = {{ repo.SubDir }}
 agent_package_version        = {{ agent_package_version}}
-snapshot_suffix              = {{ snapshot_suffix }}
+agent_package_version_with_snapshot              = {{ agent_package_version_with_snapshot }}
 `
 
 	return Expand(dumpTemplate)
@@ -342,8 +343,12 @@ func GeneratePackageManifest(beatName, packageVersion string, snapshot bool, ful
 	versionedHomePath := path.Join("data", fmt.Sprintf("%s-%s", beatName, shortHash))
 	m.Package.VersionedHome = versionedHomePath
 	m.Package.PathMappings = []map[string]string{{}}
-	m.Package.PathMappings[0][versionedHomePath] = fmt.Sprintf("data/%s-%s%s-%s", beatName, m.Package.Version, GenerateSnapshotSuffix(snapshot), shortHash)
-	m.Package.PathMappings[0][v1.ManifestFileName] = fmt.Sprintf("data/%s-%s%s-%s/%s", beatName, m.Package.Version, GenerateSnapshotSuffix(snapshot), shortHash, v1.ManifestFileName)
+	versionWithSnapshotFlag, err := version.GenerateAgentVersionWithSnapshotFlag(packageVersion, snapshot)
+	if err != nil {
+		return "", fmt.Errorf("generating package version with snapshot flag string (version=%q snapshot=%v): %w", packageVersion, snapshot, err)
+	}
+	m.Package.PathMappings[0][versionedHomePath] = fmt.Sprintf("data/%s-%s-%s", beatName, versionWithSnapshotFlag, shortHash)
+	m.Package.PathMappings[0][v1.ManifestFileName] = fmt.Sprintf("data/%s-%s-%s/%s", beatName, versionWithSnapshotFlag, shortHash, v1.ManifestFileName)
 	yamlBytes, err := yaml.Marshal(m)
 	if err != nil {
 		return "", fmt.Errorf("marshaling manifest: %w", err)
@@ -352,16 +357,13 @@ func GeneratePackageManifest(beatName, packageVersion string, snapshot bool, ful
 	return string(yamlBytes), nil
 }
 
-func SnapshotSuffix() string {
-	return GenerateSnapshotSuffix(Snapshot)
-}
-
-func GenerateSnapshotSuffix(snapshot bool) string {
-	if !snapshot {
-		return ""
+func AgentPackageVersionWithSnapshotFlag() (string, error) {
+	packageVersion, err := AgentPackageVersion()
+	if err != nil {
+		return "", fmt.Errorf("retrieving agent package version: %w", err)
 	}
-
-	return "-SNAPSHOT"
+	snapshotFlag := Snapshot
+	return version.GenerateAgentVersionWithSnapshotFlag(packageVersion, snapshotFlag)
 }
 
 var (

dev-tools/mage/settings_test.go

@@ -5,6 +5,7 @@
 package mage
 
 import (
+	"fmt"
 	"os"
 	"testing"
 
@@ -52,3 +53,121 @@ func TestAgentPackageVersion(t *testing.T) {
 		assert.Equal(t, expectedPkgVersion, mappedFuncPkgVersion)
 	})
 }
+
+func TestGeneratePackageManifest_AgentVersion(t *testing.T) {
+	// manifest format string. argument are expected in order:
+	// 1: packageVersion
+	// 2: snapshot
+	// 3: fullHash
+	// 4: shortHash
+	// 5: version string combining packageVersion and snapshot flag
+	const manifestFormat = `
+    version: co.elastic.agent/v1
+    kind: PackageManifest
+    package:
+        version: %[1]s
+        hash: %[3]s
+        snapshot: %[2]t
+        versioned-home: data/elastic-agent-%[4]s
+        path-mappings:
+            - data/elastic-agent-%[4]s: data/elastic-agent-%[5]s-%[4]s
+              manifest.yaml: data/elastic-agent-%[5]s-%[4]s/manifest.yaml
+`
+
+	type args struct {
+		beatName       string
+		packageVersion string
+		snapshot       bool
+		fullHash       string
+		shortHash      string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    string
+		wantErr assert.ErrorAssertionFunc
+	}{
+		{
+			name: "simple major.minor.patch version, no snapshot",
+			args: args{
+				beatName:       "elastic-agent",
+				packageVersion: "1.2.3",
+				snapshot:       false,
+				fullHash:       "abcdefghijkl",
+				shortHash:      "abcdef",
+			},
+			want:    "1.2.3",
+			wantErr: assert.NoError,
+		},
+		{
+			name: "simple major.minor.patch version, snapshot",
+			args: args{
+				beatName:       "elastic-agent",
+				packageVersion: "1.2.3",
+				snapshot:       true,
+				fullHash:       "abcdefghijkl",
+				shortHash:      "abcdef",
+			},
+			want:    "1.2.3-SNAPSHOT",
+			wantErr: assert.NoError,
+		},
+		{
+			name: "major.minor.patch version with build metadata, no snapshot",
+			args: args{
+				beatName:       "elastic-agent",
+				packageVersion: "1.2.3+build20240329010101",
+				snapshot:       false,
+				fullHash:       "abcdefghijkl",
+				shortHash:      "abcdef",
+			},
+			want:    "1.2.3+build20240329010101",
+			wantErr: assert.NoError,
+		},
+		{
+			name: "major.minor.patch version with build metadata, snapshot",
+			args: args{
+				beatName:       "elastic-agent",
+				packageVersion: "1.2.3+build20240329010101",
+				snapshot:       true,
+				fullHash:       "abcdefghijkl",
+				shortHash:      "abcdef",
+			},
+			want:    "1.2.3-SNAPSHOT+build20240329010101",
+			wantErr: assert.NoError,
+		},
+		{
+			name: "major.minor.patch version with prerelease and build metadata, no snapshot",
+			args: args{
+				beatName:       "elastic-agent",
+				packageVersion: "1.2.3-prerelease+build20240329010101",
+				snapshot:       false,
+				fullHash:       "abcdefghijkl",
+				shortHash:      "abcdef",
+			},
+			want:    "1.2.3-prerelease+build20240329010101",
+			wantErr: assert.NoError,
+		},
+		{
+			name: "major.minor.patch version with prerelease and build metadata, snapshot",
+			args: args{
+				beatName:       "elastic-agent",
+				packageVersion: "1.2.3-prerelease+build20240329010101",
+				snapshot:       true,
+				fullHash:       "abcdefghijkl",
+				shortHash:      "abcdef",
+			},
+			want:    "1.2.3-SNAPSHOT.prerelease+build20240329010101",
+			wantErr: assert.NoError,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := GeneratePackageManifest(tt.args.beatName, tt.args.packageVersion, tt.args.snapshot, tt.args.fullHash, tt.args.shortHash)
+			if !tt.wantErr(t, err, fmt.Sprintf("GeneratePackageManifest(%v, %v, %v, %v, %v)", tt.args.beatName, tt.args.packageVersion, tt.args.snapshot, tt.args.fullHash, tt.args.shortHash)) {
+				return
+			}
+			expectedYaml := fmt.Sprintf(manifestFormat, tt.args.packageVersion, tt.args.snapshot, tt.args.fullHash, tt.args.shortHash, tt.want)
+			assert.YAMLEqf(t, expectedYaml, got, "GeneratePackageManifest(%v, %v, %v, %v, %v)", tt.args.beatName, tt.args.packageVersion, tt.args.snapshot, tt.args.fullHash, tt.args.shortHash)
+		})
+	}
+}

dev-tools/packaging/package_test.go

@@ -37,6 +37,7 @@ import (
 
 	"github.com/elastic/elastic-agent/dev-tools/mage"
 	v1 "github.com/elastic/elastic-agent/pkg/api/v1"
+	"github.com/elastic/elastic-agent/pkg/version"
 )
 
 const (
@@ -235,9 +236,9 @@ func checkManifestFileContents(t *testing.T, extractedPackageDir string) {
 		if assert.Contains(t, m.Package.PathMappings[0], versionedHome, "path mappings in manifest do not contain the extraction path for versionedHome") {
 			// the first map should have the mapping for the data/elastic-agent-****** path)
 			mappedPath := m.Package.PathMappings[0][versionedHome]
-			assert.Contains(t, mappedPath, m.Package.Version, "mapped path for versionedHome does not contain the package version")
-			if m.Package.Snapshot {
-				assert.Contains(t, mappedPath, "SNAPSHOT", "mapped path for versionedHome does not contain the snapshot qualifier")
+			expectedVersionStringInPath, err := version.GenerateAgentVersionWithSnapshotFlag(m.Package.Version, m.Package.Snapshot)
+			if assert.NoError(t, err, "error generating agent version string with snapshot flag") {
+				assert.Contains(t, mappedPath, expectedVersionStringInPath, "mapped path for versionedHome does not contain the package version")
 			}
 		}
 	}

dev-tools/packaging/packages.yml

@@ -63,19 +63,19 @@ shared:
       /etc/init.d/{{.BeatServiceName}}:
         template: '{{ elastic_beats_dir }}/dev-tools/packaging/templates/{{.PackageType}}/elastic-agent.init.sh.tmpl'
         mode: 0755
-      /var/lib/{{.BeatName}}/data/{{.BeatName}}-{{agent_package_version}}{{snapshot_suffix}}-{{ commit_short }}/{{.BeatName}}{{.BinaryExt}}:
+      /var/lib/{{.BeatName}}/data/{{.BeatName}}-{{agent_package_version_with_snapshot}}-{{ commit_short }}/{{.BeatName}}{{.BinaryExt}}:
         source: build/golang-crossbuild/{{.BeatName}}-{{.GOOS}}-{{.Platform.Arch}}{{.BinaryExt}}
         mode: 0755
-      /var/lib/{{.BeatName}}/data/{{.BeatName}}-{{agent_package_version}}{{snapshot_suffix}}-{{ commit_short }}/package.version:
+      /var/lib/{{.BeatName}}/data/{{.BeatName}}-{{agent_package_version_with_snapshot}}-{{ commit_short }}/package.version:
         content: >
           {{ agent_package_version }}
         mode: 0644
-      /var/lib/{{.BeatName}}/data/{{.BeatName}}-{{agent_package_version}}{{snapshot_suffix}}-{{ commit_short }}/components:
+      /var/lib/{{.BeatName}}/data/{{.BeatName}}-{{agent_package_version_with_snapshot}}-{{ commit_short }}/components:
         source: '{{.AgentDropPath}}/{{.GOOS}}-{{.AgentArchName}}.tar.gz/'
         mode: 0755
         config_mode: 0644
         skip_on_missing: true
-      /var/lib/{{.BeatName}}/data/{{.BeatName}}-{{agent_package_version}}{{snapshot_suffix}}-{{ commit_short }}/manifest.yaml:
+      /var/lib/{{.BeatName}}/data/{{.BeatName}}-{{agent_package_version_with_snapshot}}-{{ commit_short }}/manifest.yaml:
         mode: 0644
         content: >
           {{ manifest }}

dev-tools/packaging/templates/darwin/elastic-agent.tmpl

@@ -7,8 +7,8 @@ symlink="/Library/Elastic/Agent/elastic-agent"
 
 if test -L "$symlink"; then
     symlinkTarget="data/elastic-agent-{{ commit_short }}/elastic-agent.app/Contents/MacOS/elastic-agent"
-    if test -f "data/elastic-agent-{{ agent_package_version }}{{ snapshot_suffix }}-{{ commit_short }}/elastic-agent.app/Contents/MacOS/elastic-agent"; then
-        symlinkTarget="data/elastic-agent-{{ agent_package_version }}{{ snapshot_suffix }}-{{ commit_short }}/elastic-agent.app/Contents/MacOS/elastic-agent"
+    if test -f "data/elastic-agent-{{ agent_package_version_with_snapshot }}-{{ commit_short }}/elastic-agent.app/Contents/MacOS/elastic-agent"; then
+        symlinkTarget="data/elastic-agent-{{ agent_package_version_with_snapshot }}-{{ commit_short }}/elastic-agent.app/Contents/MacOS/elastic-agent"
     fi
     ln -sfn "$symlinkTarget" "$symlink"
 fi

dev-tools/packaging/templates/linux/postinstall.sh.tmpl

@@ -16,7 +16,7 @@ if test -L "$symlink"; then
 fi
 
 commit_hash="{{ commit_short }}"
-version_dir="{{agent_package_version}}{{snapshot_suffix}}"
+version_dir="{{agent_package_version_with_snapshot}}"
 
 new_agent_dir="/var/lib/elastic-agent/data/elastic-agent-$version_dir-$commit_hash"
 

internal/pkg/agent/application/upgrade/step_mark.go

@@ -16,6 +16,7 @@ import (
 	"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
 	"github.com/elastic/elastic-agent/internal/pkg/fleetapi"
 	"github.com/elastic/elastic-agent/pkg/core/logger"
+	"github.com/elastic/elastic-agent/pkg/version"
 )
 
 const markerFilename = ".update-marker"
@@ -116,6 +117,7 @@ func newMarkerSerializer(m *UpdateMarker) *updateMarkerSerializer {
 }
 
 type agentInstall struct {
+	parsedVersion *version.ParsedSemVer
 	version       string
 	hash          string
 	versionedHome string

internal/pkg/agent/application/upgrade/upgrade.go

@@ -272,12 +272,15 @@ func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string
 	// while the `previous` install is the currently executing elastic-agent that is no longer reachable via the symlink.
 	// After the restart at the end of the function, everything lines up correctly.
 	current := agentInstall{
+		parsedVersion: parsedVersion,
 		version:       version,
 		hash:          unpackRes.Hash,
 		versionedHome: unpackRes.VersionedHome,
 	}
 
+	previousParsedVersion := getParsedVersionWithFallback(u.log, release.VersionWithSnapshot(), agtversion.NewParsedSemVer(8, 13, 0, "", ""))
 	previous := agentInstall{
+		parsedVersion: previousParsedVersion,
 		version:       release.VersionWithSnapshot(),
 		hash:          release.Commit(),
 		versionedHome: currentVersionedHome,
@@ -293,15 +296,8 @@ func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string
 		return nil, goerrors.Join(err, rollbackErr)
 	}
 
-	minParsedVersionForNewUpdateMarker := agtversion.NewParsedSemVer(8, 13, 0, "", "")
-	var watcherExecutable string
-	if parsedVersion.Less(*minParsedVersionForNewUpdateMarker) {
-		// use the current agent executable for watch, if downgrading the old agent doesn't understand the current agent's path structure.
-		watcherExecutable = paths.BinaryPath(paths.VersionedHome(paths.Top()), agentName)
-	} else {
-		// use the new agent executable as it should be able to parse the new update marker
-		watcherExecutable = paths.BinaryPath(filepath.Join(paths.Top(), unpackRes.VersionedHome), agentName)
-	}
+	var watcherExecutable = selectWatcherExecutable(paths.Top(), previous, current)
+
 	var watcherCmd *exec.Cmd
 	if watcherCmd, err = InvokeWatcher(u.log, watcherExecutable); err != nil {
 		u.log.Errorw("Rolling back: starting watcher failed", "error.message", err)
@@ -328,6 +324,26 @@ func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string
 	return cb, nil
 }
 
+func getParsedVersionWithFallback(log *logger.Logger, currentVersion string, fallbackVersion *agtversion.ParsedSemVer) *agtversion.ParsedSemVer {
+	previousParsedVersion, err := agtversion.ParseVersion(currentVersion)
+	if err != nil {
+		previousParsedVersion = fallbackVersion
+		log.Warnw(fmt.Sprintf("parsing of current version failed, watcher selection will fall back to comparing to %q", previousParsedVersion), "error.message", err)
+	}
+	return previousParsedVersion
+}
+
+func selectWatcherExecutable(topDir string, previous agentInstall, current agentInstall) string {
+	// check if the upgraded version is less than the previous (currently installed) version
+	if current.parsedVersion.Less(*previous.parsedVersion) {
+		// use the current agent executable for watch, if downgrading the old agent doesn't understand the current agent's path structure.
+		return paths.BinaryPath(filepath.Join(topDir, previous.versionedHome), agentName)
+	} else {
+		// use the new agent executable as it should be able to parse the new update marker
+		return paths.BinaryPath(filepath.Join(topDir, current.versionedHome), agentName)
+	}
+}
+
 func waitForWatcher(ctx context.Context, log *logger.Logger, markerFilePath string, waitTime time.Duration) error {
 	return waitForWatcherWithTimeoutCreationFunc(ctx, log, markerFilePath, waitTime, context.WithTimeout)
 }
@@ -417,8 +433,7 @@ func isSameVersion(log *logger.Logger, current agentVersion, metadata packageMet
 	} else {
 		// extract version info from the version string (we can ignore parsing errors as it would have never passed the download step)
 		parsedVersion, _ := agtversion.ParseVersion(upgradeVersion)
-		newVersion.version = strings.TrimSuffix(parsedVersion.VersionWithPrerelease(), snapshotSuffix)
-		newVersion.snapshot = parsedVersion.IsSnapshot()
+		newVersion.version, newVersion.snapshot = parsedVersion.ExtractSnapshotFromVersionString()
 	}
 	newVersion.hash = metadata.hash