Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fleet] Fix validation for agents upgrade and add warning in modal #132687

Merged

Conversation

criamico
Copy link
Contributor

@criamico criamico commented May 23, 2022

Summary

Part of #130259

  • Add a warning callout in the modal to explain that rolling upgrade is only for agent 8.3+. Callout will always be there regardless of the version of the selected agents.
    Screenshot 2022-05-23 at 15 16 59

  • Refactor and fix fleet server validation to not allow any upgrade if Fleet server is not upgraded before.

  • Add an option to the version selection combobox to type in a version (useful if the version is not available in the list for some reason). The input version is validated to check that is a valid version and is < kibana.

Checklist

Delete any items that are not applicable to this PR.

@criamico criamico self-assigned this May 23, 2022
@criamico criamico added Team:Fleet Team label for Observability Data Collection Fleet team v8.3.0 release_note:skip Skip the PR/issue when compiling release notes labels May 23, 2022
@criamico criamico marked this pull request as ready for review May 23, 2022 09:47
@criamico criamico requested a review from a team as a code owner May 23, 2022 09:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@juliaElastic
Copy link
Contributor

I find the message a bit confusing, shouldn't it say Rolling upgrade only available for Elastic Agent versions 8.3+?
Also, for agents < 8.3, should the dropdown allow selecting an option other than Immediately?

@@ -710,39 +710,6 @@ export default function (providerContext: FtrProviderContext) {
})
.expect(400);
});
it('should respond 200 if trying to bulk upgrade to a version higher than the latest fleet server version if the agents include fleet server agents', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there be a new integration test added to cover the new agent version check?

Copy link
Contributor Author

@criamico criamico May 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually there was an additional requirement that was removed. We originally had two cases:
1 when there is an agent that is a fleet server agent among the batch to be upgraded, allow the upgrade
2 if only "regular" elastic agents are in the batch, don't allow the upgrade

In this PR I'm simplifying the behaviour and removing the first case, so regardless of having or not fleet server agents in the batch, the upgrade is not allowed if the target version < max fleet server version. This case is already covered in the integration test, but I'm removing the test related to case 1.

I hope the explanation is clear.

@criamico
Copy link
Contributor Author

I find the message a bit confusing, shouldn't it say Rolling upgrade only available for Elastic Agent versions 8.3+?

Thanks for the feedback, I didn't get the finalised text of the message and tried to write it myself. I'll update it :)

Also, for agents < 8.3, should the dropdown allow selecting an option other than Immediately?

That's a good point, this feature won't work at all on older versions of elastic agents. We agreed to put a warning message for the user, but since in the selected batch there might be several versions mixed up it's ok to attempt the update. If some agents are not upgradeable the API will fail (only for them).
I think we shouldn't hide the options, however I'm sure that there will be enhancements PRs after FF.

@criamico
Copy link
Contributor Author

@elasticmachine merge upstream

@criamico criamico force-pushed the rolling_upgrade/version_validation_improvements branch from 4645aa9 to a0cd620 Compare May 23, 2022 15:59
Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 712.5KB 712.9KB +486.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fleet 91.8KB 91.8KB +2.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @criamico

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@criamico criamico merged commit de8b6fc into elastic:main May 24, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 24, 2022
@criamico criamico deleted the rolling_upgrade/version_validation_improvements branch May 24, 2022 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants