[Snapshot restore] Add support for feature_states #131310

sabarasaba · 2022-05-02T09:23:52Z

Fixes #120314 and #95128

Summary

This PR adds support for feature states in both policy creation and snapshot restoration and also thanks to elastic/elasticsearch#82671 we can now hide system indices from policy creation.

There is a few small oddities with the way the ES API's work with this feature:

When a policy which has featureStates: ['none'] is executed, the resulting snapshot will endup with featureStates: []. This is very confusing as the api states that if the array is empty then it includes all features. I've created the following issue for tracking that Make featureStates results more consistent when creating a snapshot elasticsearch#86605
When we retrieve the list of feature states from the installation it returns all of them even if they have no data to backup, which could be confusing for the user when they see no feature states in the resulting snapshot when they try to restore it. I've created the following issue for tracking that Need a way to retrieve features that have data to be backedup elasticsearch#86606

Release Note

With elastic/elasticsearch#63513 feature states were added to elasticsearch allowing users to backup system indices and other related state of certain features. Previously the user could only backup/restore all features or none by using the include global state feature. This PR adds support in kibana for allowing the user to select which features they want to backup/restore.

How to test

Start elasticsearch with yarn es snapshot --license=trial -E path.repo=/tmp/es-backups and kibana with yarn start
Navigate to Stack Management -> Snapshot and Restore -> Policies and create a new one
Verify that:
- IncludeGlobalState and includeFeatureState toggles are enabled by default
- If both toggles are disabled and includeGlobalState is enabled, includeFeatureState toggle should also be enabled
- The state from those two toggles is reflected in the review summary and request tabs
- Create two policies through the API and that each has either globalState or featureState and make sure state is reflected properly in both details flyout and edit form.
Run two policies, one that will contain featureStates and one that will contain none
Verify that:
- State for both is well reflected in the flyout
- In restore form, both options appear not enabled by default
- In restore form for the policy that contain no featureStates, the toggle should be disabled and a callout should be shown
- In restore form the state from those two toggles is reflected in the review summary and request tabs

Screenshots policy creation

Screenshots snapshot restore

…-fix'

...components/restore_snapshot_form/steps/step_logistics/system_indices_overwritten_callout.tsx

...cy_form/steps/step_settings/fields/include_global_state_field/include_global_state_field.tsx

elasticmachine · 2022-05-04T12:09:04Z

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

sabarasaba · 2022-05-10T09:01:36Z

@elasticmachine merge upstream

sabarasaba · 2022-05-10T12:35:06Z

Thanks for having a look everyone! I’ve met with @cristina-eleonora to discuss more in details how we thought this feature should work and we ended up coming to the decision to keep both include global state and feature states as separate options in the UI. This not only makes it more explicit to the user and improves discoverability, but also helps address some of the issues that @yuliacech mentioned about users perhaps creating policies using the API. We also went through the copy in a live session with @debadair to double check that everything is alright on that end.

I've also created a few ES issues to track some of the anomalies that were discovered while building this feature and listed them in the summary of this PR.

alisonelizabeth

Great work @sabarasaba! I left a couple minor comments in the code.

When we retrieve the list of feature states from the installation it returns all of them even if they have no data to backup, which could be confusing for the user when they see no feature states in the resulting snapshot when they try to restore it. I've created the following issue for tracking that elastic/elasticsearch#86606

Thanks for opening this issue! I found this behavior quite confusing.

A few other things I noticed:

When creating a new policy, and global state is enabled, I do not see it represented in the request. Technically, I think this is OK because global state is enabled by default. However, I think it'd be helpful to be explicit about this.

From a design perspective, it seemed a little strange to have almost a double heading for feature states. Do we need the "Only from" part, or can it be incorporated into the top-level heading?

alisonelizabeth · 2022-05-10T13:21:11Z

x-pack/plugins/snapshot_restore/__jest__/client_integration/policy_add.test.ts


      testBed = await setup(httpSetup);
-      await nextTick();


Thanks for cleaning this up!

x-pack/plugins/snapshot_restore/__jest__/client_integration/policy_add.test.ts

x-pack/plugins/snapshot_restore/__jest__/client_integration/policy_list.test.tsx

.../public/application/components/restore_snapshot_form/steps/step_logistics/step_logistics.tsx

alisonelizabeth · 2022-05-10T15:13:05Z

x-pack/plugins/snapshot_restore/public/application/components/policy_form/policy_form.tsx

@@ -67,6 +67,11 @@ export const PolicyForm: React.FunctionComponent<Props> = ({
  const [policy, setPolicy] = useState<SlmPolicyPayload>({
    ...originalPolicy,
    config: {
+      // When creating a new policy includesGlobalState is enabled by default and the API will also


thanks for adding this comment 👍

sabarasaba · 2022-05-10T16:41:09Z

Thanks for having a look @alisonelizabeth! I've addressed your feedback with 5b3dc19

alisonelizabeth

Thanks for addressing my feedback! Didn't retest, but code changes LGTM. Great work 🎉

debadair

Left a suggestion for changing the stacked toggle & menu based on a convo with @gchaps, and her suggested label lets us drop the "from" language entirely.

...orm/steps/step_settings/fields/include_feature_states_field/include_feature_states_field.tsx

...t_restore/public/application/components/summaries/policies/policy_feature_states_summary.tsx

yuliacech

Great work, @sabarasaba 🎉
Thanks a lot for addressing my feedback and also for creating issues to follow up with ES. For the list of all features that might not have any data to backup, we can maybe just mention that in the description. Otherwise, LGTM!

sabarasaba · 2022-05-12T14:46:30Z

@elasticmachine merge upstream

sabarasaba · 2022-05-16T08:52:05Z

After speaking to @cristina-eleonora we ended up finding a couple of things that need to be improved UX wise, but they seem a bit out of scope for this PR. I created a new issue for tackling those #132211

sabarasaba · 2022-05-16T08:52:42Z

@elasticmachine merge upstream

kibana-ci · 2022-05-16T10:02:42Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`snapshotRestore`	184	196	+12

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
`snapshotRestore`	21	22	+1

Async chunks

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

id	before	after	diff
`snapshotRestore`	258.8KB	266.4KB	+7.6KB

Page load bundle

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

id	before	after	diff
`snapshotRestore`	26.5KB	27.5KB	+962.0B

Unknown metric groups

API count

id	before	after	diff
`snapshotRestore`	21	22	+1

History

💚 Build #44797 succeeded eca5016
💚 Build #44487 succeeded fef1131
💚 Build #44308 succeeded 1413dd6
💛 Build #43789 was flaky 5b3dc19
💚 Build #43644 succeeded 63c69e6
💔 Build #43639 failed 1c514ee

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

cc @sabarasaba

sabarasaba and others added 9 commits April 28, 2022 11:03

Update copy to not include system indices

837cb19

Dont include system indices in ds/indices dropdown

5de8d29

Start working on supporting feature states

236f978

Store feature states array of options in local state

a3bda45

Fix up server side integration and show deets in flyout

8a6473c

Fix linter issues

181d500

commit using @elastic.co

cb899ef

Connect the dots in restore snapshot wizard

8e16971

Fix linter issues

0080dcd

sabarasaba self-assigned this May 2, 2022

sabarasaba added 5 commits May 2, 2022 13:47

Finish up wiring up last features

469a53c

Fix copy

5e5ada2

CR

b8a7dd9

Refactor tooltip implementation

3d4c2ca

Fix tests

13c83ef

sabarasaba and others added 4 commits May 2, 2022 16:18

Fix i18n

1595fa7

Add tests

7711a1c

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

d6f0a7d

…-fix'

Add missing tests

e0b2f84

sabarasaba commented May 4, 2022

View reviewed changes

...components/restore_snapshot_form/steps/step_logistics/system_indices_overwritten_callout.tsx Show resolved Hide resolved

sabarasaba commented May 4, 2022

View reviewed changes

...cy_form/steps/step_settings/fields/include_global_state_field/include_global_state_field.tsx Outdated Show resolved Hide resolved

sabarasaba requested a review from gchaps May 4, 2022 12:07

sabarasaba marked this pull request as ready for review May 4, 2022 12:09

sabarasaba requested a review from a team as a code owner May 4, 2022 12:09

sabarasaba added 4 commits May 9, 2022 15:13

Fix docs link

5cea032

Copy review

38fe3d1

Fix tests

fa8c67b

Fix small bug and add more tests

3aad8f3

kibanamachine and others added 4 commits May 10, 2022 04:01

Merge branch 'main' into snapshot_restore/system_indices_fix

d8f782f

Fix linter issue

8f16d3b

Address CR

1c514ee

Change duped locale id

63c69e6

sabarasaba requested review from debadair, alisonelizabeth and cristina-eleonora May 10, 2022 12:36

alisonelizabeth reviewed May 10, 2022

View reviewed changes

Address CR changes

5b3dc19

alisonelizabeth approved these changes May 10, 2022

View reviewed changes

debadair reviewed May 11, 2022

View reviewed changes

Copy updates

1413dd6

yuliacech approved these changes May 12, 2022

View reviewed changes

Merge branch 'main' into snapshot_restore/system_indices_fix

fef1131

sabarasaba mentioned this pull request May 16, 2022

[Snapshot & Restore] UX enhancements #132211

Closed

CR changes

eca5016

Merge branch 'main' into snapshot_restore/system_indices_fix

9124877

sabarasaba merged commit 8e33d5d into elastic:main May 16, 2022

sabarasaba mentioned this pull request May 16, 2022

[Snapshot + Restore] Add support for "feature states" on restore #95128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Snapshot restore] Add support for feature_states #131310

[Snapshot restore] Add support for feature_states #131310

sabarasaba commented May 2, 2022 •

edited

Loading

elasticmachine commented May 4, 2022

sabarasaba commented May 10, 2022

sabarasaba commented May 10, 2022

alisonelizabeth left a comment

alisonelizabeth May 10, 2022

alisonelizabeth May 10, 2022

sabarasaba commented May 10, 2022

alisonelizabeth left a comment

debadair left a comment

yuliacech left a comment

sabarasaba commented May 12, 2022

sabarasaba commented May 16, 2022

sabarasaba commented May 16, 2022

kibana-ci commented May 16, 2022

API count

[Snapshot restore] Add support for feature_states #131310

[Snapshot restore] Add support for feature_states #131310

Conversation

sabarasaba commented May 2, 2022 • edited Loading

Summary

Release Note

elasticmachine commented May 4, 2022

sabarasaba commented May 10, 2022

sabarasaba commented May 10, 2022

alisonelizabeth left a comment

Choose a reason for hiding this comment

alisonelizabeth May 10, 2022

Choose a reason for hiding this comment

alisonelizabeth May 10, 2022

Choose a reason for hiding this comment

sabarasaba commented May 10, 2022

alisonelizabeth left a comment

Choose a reason for hiding this comment

debadair left a comment

Choose a reason for hiding this comment

yuliacech left a comment

Choose a reason for hiding this comment

sabarasaba commented May 12, 2022

sabarasaba commented May 16, 2022

sabarasaba commented May 16, 2022

kibana-ci commented May 16, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

sabarasaba commented May 2, 2022 •

edited

Loading