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

[SLM] Hide system indices in snapshot restore flow #123365

Conversation

sabarasaba
Copy link
Member

@sabarasaba sabarasaba commented Jan 19, 2022

Partially addresses #120314

Summary

The restore snapshot wizard lets you select system indices to restore. However, a restore request that includes a system index returns an error. Since we currently use the get snapshot api, we can leverage the feature_states array (which gives us a list of system indices in the snapshot) in order to create a new array of "normal" indices that don't include any system ones.

Steps to test:

  1. Run elasticsearch with yarn es snapshot --license=trial -E path.repo=/tmp/es-backups and also run kibana
  2. Navigate to console and run the following command to create a repository:
.PUT /_snapshot/my_backup
{
  "type": "fs",
  "settings": {
    "location": "/tmp/es-backups",
    "chunk_size": "10mb"
  }
}
  1. Go to the homepage and add sample data so that we have some indices to play with
  2. Go to snapshot and restore app and create a new policy and leave all default values as they are. Run the policy.
  3. Go to the snapshots tab and verify that in the details flyout there are no system indices listed
  4. Verify that clicking restore and disabling the toggle from the Data streams and indices section also doesn't list any system indices.

Note: here's a list of system index patterns elastic/elasticsearch#50251

@sabarasaba sabarasaba added 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 Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 labels Jan 19, 2022
@sabarasaba sabarasaba self-assigned this Jan 19, 2022
@sabarasaba sabarasaba changed the title Dedupe system indices from indices array [SLM] Avoid returning any system indices in indices array Jan 20, 2022
@sabarasaba sabarasaba changed the title [SLM] Avoid returning any system indices in indices array [SLM] Hide system indices in snapshot restore flow Jan 20, 2022
@sabarasaba sabarasaba marked this pull request as ready for review January 20, 2022 09:38
@sabarasaba sabarasaba requested a review from a team as a code owner January 20, 2022 09:38
@elasticmachine
Copy link
Contributor

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

@sabarasaba
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thank you so much for fixing this issue, @sabarasaba!
I tested locally and can confirm that system indices are no longer listed in the indices list. I noticed a pre-existing issue: it's possible to provide an index pattern that doesn't match any indices in the snapshot. Doing so doesn't result in an error, but I think it also doesn't restore anything. I think it would be better to only allow index patterns that match something. I'll open an issue for that.

@sabarasaba
Copy link
Member Author

@elasticmachine merge upstream

@sabarasaba
Copy link
Member Author

Thanks for having a look and creating that issue @yuliacech!

@kibana-ci
Copy link
Collaborator

💚 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
snapshotRestore 258.6KB 258.6KB +8.0B

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.6KB 26.7KB +52.0B

History

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

cc @sabarasaba

@sabarasaba sabarasaba merged commit ed51852 into elastic:main Jan 24, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 24, 2022
@kibanamachine
Copy link
Contributor

💔 Backport failed

The pull request could not be backported due to the following error:
There are no branches to backport to. Aborting.

How to fix

Re-run the backport manually:

node scripts/backport --pr 123365

Questions ?

Please refer to the Backport tool documentation

@sabarasaba sabarasaba removed the auto-backport Deprecated - use backport:version if exact versions are needed label Jan 24, 2022
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 Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI 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 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants