Make action optional when upgrade details are provided

changelog/fragments/1764085508-make-action-optional-when-upgrade-details-are-provided.yaml

@@ -0,0 +1,45 @@
+# REQUIRED
+# 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
+
+# REQUIRED for all kinds
+# Change summary; a 80ish characters long description of the change.
+summary: make action optional when upgrade details are provided
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# description:
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# impact:
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# action:
+
+# REQUIRED for all kinds
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: fleet-server
+
+# AUTOMATED
+# OPTIONAL to manually add other PR URLs
+# PR URL: A link the PR 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
+
+# AUTOMATED
+# OPTIONAL to manually add other issue URLs
+# 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

internal/pkg/api/handleAck_test.go

@@ -719,67 +719,6 @@ func TestAckHandleUpgrade(t *testing.T) {
 			ESDocument: model.ESDocument{Id: "ab12dcd8-bde0-4045-92dc-c4b27668d735"},
 			Agent:      &model.AgentMetadata{Version: "8.0.0"},
 		},
-	}, {
-		name: "retry signaled",
-		event: UpgradeEvent{
-			Error: ptr("upgrade error"),
-			Payload: &struct {
-				Retry        bool `json:"retry"`
-				RetryAttempt int  `json:"retry_attempt"`
-			}{
-				Retry:        true,
-				RetryAttempt: 1,
-			},
-		},
-		bulker: func(t *testing.T) *ftesting.MockBulk {
-			m := ftesting.NewMockBulk()
-			m.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(p []byte) bool {
-				var body struct {
-					Doc struct {
-						Status string `json:"upgrade_status"`
-					} `json:"doc"`
-				}
-				if err := json.Unmarshal(p, &body); err != nil {
-					t.Fatal(err)
-				}
-				return body.Doc.Status == "retrying"
-			}), mock.Anything).Return(nil).Once()
-			return m
-		},
-		agent: &model.Agent{
-			ESDocument: model.ESDocument{Id: "ab12dcd8-bde0-4045-92dc-c4b27668d735"},
-			Agent:      &model.AgentMetadata{Version: "8.0.0"},
-		},
-	}, {
-		name: "no more retries",
-		event: UpgradeEvent{
-			Error: ptr("upgrade error"),
-			Payload: &struct {
-				Retry        bool `json:"retry"`
-				RetryAttempt int  `json:"retry_attempt"`
-			}{
-				Retry: false,
-			},
-		},
-		bulker: func(t *testing.T) *ftesting.MockBulk {
-			m := ftesting.NewMockBulk()
-			m.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(p []byte) bool {
-				var body struct {
-					Doc struct {
-						Status string `json:"upgrade_status"`
-					} `json:"doc"`
-				}
-				if err := json.Unmarshal(p, &body); err != nil {
-					t.Fatal(err)
-				}
-				return body.Doc.Status == "failed"
-			}), mock.Anything).Return(nil).Once()
-			return m
-		},
-		agent: &model.Agent{
-			ESDocument: model.ESDocument{Id: "ab12dcd8-bde0-4045-92dc-c4b27668d735"},
-			Agent:      &model.AgentMetadata{Version: "8.0.0"},
-		},
 	}, {
 		name:  "keep upgrade_attempts if upgrade_details is not nil",
 		event: UpgradeEvent{},

internal/pkg/api/handleCheckin.go

@@ -28,6 +28,7 @@ import (
 	"github.com/elastic/fleet-server/v7/internal/pkg/checkin"
 	"github.com/elastic/fleet-server/v7/internal/pkg/config"
 	"github.com/elastic/fleet-server/v7/internal/pkg/dl"
+	"github.com/elastic/fleet-server/v7/internal/pkg/es"
 	"github.com/elastic/fleet-server/v7/internal/pkg/logger/ecs"
 	"github.com/elastic/fleet-server/v7/internal/pkg/model"
 	"github.com/elastic/fleet-server/v7/internal/pkg/monitor"
@@ -454,22 +455,21 @@ func (ct *CheckinT) processUpgradeDetails(ctx context.Context, agent *model.Agen
 		return nil
 	}
 	// update docs with in progress details
-
 	vSpan, vCtx := apm.StartSpan(ctx, "Check update action", "validate")
-	action, err := ct.verifyActionExists(ctx, vSpan, agent, details)
-	if err != nil {
+
+	var actionTraceparent string
+	if action, err := ct.verifyActionExists(ctx, vSpan, agent, details); err == nil && action != nil {
+		actionTraceparent = action.Traceparent
+	} else if !errors.Is(err, es.ErrNotFound) {
 		return err
 	}
-	if action == nil {
-		return nil
-	}
 
 	// link action with APM spans
 	var links []apm.SpanLink
-	if ct.bulker.HasTracer() && action.Traceparent != "" {
-		traceCtx, err := apmhttp.ParseTraceparentHeader(action.Traceparent)
+	if ct.bulker.HasTracer() && actionTraceparent != "" {
+		traceCtx, err := apmhttp.ParseTraceparentHeader(actionTraceparent)
 		if err != nil {
-			zerolog.Ctx(vCtx).Trace().Err(err).Msgf("Error parsing traceparent: %s %s", action.Traceparent, err)
+			zerolog.Ctx(vCtx).Trace().Err(err).Msgf("Error parsing traceparent: %s %s", actionTraceparent, err)
 		} else {
 			links = []apm.SpanLink{
 				{

internal/pkg/api/handleCheckin_test.go

@@ -467,20 +467,36 @@ func TestProcessUpgradeDetails(t *testing.T) {
 		},
 		err: nil,
 	}, {
-		name:    "upgrade requested action invalid",
+		name:    "upgrade requested action not found",
 		agent:   &model.Agent{ESDocument: esd, Agent: &model.AgentMetadata{ID: "test-agent"}},
 		details: &UpgradeDetails{ActionId: "test-action", State: UpgradeDetailsStateUPGREQUESTED},
 		bulk: func() *ftesting.MockBulk {
 			mBulk := ftesting.NewMockBulk()
 			mBulk.On("Search", mock.Anything, dl.FleetActions, mock.Anything, mock.Anything).Return(&es.ResultT{}, es.ErrNotFound)
+			mBulk.On("Update", mock.Anything, dl.FleetAgents, "doc-ID", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+			return mBulk
+		},
+		cache: func() *testcache.MockCache {
+			mCache := testcache.NewMockCache()
+			mCache.On("GetAction", "test-action").Return(model.Action{}, false)
+			return mCache
+		},
+		err: nil,
+	}, {
+		name:    "upgrade requested action failed to fetch",
+		agent:   &model.Agent{ESDocument: esd, Agent: &model.AgentMetadata{ID: "test-agent"}},
+		details: &UpgradeDetails{ActionId: "test-action", State: UpgradeDetailsStateUPGREQUESTED},
+		bulk: func() *ftesting.MockBulk {
+			mBulk := ftesting.NewMockBulk()
+			mBulk.On("Search", mock.Anything, dl.FleetActions, mock.Anything, mock.Anything).Return(&es.ResultT{}, es.ErrTimeout)
 			return mBulk
 		},
 		cache: func() *testcache.MockCache {
 			mCache := testcache.NewMockCache()
 			mCache.On("GetAction", "test-action").Return(model.Action{}, false)
 			return mCache
 		},
-		err: es.ErrNotFound,
+		err: es.ErrTimeout,
 	}, {
 		name:  "upgrade scheduled action in cache",
 		agent: &model.Agent{ESDocument: esd, Agent: &model.AgentMetadata{ID: "test-agent"}},