diff --git a/changelog/fragments/1764085508-make-action-optional-when-upgrade-details-are-provided.yaml b/changelog/fragments/1764085508-make-action-optional-when-upgrade-details-are-provided.yaml new file mode 100644 index 000000000..fd1654f83 --- /dev/null +++ b/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 diff --git a/internal/pkg/api/handleAck_test.go b/internal/pkg/api/handleAck_test.go index d6a68ceac..67bb6b2b1 100644 --- a/internal/pkg/api/handleAck_test.go +++ b/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{}, diff --git a/internal/pkg/api/handleCheckin.go b/internal/pkg/api/handleCheckin.go index 1aa720f8a..92e50976b 100644 --- a/internal/pkg/api/handleCheckin.go +++ b/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{ { diff --git a/internal/pkg/api/handleCheckin_test.go b/internal/pkg/api/handleCheckin_test.go index a02bd9c0e..d076aaa66 100644 --- a/internal/pkg/api/handleCheckin_test.go +++ b/internal/pkg/api/handleCheckin_test.go @@ -467,12 +467,28 @@ 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 { @@ -480,7 +496,7 @@ func TestProcessUpgradeDetails(t *testing.T) { 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"}},