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] Add fleet server host UI #142894

Merged
merged 26 commits into from
Oct 28, 2022

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Oct 6, 2022

Summary

Closes #137785
Depends on #142746

That PR allow to edit fleet server host in the UI.

I also changed the full policy generation to use the new fleet server host instead of the settings saved object.

UI Changes

Screen Shot 2022-10-06 at 2 07 01 PM

Screen Shot 2022-10-06 at 2 07 10 PM

Screen Shot 2022-10-06 at 2 07 25 PM

Screen Shot 2022-10-06 at 2 07 32 PM

Screenshot 2022-10-24 at 17 16 29

Screenshot 2022-10-24 at 17 16 39

Todo

  • UI To add a Fleet server
  • Update the Add Fleet server instructions to use the new Fleet Server Host resource
  • Support per agent policy Fleet server host (could be done in a separate PR for readability)

@nchaulet nchaulet added release_note:skip Skip the PR/issue when compiling release notes v8.6.0 labels Oct 6, 2022
@nchaulet nchaulet self-assigned this Oct 6, 2022
@nchaulet nchaulet added the Team:Fleet Team label for Observability Data Collection Fleet team label Oct 6, 2022
@nchaulet nchaulet force-pushed the feature-fleet-server-host-ui branch from 446e7ec to 8138d4d Compare October 6, 2022 18:19
@nchaulet nchaulet force-pushed the feature-fleet-server-host-ui branch from 8138d4d to 60ee8aa Compare October 6, 2022 21:54
@criamico
Copy link
Contributor

@elasticmachine merge upstream

@criamico
Copy link
Contributor

@elasticmachine merge upstream

@criamico
Copy link
Contributor

@elasticmachine merge upstream

@criamico
Copy link
Contributor

@elasticmachine merge upstream

@criamico
Copy link
Contributor

@elasticmachine merge upstream

@criamico
Copy link
Contributor

@elasticmachine merge upstream

@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

expected head sha didn’t match current head ref.

@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

expected head sha didn’t match current head ref.

@nchaulet nchaulet requested review from juliaElastic and a team October 27, 2022 15:02
@juliaElastic
Copy link
Contributor

Fleet Server name should be optional according to designs:
image

Currently it is required in the pr:
image

@juliaElastic
Copy link
Contributor

Is it expected that the same host url can be added multiple times?
image

@juliaElastic
Copy link
Contributor

The landing page is different in the designs, is that coming in a different pr?
image

@juliaElastic
Copy link
Contributor

juliaElastic commented Oct 28, 2022

According to the design, on /settings page, the Add Fleet Server button should open a different Add Fleet Server flyout, not only editing the host urls.
image

Current pr:
image

@criamico
Copy link
Contributor

@elasticmachine merge upstream

@criamico
Copy link
Contributor

The landing page is different in the designs, is that coming in a different pr? image

Yes, this is coming in the next PR. This is already quite big, so we decided to make the changes to make the UI functional again and the rest of changes will be done subsequently.

@criamico
Copy link
Contributor

According to the design, on /settings page, the Add Fleet Server button should open a different Add Fleet Server flyout, not only editing the host urls. image

Current pr: image

Let me check this with @nchaulet in the afternoon. I'm not sure why it was done this way.

@criamico
Copy link
Contributor

Fleet Server name should be optional according to designs: image

Currently it is required in the pr: image

Yes I saw that, but tbh I'm not sure it makes much sense to have it optional, the other objects in that page all have required names and it seems weird to me to assign multiple hosts without indicating them. Happy to change it if this is an issue :)

@criamico
Copy link
Contributor

Is it expected that the same host url can be added multiple times? image

Mmm, no I think it shouldn't allow to add multiple values all the same. I'll check the validation, thanks for highlighting it.

@nchaulet
Copy link
Member Author

Mmm, no I think it shouldn't allow to add multiple values all the same. I'll check the validation, thanks for highlighting it.

I think we can multiple Fleet server config entry with the same hosts, and it could make sense later when we add proxy here.

@nchaulet
Copy link
Member Author

Let me check this with @nchaulet in the afternoon. I'm not sure why it was done this way.

@juliaElastic I miss that during the implementation, if it's okay with you I think it can come as a followup , where we address the landing page too.

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

@nchaulet nchaulet added the ci:cloud-deploy Create or update a Cloud deployment label Oct 28, 2022
@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

kibana-ci commented Oct 28, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 796 797 +1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
fleet 893 896 +3

Async chunks

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

id before after diff
fleet 916.8KB 922.2KB +5.4KB

Page load bundle

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

id before after diff
fleet 112.8KB 114.0KB +1.2KB
Unknown metric groups

API count

id before after diff
fleet 996 999 +3

History

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

cc @nchaulet

@nchaulet nchaulet merged commit d73b57a into elastic:main Oct 28, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 28, 2022
@nchaulet nchaulet deleted the feature-fleet-server-host-ui branch October 28, 2022 14:29
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 ci:cloud-deploy Create or update a Cloud deployment release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] Support multiple Fleet Servers in Fleet UI
6 participants