search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.35k stars 7.98k forks source link

[Fleet]: Able to upgrade by adding lower version than agent version under Upgrade agent flyout. #180791

Closed harshitgupta-qasource closed 1 week ago

harshitgupta-qasource commented 1 month ago

Kibana Build details:

VERSION: 8.14.0 SNAPSHOT
BUILD: 73332
COMMIT: 3f75f6aa5da418e85d03a43e89f0dbb89afe75a2

Host OS: All

Preconditions:

  1. 8.14.0-SNAPSHOT Kibana cloud environment should be available.
  2. Lower agent version should be added.

Steps to reproduce:

  1. Navigate to Fleet>Agents tab.
  2. Select any agent and Click on action button > Upgrade Agent
  3. Enter lower Version than Agent version (8.11.0) and Click on Upgrade Agent Button.
  4. Able to upgrade by adding lower version than agent version

Expected Result: User should not be able to upgrade by adding lower version than agent version under Upgrade agent flyout.

Screen Recording:

https://github.com/elastic/kibana/assets/101545338/034020f4-ecd0-49e8-8a6b-bf7486d717b2

elasticmachine commented 1 month ago

Pinging @elastic/fleet (Team:Fleet)

harshitgupta-qasource commented 1 month ago

@amolnater-qasource Kindly review

amolnater-qasource commented 1 month ago

Secondary review for this ticket is Done

juliaElastic commented 1 month ago

The upgrade is failing when using a lower version, right? It seems only the UI validation is missing to prevent entering a lower version manually.

harshitgupta-qasource commented 1 month ago

Yes, it triggers the upgrade with lower version, however the upgrade gets failed. Thanks

cmacknz commented 1 month ago

I wouldn't want to widely allow downgrades until the discussions in https://github.com/elastic/kibana/issues/172745 are resolved.

We definitely don't test all the possible combinations of downgrades (e.g. 8.13.2 -> 7.17.x) the way we do for upgrades.

jen-huang commented 2 weeks ago

How should we move forward on this bug for now @cmacknz @kpollich? Shall we add validation to prevent lower versions?

cmacknz commented 2 weeks ago

Safest thing thing to do is to forbid them from the UI but still allow them from the Fleet API if that is possible.

jen-huang commented 2 weeks ago

I'm sure that is possible. Will take a look.

jen-huang commented 2 weeks ago

(My first time working in this area of the code, so just documenting some discoveries.)

It appears we do have validation for preventing downgrades but only when a single agent is selected: Image

I will look into having this kick in when multiple agents are selected, but similar to checks we do for other bulk agent actions, it probably won't be able to be done for agents by kuery, only for a discrete amount of agents.

jen-huang commented 2 weeks ago

@juliaElastic @kpollich I would like to get your thoughts on the approach in https://github.com/elastic/kibana/pull/182530 before I spend more time on it. We have good checks for upgradability when only a single agent is selected for upgrade. And if that agent is not upgradable, we return a detailed message about why.

We don't apply the same for multiple agents, and I don't think we can if the list is by kuery, but the changes in that PR apply the same checks and messages for a discrete selection of agents. Do you think this approach might be too heavy handed? For example, if 20 agent are selected and only 1 of the 20 is not upgradable, this PR disables submitting the upgrade entirely and returns the first error message found from the list of agents:

image image

I can almost see the original behavior as a feature in which we allow users to attempt upgrading any number of agents and let any failures surface as they may, rather than attempting to foresee failures in the UI. WDYT?

juliaElastic commented 2 weeks ago

Yes, I think we don't want to be too restrictive with bulk upgrade, as the upgrade will fail anyway for agents that don't support it. We could do a similar approach to Restart upgrade for bulk selections, where the restart will only apply for agents which are stuck in updating.

kpollich commented 2 weeks ago

I can almost see the original behavior as a feature in which we allow users to attempt upgrading any number of agents and let any failures surface as they may, rather than attempting to foresee failures in the UI. WDYT?

Yes this is the intended behavior. Failed upgrade are surfaced via agent activity, but we don't "overvalidate" the list of selected agents to determine their compatibility with the desired upgrade configuration. We used to perform more validation when selecting agents, but it was the cause of many bugs and broken states in the agent table, so pushing the validation to the backend and allowing errors to be reported async in the agent activity flyout is the desired approach as of now.

cmacknz commented 2 weeks ago

. Failed upgrade are surfaced via agent activity,

Just to double check on this, these errors include validation that would be done by Kibana itself? Agent today doesn't refuse downgrades, so if you dispatch a "downgrade" action the agent will do it.

juliaElastic commented 2 weeks ago

Yes, the Kibana Fleet API validates a few things, including downgrade, that is not allowed.

jen-huang commented 2 weeks ago

So it looks like we shouldn't take any action for this bug as far as validating the version input for bulk upgrade on the frontend, since restricting downgrades is already happening at the API level. Thanks folks.

We could do a similar approach to Restart upgrade for bulk selections, where the restart will only apply for agents which are stuck in updating.

@juliaElastic Could you tell me more about this? Do you mean that currently Restart upgrade is only available for single agents, and we should allow Restart upgrade as a bulk action too?

juliaElastic commented 1 week ago

@juliaElastic Could you tell me more about this? Do you mean that currently Restart upgrade is only available for single agents, and we should allow Restart upgrade as a bulk action too?

Restart upgrade is also available as a bulk action. For single agent, Restart upgrade is only enabled if the agent is in updating state, while for bulk selections, the action is always enabled, and when clicking on it, the modal window will show how many agents out of the selection are in updating state, and only those will be included in the action. This was added here: https://github.com/elastic/kibana/pull/166154

image

jen-huang commented 1 week ago

Thanks @juliaElastic, so you mean that we could show a count of eligible agents after a version is selected, i.e. X out Y agents who have a lower version than the one selected. That makes sense to me.

@kpollich WDYT about doing that as a separate enhancement and closing this bug? IMO there is no bug here, the permissive selection and surfacing downgrade errors later on is by design.

kpollich commented 1 week ago

WDYT about doing that as a separate enhancement and closing this bug? IMO there is no bug here, the permissive selection and surfacing downgrade errors later on is by design.

+1 from me - a new feature to display the count of eligible agents for the action dynamically makes sense. Do you mind filing an issue for that? I'll close this bug in the meantime.

jen-huang commented 1 week ago

I created https://github.com/elastic/kibana/issues/182899.