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

[ILM] Searchable snapshot default "enabled" on cloud #88582

Merged
merged 2 commits into from
Jan 19, 2021

Conversation

jloleysens
Copy link
Contributor

Summary

Adds logic for turning searchable snapshots on by default on cloud.

How to test

  1. Simulate being cloud by setting a value for xpack.cloud.id and xpack.cloud.deploymentUrl in kibana.dev.yml
  2. Start Kibana on min basic license
  3. Navigate to ILM plugin
  4. Create a new policy
  5. Enable the cold phase
  6. Searchable snapshots should be on with a default value of found-snapshots for the snapshot repository
  7. Repeat the above but with xpack.cloud.id and xpack.cloud.deploymentUrl unset (so non-cloud deployment)
  8. Searchable snapshots should not be on when cold phase is turned on. When turning searchable snapshots on, the default value for snapshot repository should be empty

Screenshot 2021-01-18 at 10 22 18

Checklist

@jloleysens jloleysens added Feature:ILM Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v7.12.0 labels Jan 18, 2021
@jloleysens jloleysens requested a review from a team as a code owner January 18, 2021 10:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@kibanamachine
Copy link
Contributor

💚 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
indexLifecycleManagement 251.2KB 251.4KB +161.0B

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

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Code LGTM. I left one nit regarding the tests.

I also noticed I get a warning that no repositories are found, but found-snapshots is shown as the default. I imagine this is because I was not using a valid cloud id and deployment url, but just wanted to point this out.

Screen Shot 2021-01-19 at 9 03 46 AM

nodesByRoles: { data: ['123'] },
});
httpRequestsMockHelpers.setListSnapshotRepos({ repositories: ['found-snapshots'] });
describe('new policy', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed the test file is called edit_policy.test.ts. I'm not familiar with all the test cases here, but perhaps it should be renamed if we're testing both new and existing policies here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a really good point. For now this lines up with the component/section name of the plugin. The semantics are not quite right, if you think it is worth it, would you mind opening an issue for this change? It seems like something that needs a bit of thought as to what it should be called and whether any restructuring/refactoring is required.

@jloleysens
Copy link
Contributor Author

Thanks for the review @alisonelizabeth !

I also noticed I get a warning that no repositories are found, but found-snapshots is shown as the default. I imagine this is because I was not using a valid cloud id and deployment url, but just wanted to point this out.

Right, on-prem the field will not autocomplete to a non-existent snapshot repository. For the sake of the screenshot I hid the warning 😄

@jloleysens jloleysens merged commit fbf600d into elastic:master Jan 19, 2021
@jloleysens jloleysens deleted the ilm/ss-default-on-cloud branch January 19, 2021 15:24
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 19, 2021
* default searchable snapshot to enabled on cloud when creating a new policy

* added test for searchable snapshot on new policy on cloud
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 19, 2021
* default searchable snapshot to enabled on cloud when creating a new policy

* added test for searchable snapshot on new policy on cloud
jloleysens added a commit that referenced this pull request Jan 19, 2021
* default searchable snapshot to enabled on cloud when creating a new policy

* added test for searchable snapshot on new policy on cloud
jloleysens added a commit that referenced this pull request Jan 19, 2021
* default searchable snapshot to enabled on cloud when creating a new policy

* added test for searchable snapshot on new policy on cloud
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ILM NeededFor:Cloud release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.11.0 v7.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants