Skip to content

[Fleet] Add redesigned Fleet Server flyout#127786

Merged
kpollich merged 65 commits intoelastic:mainfrom
kpollich:125640-redesign-fleet-server-flyout
Apr 26, 2022
Merged

[Fleet] Add redesigned Fleet Server flyout#127786
kpollich merged 65 commits intoelastic:mainfrom
kpollich:125640-redesign-fleet-server-flyout

Conversation

@kpollich
Copy link
Member

@kpollich kpollich commented Mar 15, 2022

Summary

Resolves #125640

Blocked by #128381

Implements the redesigned Fleet Server flyout and refactors various "fleet server instruction" components.

  • Separate "Quick Start" and "Advanced" tabs
    • Quick start tab is "net new" and includes largely new hooks/components
    • Advanced tab is a refactoring of the previously existing "Fleet server instructions" components
  • Add button to "Add Agent" flyout that opens the Fleet Server flyout
  • Fix failing tests (see follow-up comments)

@kpollich kpollich self-assigned this Mar 15, 2022
@kpollich
Copy link
Member Author

@elasticmachine merge upstream

@kpollich
Copy link
Member Author

@elasticmachine merge upstream

@joshdover joshdover linked an issue Mar 23, 2022 that may be closed by this pull request
9 tasks
fleetServerHostForm,
deploymentMode,
setDeploymentMode,
} = useAdvancedForm();
Copy link
Member Author

Choose a reason for hiding this comment

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

I was a little on the fence about wrapping all of this data up into a single hook like this, and I could probably be convinced that it'd be better to simply compose multiple hooks in these "tab" files instead. The only real problem this solves is in managed_instructions.tsx where we import this same useAdvancedForm hook because we need almost all of this state again, so it made sense to simply ship the "composed" set of hooks all at once to prevent too much repetition between these two places.

However, I could see the argument that our hooks should be a little more specific than this, which is very coupled to a given UI.

};

// Renders instructions inside of a flyout
export const FleetServerFlyout: React.FunctionComponent<Props> = ({ onClose }) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This file exports both the flyout "version" of this component as well as the non-flyout version. It seemed a bit extreme to separate these into two files, and then share the tab implementation between those files, but it's something we might want to consider.

}),
getInstallFleetServerStep({
isFleetServerReady,
disabled: quickStartCreateForm.status !== 'success',
Copy link
Member Author

Choose a reason for hiding this comment

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

Quick note on disabled/complete status for these steps: since the designs call for showing/hiding individual steps based on current form state, we allow a disabled prop to be passed in to support this use case. If a given step needs to implement a "complete" state (only a subset of steps do this), it's done within the component itself.

@@ -1,752 +0,0 @@
/*
Copy link
Member Author

Choose a reason for hiding this comment

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

The contents of this file have been replaced with distinct components in the fleet/components/fleet_server_instructions files above.

@kpollich
Copy link
Member Author

@elasticmachine merge upstream

@joshdover joshdover mentioned this pull request Mar 29, 2022
9 tasks
@kpollich
Copy link
Member Author

@elasticmachine merge upstream

@kpollich
Copy link
Member Author

@elasticmachine merge upstream

@kpollich
Copy link
Member Author

Created a follow up issue for converting the tab implementation (at least on the /agents version of the UI) to use EuiButtonGroup instead: #130888

@kpollich
Copy link
Member Author

Another follow-up issue for the "stuck state" when selecting a Fleet Server policy from the dropdown in the "Add Agent" flyout here: #130892

@kpollich
Copy link
Member Author

@elasticmachine merge upstream

@kpollich
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@hop-dev hop-dev left a comment

Choose a reason for hiding this comment

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

Code review only, a few nits none of them should stop you from merging

kpollich and others added 5 commits April 25, 2022 13:46
…t_server_instructions/components/fleet_server_host_combobox.tsx

Co-authored-by: Mark Hopkin <mark.hopkin@elastic.co>
…t_server_instructions/steps/install_fleet_server.tsx

Co-authored-by: Mark Hopkin <mark.hopkin@elastic.co>
…t_server_instructions/steps/set_deployment_mode.tsx

Co-authored-by: Mark Hopkin <mark.hopkin@elastic.co>
@kibana-ci
Copy link

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 595 616 +21

Async chunks

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

id before after diff
fleet 693.3KB 703.2KB +9.9KB

Page load bundle

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

id before after diff
fleet 110.7KB 111.0KB +224.0B

History

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

cc @kpollich

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

@juliaElastic
Copy link
Contributor

Another follow-up issue for the "stuck state" when selecting a Fleet Server policy from the dropdown in the "Add Agent" flyout here: #130892

Is this an already existing issue before this pr?

@kpollich
Copy link
Member Author

Another follow-up issue for the "stuck state" when selecting a Fleet Server policy from the dropdown in the "Add Agent" flyout here: #130892

Is this an already existing issue before this pr?

It is not. We don't limit the set of policies in the dropdown on main currently. Maybe I misunderstood the designs and we shouldn't be limiting this dropdown?

Screen.Recording.2022-04-26.at.10.14.05.AM.mov

@kpollich
Copy link
Member Author

I took a look at addressing the issue reported in #130892 as part of this PR, but I think it falls a bit out of scope. I'm landing this one and will address follow-ups separately.

@kpollich kpollich merged commit b655b48 into elastic:main Apr 26, 2022
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Apr 26, 2022
@kpollich kpollich deleted the 125640-redesign-fleet-server-flyout branch April 26, 2022 19:54
@kibanamachine
Copy link
Contributor

⚪ Backport skipped

The pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually.

Manual backport

To create the backport manually run:

node scripts/backport --pr 127786

Questions ?

Please refer to the Backport tool documentation

@juliaElastic
Copy link
Contributor

Another follow-up issue for the "stuck state" when selecting a Fleet Server policy from the dropdown in the "Add Agent" flyout here: #130892

Is this an already existing issue before this pr?

It is not. We don't limit the set of policies in the dropdown on main currently. Maybe I misunderstood the designs and we shouldn't be limiting this dropdown?

I was asking to check if we introduce a new bug. Maybe we could avoid limiting the dropdown so it is not stuck in fleet server state?

@kpollich
Copy link
Member Author

I was asking to check if we introduce a new bug. Maybe we could avoid limiting the dropdown so it is not stuck in fleet server state?

I'm going to update #130892 shortly to detail the implementation for a root fix here. The summary is:

  • We'll add a distinct "Add Fleet Server" button that opens the Fleet Server flyout
  • We'll remove the logic from the "Add Agent" flyout that swaps in the Fleet Server instructions when selecting a Fleet Server policy
  • We'll remove Fleet Server policies from the "Add Agent" dropdown

kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
* WIP new fleet server flyout

* WIP finish up quick start tab

* Refactor quick start steps into separate files:

* Initial refactor of existing fleet server instructions

* Move quick start form return value to explicit type

* Flesh out fleet server commands

* Fix translation error

* Migrate on prem instructions component over to new file structure

* Makes quick start tab actually create policy

* Fix type errors

* Fix missing hooks + update snapshots

* Fix paths in mocks

* Fix translations

* WIP test fixes

* Implement enabled/disabled state for new steps

* Fix cypress tests

* Force re-render to get last test passing

* Fix failing tests

* Fix import errors after conflicts

* Fix snapshot tests

* Use id instead of full policy for policy selector

* Replace Fleet Server instructions w/ Advanced Tab contents

* First pass at integrating add agent/fleet server flyouts

* Test commit

* Fix imports

* Fix imports

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Enforce https-only fleet server URL's + improve errors

* Fix failing tests

* Fix fleet server command in quick start

* Show success state in Quick start when policy exists + use first fleet server host if it exists

* Set initial service token value when Fleet Server policy ID is initiall set

* Generate service token instead of enrollment token

* Fix fleet server flyout opening from unhealthy callout

* Revert service token change + use EuiComboBox for fleet server host

* Fix checks + use custom option text

* Move fleet server host combobox to component

* Use new combobox in advanced tab

* Fix translations

* Fix unused import

* Don't recreate quick start policy if it already exists

* Actually use quick start policy fields 🙃

* Fix policy check

* Update x-pack/plugins/fleet/public/applications/fleet/components/fleet_server_instructions/components/fleet_server_host_combobox.tsx

Co-authored-by: Mark Hopkin <mark.hopkin@elastic.co>

* Update x-pack/plugins/fleet/public/applications/fleet/components/fleet_server_instructions/steps/install_fleet_server.tsx

Co-authored-by: Mark Hopkin <mark.hopkin@elastic.co>

* Update x-pack/plugins/fleet/public/applications/fleet/components/fleet_server_instructions/steps/set_deployment_mode.tsx

Co-authored-by: Mark Hopkin <mark.hopkin@elastic.co>

* Fix formatting issue

* Clean up fleet server settings variable declaration per PR review

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Mark Hopkin <mark.hopkin@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Deprecated - use backport:version if exact versions are needed backport:skip This PR does not require backporting release_note:enhancement 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.

[Fleet] Add Fleet Server Flyout Redesign

8 participants