[ILM] Add searchable snapshot field #83783

jloleysens · 2020-11-19T10:56:44Z

Summary

Fix #83505

Adds functionality for configuring searchable snapshots in the policy UI.

How to test

Start ES with a repo path configured, like: yarn es snapshot -E path.repo=/tmp/es-snaps
Start Kibana (yarn start), go to the "Management" in Kibana and open "Index Lifecycle Policies"
Go to create a new policy
Enable "Cold phase" and you should see searchable snapshots, but disabled due to license
Upload an enterprise license to Elasticsearch and refresh until searchable snapshot is usable in cold (i.e., the toggle appears)
Configure searchable snapshot in cold - (optional) you should see a prompt about not having any repositories, follow the instructions to create one use the path you used when you started ES.
Give the policy a name and save
Open the policy you just created and this time open the advanced settings section in the hot phase and enable searchable snapshots
There should be a message about subsequent actions being disabled - this has an effect through the entire form of removing certain fields, like the ability to set searchable snapshot further down in cold (verify this by going to the cold section and seeing that it is now disabled with a callout).
Save the policy, re-open it and check that everything was stored correctly.

Notes

copied functionality from pre-existing "wait for snapshot policy field"

Screenshots

Hot phase

New collapsed with "advanced settings" section

Searchable snapshots disabled in advanced settings

Searchable snapshots enabled in hot phase with no repository created and information callout

Cold phase

disabled due to license

enabled in cold

Gif

Release note

The ILM policy UI now supports configuring searchable snapshot in the cold phase and in the hot phase.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

- added to both hot and cold - serializing into JSON payload

…s being used

…t in the hot phase

jloleysens · 2020-11-20T09:19:21Z

@elasticmachine merge upstream

- removed nested EuiFieldRow components in data allocation field - added SCSS files for data allocation field and searchable snapshot field

jloleysens · 2020-11-23T10:40:37Z

@elasticmachine merge upstream

…test

lockewritesdocs · 2020-11-23T16:02:59Z

...licy/components/phases/shared_fields/searchable_snapshot_field/searchable_snapshot_field.tsx

+            >
+              {i18n.translate('xpack.indexLifecycleMgmt.editPolicy.searchableSnapshotCalloutBody', {
+                defaultMessage:
+                  'Force merge, shrink, freeze and searchable snapshots are not allowed when this action is enabled in the hot phase.',


Suggested change

'Force merge, shrink, freeze and searchable snapshots are not allowed when this action is enabled in the hot phase.',

'Force merge, shrink, and freeze are not allowed when searchable snapshots are enabled in the hot phase.',

The toggle itself indicates Use searchable snapshots, which I believe is "this action" in the description. If that's the case, then I think we can remove the first mention of searchable snapshots and replace "this action" with "searchable snapshots".

Thanks for the review here :). We still need to communicate that the searchable snapshot action in cold will also be disabled/disallowed - which what the origin "searchable_snapshot" text was trying to communicate 😄

Will we use the same message for both hot/cold phase?

Will the message for searchable snapshots in the cold phase be conditional on the setting in the hot phase? For example, if searchable snapshots are disabled in the hot phase and users are enabling this setting only in the cold phase, it seems odd to show them a message about enabling searchable snapshots in the hot phase.

Similarly, if enabled in the hot phase, we should show a different message in the cold phase indicating why searchable snapshots are disabled.

Enabled in hot phase, disabled in cold

Hot phase
[ x ] Create searchable snapshot

Force merge, shrink, and freeze are not allowed when searchable snapshots are enabled in the hot phase. Also, searchable snapshots will be disabled in the cold phase.

Cold phase
[ ] Create searchable snapshot

Searchable snapshots are disabled in the cold phase when already enabled in the hot phase.

Disabled in hot phase, enabled in cold

Hot phase
[ ] Create searchable snapshot

Cold phase
[ x ] Create searchable snapshot

Will we use the same message for both hot/cold phase?

This particular message only applies to setting searchable snapshot in the hot phase. Happy to add the additional message per your recommendation.

I think it depends on how the UI responds. When the user enables searchable snapshots in the hot phase, will the ability to enable them in the cold phase be greyed out or made unavailable (not displayed)? If the former, I think it might be helpful for users to know why that option is greyed out. We mention in the searchable snapshots description in the hot phase that snapshots will be disabled in the cold phase, but users could miss that mention.

I could go either way on this one, but I'm trying to think through the UX from the user's standpoint. If we remove the option for enabling searchable snapshots in the cold tier when that option is enabled in the hot tier, then users wouldn't have to question the option.

Yeah totally, I agree. I have updated the PR description with some new screenshots for the "disabled" version of the control in cold phase (it is a callout instead of just a disabled control).

lockewritesdocs · 2020-11-23T16:07:07Z

...licy/components/phases/shared_fields/searchable_snapshot_field/searchable_snapshot_field.tsx

+        'data-test-subj': 'searchableSnapshotToggle',
+        label: i18n.translate(
+          'xpack.indexLifecycleMgmt.editPolicy.searchableSnapshotsToggleLabel',
+          { defaultMessage: 'Use searchable snapshot' }


Suggested change

{ defaultMessage: 'Use searchable snapshot' }

{ defaultMessage: 'Create searchable snapshot' }

Users are creating and mounting the searchable snapshot, so I think that Create is more accurate here than Use.

lockewritesdocs · 2020-11-23T16:23:25Z

...licy/components/phases/shared_fields/searchable_snapshot_field/searchable_snapshot_field.tsx

+          {i18n.translate(
+            'xpack.indexLifecycleMgmt.editPolicy.searchableSnapshotFieldDescription',
+            {
+              defaultMessage: 'Take a snapshot of the index and mount it as a searchable snapshot.',


I think that a bit more context here is useful.

Take a snapshot of the managed index in the selected repository and mount it as a searchable snapshot. Learn more

Where Learn more links to the searchable snapshots page: https://github.com/elastic/elasticsearch/blob/master/docs/reference/ilm/actions/ilm-searchable-snapshot.asciidoc

lockewritesdocs

Added some suggested changes that are mostly clarifications. Happy to review again when this PR emerges from the draft state.

jloleysens · 2020-11-24T11:59:50Z

@elasticmachine merge upstream

…sed for other issues

jloleysens · 2020-12-03T15:58:15Z

@myasonik Thanks for taking a look! We are in the process of working through design feedback so things may yet change a little bit.

I think adding a11y test coverage merits having it's own PR since this one has already grown considerably and we'd probably want to do any further work on this in follow up PRs.

mdefazio · 2020-12-03T18:02:12Z

Here's some feedback, mostly around the callouts:
As discussed, we want to move the activate/deactivate switches for the features to the left so they're consistent with our EuiDescribedFormRows. I'm guessing this can be done in a separate PR though.

Cold snapshot:
(Before)

In the case of the cold snapshot, I would still keep the switch, but disable it. Then we can use a s callout with only a title (Cannot perform searchable snapshot in cold when it is configured in hot phase.) --> I don't think 'Snapshot is disabled' is necessary if the switch itself is disabled. I also don't think we need an icon here.

(After)

Hot snapshot:

Remove icon in the callouts: I don't think they add that much context.
Do we need a way for the user to create another repository even if they have some created already? In which case, maybe 'Create repository' copy line can be above the callout/below the repository input.
Should the repository input be a single option combobox? So I can then see the repositories I have and search as well.
It might also be good to stack the callouts on the left (per EUI guidelines :) ). See screenshot of this below.

Can then also include the 'no repos' text in the combo box:

Happy to discuss these further and which are necessary for this PR and which make sense for future ones.

cjcenizal · 2020-12-03T23:34:11Z

@mdefazio I mentioned in #83783 (comment) that the "Some actions have been disabled" callout seems overly prominent to me, considering that the user could happily and safely proceed through the form without ever reading that information. What are your thoughts on that?

jloleysens · 2020-12-04T11:03:22Z

@elasticmachine merge upstream

… to control field toggles

jloleysens · 2020-12-04T13:03:12Z

Thanks for the review @mdefazio

As discussed, we want to move the activate/deactivate switches for the features to the left so they're consistent with our EuiDescribedFormRows. I'm guessing this can be done in a separate PR though.

We can definitely do this in a follow up PR!

Do we need a way for the user to create another repository even if they have some created already? In which case, maybe 'Create repository' copy line can be above the callout/below the repository input.

Yes we do. I thought the following alternative to a callout would work well:

Should the repository input be a single option combobox? So I can then see the repositories I have and search as well.

Yes, this needs to be a single option.

It might also be good to stack the callouts on the left (per EUI guidelines :) ). See screenshot of this below.

I'd like to address this in a follow up PR as it will affect another field too.

Can then also include the 'no repos' text in the combo box...

This is really cool, which prop do we use on the combo box for his?

…fields * 'master' of github.com:elastic/kibana: [ILM] Fix delete phase serialization bug (elastic#84870) chore(NA): removes auto install of pre-commit hook (elastic#83566) chore(NA): upgrade node-sass into last v4.14.1 to stop shipping old node-gyp (elastic#84935) [Alerting] Enables AlertTypes to define the custom recovery action groups (elastic#84408) [ML] Functional tests - add missing test data cleanup (elastic#84998) Migrate privilege/role/user-related operations to a new Elasticsearch client. (elastic#84641) # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts

mdefazio · 2020-12-04T15:30:30Z

Thanks for making the edits. I'm good with tackling the items you mentioned in separate PRs.

One minor ask: Let's avoid using the help text for as many things as we are here:

I'm good with going back to using a callout below to mention there are no snapshot repos found (similar to how you're doing it elsewhere as we discussed).

@cjcenizal In regards to the prominence of the callout for items being disabled, one idea might be to move it to a tooltip after the switch—I'm not sure if this goes too far in hiding it though.

In regards to this:

The "Enterprise license required" callout seems like it could be located in the right column, replacing the "Create searchable snapshot" switch. Similar to how CCR handles the absence of remote clusters

I think this will solve itself in follow-up PRs when the switches are moved to the left side as is typical with EuiDescribedFormRows. I agree with you that the callout should be where the action is to turn it on—but this should be on the left side below the feature title + description.

Here is a layout including some of this, but I believe most of this will be done in a follow-up PR. I will approve this one per the discussion with JL to make sure we can merge this and start tackling things in smaller chunks.

mdefazio

Approving with the understanding that minor polish items (and accessibility edits) will be done in smaller, follow-up PRs.

ryankeairns · 2020-12-04T15:40:14Z

Looking good!

I would suggest creating an issue(s) to track the feedback items that are being proposed for a follow-up PR. There's been some nice suggestions here, and it can be difficult to sort back through the thread later. Thanks

jloleysens · 2020-12-04T15:49:21Z

Will do!

…

On Fri, 04 Dec 2020, 16:40 Ryan Keairns, ***@***.***> wrote: Looking good! I would suggest creating an issue(s) to track the feedback items that are being proposed for a follow-up PR. There's been some nice suggestions here, and it can be difficult to sort back through the thread later. Thanks — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#83783 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB6G67CEUN2QXADL4S5F2T3STD7G7ANCNFSM4T3HQYAA> .

jloleysens · 2020-12-06T14:14:21Z

@elasticmachine merge upstream

* Added server-side endpoint for getting list of repos * part one of client side changes: added searchable snapshot field - added to both hot and cold - serializing into JSON payload * make searchable snapshot field toggleable and require value when it is being used * fix typo in file name and remove whitespace * added searchable snapshot state context, wip * finished updating fields to show and hide based on searchable snapshot in the hot phase * hiding fields when searchable snapshots is enabled in hot - removed nested EuiFieldRow components in data allocation field - added SCSS files for data allocation field and searchable snapshot field * added translations and a first hot phase serialization test * appease type check and i18n * added cloud-specific behaviour for searchable snapshot default inlc. test * refactor snapshot state -> configuration issues as this a can be re-used for other issues * added configuration context file * hide replicas in cold if snapshotting * updated new field copy * update test coverage, test for hiding certain fields too * added license check to client side! * moved warning to below field again and moved hot phase searchable snapshot notice to right * make described form field lazy if needed * render field even when license requirement is not met * update serializer for removing searchable_snapshot field * handle 404 from ES when looking up * snapshot repos - we return an empty array * address license TODO * added tests for license check and removed license check HoC * fix legacy jest tests * added readme about legacy tests * updated jest tests and fix type issues * remove unused import * refactor component names to have "Field" at the end * refactor searchable snapshot to single interface def and add comment about force_merge_index option * address stakeholder feebdack and pr comments * update tests after latest changes * link to force merge * Revert "link to force merge" This reverts commit 6c714fb. * introduce advanced section to hot, warm and cold * added test for correctly deserializing delete phase, and added fix * remove unused variable * moved fields into advanced settings * move learn more copy below enable toggle * fix warm phase on rollover default * remove label space above rollover toggle * remove unused import * update test after fixing warm on rollover default * removed icons in callouts in searchable snapshot field, added ability to control field toggles * move callouts to description text * readd warning callouts to the searchable snapshot field * slight i18n adjustment * made callout for actions disabled a bit smaller * fix i18n Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* Added server-side endpoint for getting list of repos * part one of client side changes: added searchable snapshot field - added to both hot and cold - serializing into JSON payload * make searchable snapshot field toggleable and require value when it is being used * fix typo in file name and remove whitespace * added searchable snapshot state context, wip * finished updating fields to show and hide based on searchable snapshot in the hot phase * hiding fields when searchable snapshots is enabled in hot - removed nested EuiFieldRow components in data allocation field - added SCSS files for data allocation field and searchable snapshot field * added translations and a first hot phase serialization test * appease type check and i18n * added cloud-specific behaviour for searchable snapshot default inlc. test * refactor snapshot state -> configuration issues as this a can be re-used for other issues * added configuration context file * hide replicas in cold if snapshotting * updated new field copy * update test coverage, test for hiding certain fields too * added license check to client side! * moved warning to below field again and moved hot phase searchable snapshot notice to right * make described form field lazy if needed * render field even when license requirement is not met * update serializer for removing searchable_snapshot field * handle 404 from ES when looking up * snapshot repos - we return an empty array * address license TODO * added tests for license check and removed license check HoC * fix legacy jest tests * added readme about legacy tests * updated jest tests and fix type issues * remove unused import * refactor component names to have "Field" at the end * refactor searchable snapshot to single interface def and add comment about force_merge_index option * address stakeholder feebdack and pr comments * update tests after latest changes * link to force merge * Revert "link to force merge" This reverts commit 6c714fb. * introduce advanced section to hot, warm and cold * added test for correctly deserializing delete phase, and added fix * remove unused variable * moved fields into advanced settings * move learn more copy below enable toggle * fix warm phase on rollover default * remove label space above rollover toggle * remove unused import * update test after fixing warm on rollover default * removed icons in callouts in searchable snapshot field, added ability to control field toggles * move callouts to description text * readd warning callouts to the searchable snapshot field * slight i18n adjustment * made callout for actions disabled a bit smaller * fix i18n Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2020-12-08T17:35:47Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c7e94e7

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`indexLifecycleManagement`	111	137	+26

Async chunks

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

id	before	after	diff
`indexLifecycleManagement`	214.4KB	239.8KB	+25.4KB

Distributable file count

id	before	after	diff
`default`	46898	47663	+765

Page load bundle

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

id	before	after	diff
`indexLifecycleManagement`	76.6KB	77.3KB	+747.0B

History

💚 Build #92208 succeeded c7e94e7
💚 Build #92090 succeeded 7ba6be3
💔 Build #92008 failed 9a50b58
💔 Build #91962 failed 39a420f
💚 Build #91947 succeeded d2dbed7

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

jloleysens added 6 commits November 18, 2020 14:20

Added server-side endpoint for getting list of repos

731cd57

part one of client side changes: added searchable snapshot field

f7a62aa

- added to both hot and cold - serializing into JSON payload

make searchable snapshot field toggleable and require value when it i…

cb621a5

…s being used

fix typo in file name and remove whitespace

f9a13fe

added searchable snapshot state context, wip

dc03f43

finished updating fields to show and hide based on searchable snapsho…

5f1eb42

…t in the hot phase

jloleysens added release_note:enhancement Feature:ILM v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.11.0 labels Nov 19, 2020

kibanamachine and others added 4 commits November 20, 2020 04:19

Merge branch 'master' into ilm/snapshot-fields

50f278e

hiding fields when searchable snapshots is enabled in hot

e9453a9

- removed nested EuiFieldRow components in data allocation field - added SCSS files for data allocation field and searchable snapshot field

added translations and a first hot phase serialization test

7ea9021

appease type check and i18n

6c50e0c

jloleysens mentioned this pull request Nov 23, 2020

Add Searchable Snapshot configuration to Hot and Cold phases of ILM UI #83505

Closed

kibanamachine and others added 2 commits November 23, 2020 05:40

Merge branch 'master' into ilm/snapshot-fields

e2b9733

added cloud-specific behaviour for searchable snapshot default inlc. …

f27b66c

…test

jloleysens requested a review from yuliacech November 23, 2020 13:19

lockewritesdocs reviewed Nov 23, 2020

View reviewed changes

lockewritesdocs approved these changes Nov 23, 2020

View reviewed changes

kibanamachine and others added 4 commits November 24, 2020 06:59

Merge branch 'master' into ilm/snapshot-fields

7c012dd

refactor snapshot state -> configuration issues as this a can be re-u…

cffee98

…sed for other issues

added configuration context file

96591fe

hide replicas in cold if snapshotting

edc0d52

jloleysens added 2 commits December 3, 2020 17:32

remove unused import

c1ae8e4

update test after fixing warm on rollover default

76370a7

kibanamachine and others added 3 commits December 4, 2020 06:03

Merge branch 'master' into ilm/snapshot-fields

d2dbed7

removed icons in callouts in searchable snapshot field, added ability…

12de66f

… to control field toggles

move callouts to description text

39a420f

jloleysens added 4 commits December 4, 2020 16:10

readd warning callouts to the searchable snapshot field

0d1a65b

slight i18n adjustment

f6e1474

made callout for actions disabled a bit smaller

9a50b58

mdefazio approved these changes Dec 4, 2020

View reviewed changes

fix i18n

7ba6be3

Merge branch 'master' into ilm/snapshot-fields

c7e94e7

jloleysens merged commit 34be1e7 into elastic:master Dec 7, 2020

jloleysens deleted the ilm/snapshot-fields branch December 7, 2020 09:52

This was referenced Dec 7, 2020

[ILM] Improvements after searchable snapshot field #85115

Closed

[7.x] [ILM] Add searchable snapshot field (#83783) #85138

Merged

jloleysens mentioned this pull request Dec 7, 2020

[ILM] Should block searchable snapshot action in cold when rollover is disabled #85167

Closed

jloleysens mentioned this pull request Dec 10, 2020

[ILM] Hide rollover controls in advanced #84198

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ILM] Add searchable snapshot field #83783

[ILM] Add searchable snapshot field #83783

jloleysens commented Nov 19, 2020 •

edited

Loading

jloleysens commented Nov 20, 2020

jloleysens commented Nov 23, 2020

lockewritesdocs Nov 23, 2020

lockewritesdocs Nov 23, 2020

jloleysens Nov 23, 2020

lockewritesdocs Nov 23, 2020

jloleysens Nov 25, 2020

lockewritesdocs Nov 30, 2020

jloleysens Dec 2, 2020

lockewritesdocs Nov 23, 2020

lockewritesdocs Nov 23, 2020

lockewritesdocs Nov 23, 2020

lockewritesdocs left a comment

jloleysens commented Nov 24, 2020

jloleysens commented Dec 3, 2020

mdefazio commented Dec 3, 2020

cjcenizal commented Dec 3, 2020

jloleysens commented Dec 4, 2020

jloleysens commented Dec 4, 2020

mdefazio commented Dec 4, 2020

mdefazio left a comment

ryankeairns commented Dec 4, 2020

jloleysens commented Dec 4, 2020 via email

jloleysens commented Dec 6, 2020

kibanamachine commented Dec 8, 2020

	'Force merge, shrink, freeze and searchable snapshots are not allowed when this action is enabled in the hot phase.',
	'Force merge, shrink, and freeze are not allowed when searchable snapshots are enabled in the hot phase.',

	{ defaultMessage: 'Use searchable snapshot' }
	{ defaultMessage: 'Create searchable snapshot' }

[ILM] Add searchable snapshot field #83783

[ILM] Add searchable snapshot field #83783

Conversation

jloleysens commented Nov 19, 2020 • edited Loading

Summary

How to test

Notes

Screenshots

Release note

Checklist

jloleysens commented Nov 20, 2020

jloleysens commented Nov 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Enabled in hot phase, disabled in cold

Disabled in hot phase, enabled in cold

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lockewritesdocs left a comment

Choose a reason for hiding this comment

jloleysens commented Nov 24, 2020

jloleysens commented Dec 3, 2020

mdefazio commented Dec 3, 2020

cjcenizal commented Dec 3, 2020

jloleysens commented Dec 4, 2020

jloleysens commented Dec 4, 2020

mdefazio commented Dec 4, 2020

mdefazio left a comment

Choose a reason for hiding this comment

ryankeairns commented Dec 4, 2020

jloleysens commented Dec 4, 2020 via email

jloleysens commented Dec 6, 2020

kibanamachine commented Dec 8, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Distributable file count

Page load bundle

History

jloleysens commented Nov 19, 2020 •

edited

Loading