Log warning on same version upgrade to prevent failed status

changelog/fragments/1733862556-Log-warning-on-same-version-upgrade-attempts.yaml

@@ -0,0 +1,34 @@
+# 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: bug-fix
+
+# Change summary; a 80ish characters long description of the change.
+summary: Log warning on same version upgrade attempts
+
+# 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: |
+  Log a warning instead of reporting an error whan a same-version upgrade is
+  attempted. This prevents the agent from reporting a "failed" status.
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+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/elastic/elastic-agent/issues/6186

internal/pkg/agent/application/coordinator/coordinator.go

@@ -575,6 +575,11 @@ func (c *Coordinator) Upgrade(ctx context.Context, version string, sourceURI str
 	cb, err := c.upgradeMgr.Upgrade(ctx, version, sourceURI, action, det, skipVerifyOverride, skipDefaultPgp, pgpBytes...)
 	if err != nil {
 		c.ClearOverrideState()
+		if errors.Is(err, upgrade.ErrUpgradeSameVersion) {
+			// Set upgrade state to completed, but return an error if a same version-upgrade is attempted.
+			det.SetState(details.StateCompleted)
+			return err
+		}
 		det.Fail(err)
 		return err
 	}

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

@@ -55,6 +55,7 @@ var agentArtifact = artifact.Artifact{
 }
 
 var ErrWatcherNotStarted = errors.New("watcher did not start in time")
+var ErrUpgradeSameVersion = errors.New("upgrade did not occur because it is the same version")
 
 // Upgrader performs an upgrade
 type Upgrader struct {
@@ -162,6 +163,19 @@ func (av agentVersion) String() string {
 func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, det *details.Details, skipVerifyOverride bool, skipDefaultPgp bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error) {
 	u.log.Infow("Upgrading agent", "version", version, "source_uri", sourceURI)
 
+	currentVersion := agentVersion{
+		version:  release.Version(),
+		snapshot: release.Snapshot(),
+		hash:     release.Commit(),
+	}
+
+	// check version before download
+	same, _ := isSameVersion(u.log, currentVersion, packageMetadata{}, version)
+	if same {
+		u.log.Warnf("Upgrade action skipped because agent is already at version %s", currentVersion)
+		return nil, ErrUpgradeSameVersion
+	}
+
 	// Inform the Upgrade Marker Watcher that we've started upgrading. Note that this
 	// is only possible to do in-memory since, today, the  process that's initiating
 	// the upgrade is the same as the Agent process in which the Upgrade Marker Watcher is
@@ -206,15 +220,11 @@ func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string
 		return nil, fmt.Errorf("reading metadata for elastic agent version %s package %q: %w", version, archivePath, err)
 	}
 
-	currentVersion := agentVersion{
-		version:  release.Version(),
-		snapshot: release.Snapshot(),
-		hash:     release.Commit(),
-	}
-
+	// Recheck version here in case of a snapshot->snapshot upgrade on the same version.
 	same, newVersion := isSameVersion(u.log, currentVersion, metadata, version)
 	if same {
-		return nil, fmt.Errorf("agent version is already %s", currentVersion)
+		u.log.Warnf("Upgrade action skipped because agent is already at version %s", currentVersion)
+		return nil, ErrUpgradeSameVersion
 	}
 
 	u.log.Infow("Unpacking agent package", "version", newVersion)

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

@@ -733,6 +733,23 @@ func TestIsSameVersion(t *testing.T) {
 				newVersion: agentVersion123SNAPSHOTghijkl,
 			},
 		},
+		{
+			name: "same version and snapshot, no hash (SNAPSHOT upgrade before download)",
+			args: args{
+				current: agentVersion123SNAPSHOTabcdef,
+				metadata: packageMetadata{
+					manifest: nil,
+				},
+				version: "1.2.3-SNAPSHOT",
+			},
+			want: want{
+				same: false,
+				newVersion: agentVersion{
+					version:  "1.2.3",
+					snapshot: true,
+				},
+			},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {

testing/integration/upgrade_standalone_same_commit_test.go

@@ -74,7 +74,7 @@ func TestStandaloneUpgradeSameCommit(t *testing.T) {
 			upgradetest.WithUnprivileged(unprivilegedAvailable),
 			upgradetest.WithDisableHashCheck(true),
 		)
-		assert.ErrorContainsf(t, err, fmt.Sprintf("agent version is already %s", currentVersion), "upgrade should fail indicating we are already at the same version")
+		assert.ErrorContains(t, err, "upgrade did not occur because it is the same version")
 	})
 
 	t.Run(fmt.Sprintf("Upgrade on a repackaged version of agent %s (%s)", currentVersion, unPrivilegedString), func(t *testing.T) {

testing/upgradetest/upgrader.go

@@ -332,11 +332,15 @@ func PerformUpgrade(
 
 	upgradeOutput, err := startFixture.Exec(ctx, upgradeCmdArgs)
 	if err != nil {
+		logger.Logf("Upgrade err: %v, output: %s", err, upgradeOutput)
 		// Sometimes the gRPC server shuts down before replying to the command which is expected
 		// we can determine this state by the EOF error coming from the server.
 		// If the server is just unavailable/not running, we should not succeed.
 		// Starting with version 8.13.2, this is handled by the upgrade command itself.
 		outputString := string(upgradeOutput)
+		if strings.Contains(outputString, "upgrade did not occur because it is the same version") {
+			return fmt.Errorf("upgrade did not occur because it is the same version")
+		}
 		isConnectionInterrupted := strings.Contains(outputString, "Unavailable") && strings.Contains(outputString, "EOF")
 		if !isConnectionInterrupted {
 			return fmt.Errorf("failed to start agent upgrade to version %q: %w\n%s", endVersionInfo.Binary.Version, err, upgradeOutput)