Make action optional when upgrade details are provided #5609
Conversation
michalpristas
added
Team:Elastic-Agent-Control-Plane
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
michalpristas
changed the title
…-server into fix/migrate-watching
blakerouse
left a comment
blakerouse
left a comment
There was a problem hiding this comment.
Sorry I am having trouble following how this PR works.
Does this fix the issue where if you check-in to a new cluster for the first time after migrate that it will reflect the upgrade state of that agent? If so, could you walk me through how this is done in this code.
|
@michalpristas is this PR still relevant? |
|
Yes it is |
There was a problem hiding this comment.
The PR is doing a few things; the changes to allow for missing actions (in case of migrations) makes sense; however I don't understand why the ack endpoint is being changed, or why the status is not being set to nil if a checkin indicates an upgrade completes.
Also can you please add a changelog fragment
|
code cleanup: moved updgrade status removal to a separate PR: #5970 |
| if err != nil { | ||
|
|
||
| var actionTraceparent string | ||
| if action, err := ct.verifyActionExists(ctx, vSpan, agent, details); err == nil && action != nil { |
There was a problem hiding this comment.
Does Kibana tolerate having an action ID it doesn't recognize as well?
I think passing through the upgrade details unmodified is correct, but we need to avoid Kibana now or in the future trying to do this same validation or making assumption about this field.
There was a problem hiding this comment.
I think passing through the upgrade details unmodified is correct
Well a possibility here would be removing the action_id if it doesn't exist so kibana can't depend on it, or putting some marker in the details about whether the action exists or not (though kibana can also just check this).
There was a problem hiding this comment.
@juliaElastic any opinions on if sending upgrade_details to Kibana for action_ids that don't exist or are from another cluster is a problem?
There was a problem hiding this comment.
It should be fine to have an upgrade_details with a non existent action_id, the UI doesn't validate the action_id. The only difference I can think of is that these actions won't appear in Agent activity.
There was a problem hiding this comment.
actions not in activity is fine imho. this should make reporting upgrades possible, during migration or triggered from CLI
6 participants
In a context of agent migration.
When agent is migrated and still has some upgrade details or upgrade was triggered manually for some reason we should reflect that in UI.
This PR makes action optional and updates upgrade details anyways.
I'd like ACK handling of upgrade to be reworked, but ideally in a separate PR to lower risk as I'd like this one to be in 9.2
Fixes: #5564