Don't fail checkin if agent has upgrade details with no action

[jillguyonnet]

What is the problem this PR solves?

Relates https://github.com/elastic/ingest-dev/issues/4223

We're investigating an issue in a serverless project where Fleet Server keeps throwing 500 on agent checkin because the agent has upgrade details but the action id they refer to is not found.

How does this PR solve the problem?

Modify Checkin.processUpgradeDetails so that checkin does not fail in this scenario and log a warning instead.

[mergify]

This pull request does not have a backport label. Could you fix it @jillguyonnet? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-./d./d is the label to automatically backport to the 8./d branch. /d is the digit

[mergify]

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

[jlind23]

After chatting about this with @cmacknz we would like to change this behaviour and not fail nor even return a 5xx whenever this is happening. At most, a warning should be logged but that would be it.

[jillguyonnet]

Thanks @jlind23 - I'm not sure how to test this properly yet but I pushed a commit as it seems like a minimal change.

[jlind23]

I would like at least to get @michel-laterman's eyes on this before merging.

[michel-laterman]

We should add a changelog entry for this.

Also we can add an e2e test, we can enrol the agent then checkin with upgrade_details where we specify the action id

[cmacknz]

After chatting about this with @cmacknz we would like to change this behaviour and not fail nor even return a 5xx whenever this is happening. At most, a warning should be logged but that would be it.

To add some context for why I suggested this, if you run elastic-agent upgrade for a Fleet managed agent there will be no action, and then all sub-sequent check in will fail with 500s.

We need the system to keep operating normally if this happens, this should not be a way to block agent from successfully checking in.

Taking a larger step back, if an agent says it is updating, and Fleet doesn't expect that, the agent should probably be flagged with an "unexpected update" status or something in the Fleet UI. Most users are not going to look in the Fleet Server logs to see things like this.

We could also arguably prevent using elastic-agent upgrade at all for Fleet managed upgrades, but the system would still have to tolerate this "surprise upgrade details" situation well.

[jillguyonnet]

We should add a changelog entry for this.

Also we can add an e2e test, we can enrol the agent then checkin with upgrade_details where we specify the action id

Thanks @michel-laterman - I've added a changelog entry. I've tried adding an e2e test but running into issues so far. I'll reach out to you about this.

[jillguyonnet]

Thank you @cmacknz for the added context, it makes definite sense that a non existent agent action should not cause the checkin to fail.

On the serverless project where the issue is happening, the agentless agent version was updated as part of a new k8s deployment running a newer version of the agent. IIUC, because this is not a Fleet-managed upgrade process, it could have resulted in agents with upgrade details but no agent actions.

I'm waiting to check the proposed fix implementation with @michel-laterman.

It sounds like it would be worth capturing your last observation (handling non Fleet managed upgrades) somewhere else, perhaps in an ingest-dev issue?

[cmacknz]

On the serverless project where the issue is happening, the agentless agent version was updated as part of a new k8s deployment running a newer version of the agent. IIUC, because this is not a Fleet-managed upgrade process, it could have resulted in agents with upgrade details but no agent actions.

Containers don't trigger the upgrade process, and you can't send upgrade actions to agents in containers because they report themselves (or should) as non-upgradable. You can exec into a container and run elastic-agent upgrade though which may be where this came from.

I am a little bit hesitant to start forbidding use of the elastic-agent upgrade completely for Fleet managed agents as it is a useful escape hatch if something goes terribly wrong. For now I've written elastic/kibana#196426 as an enhancement on the Fleet side to flag agents that are upgrading unexpectedly.

[jillguyonnet]

I'm using the CI to run my test as e2e tests for the time being are they are not running properly locally, this might take a few commits.

[elastic-sonarqube]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[michel-laterman]

lgtm

thanks for adding the e2e test!

[pierrehilbert]

Should we have this fix also in 8.16?
cc @jillguyonnet

[jillguyonnet]

@pierrehilbert Yes, it would make sense. Does the backport-8.x label not cover it?

[pierrehilbert]

No, we already created the 8.16 branch, I'm adding the backport to 8.16, thanks for confirming :-)