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

Improve Clean Repository error message. #117947

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

cjcenizal
Copy link
Contributor

@cjcenizal cjcenizal commented Nov 8, 2021

I updated the error-handling and display logic to surface only the pertinent details of the error instead of rendering the raw JSON response from ES (originally introduced in #53047). I also updated the tests.

Here's how this error looks now:

image

Steps to test

  1. Follow the steps from the Snapshot and Restore README to set up a repository
  2. Create a policy that will capture your entire cluster (the default)
  3. Install the logs sample data and then duplicate it using the requests below, in Console. This will increase the size of the snapshot you'll delete later, so that deleting it takes a few seconds.
  4. Execute the policy to take a snapshot
  5. In one tab, open the details of the repository and in another table open the list of snapshots
  6. Delete the snapshot and immediately click the "Clean up repository" button in the other tab

The repository will fail to clean up because it's in the process of deleting a snapshot and you'll see the error above.

# Clone the sample data a bunch of times
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-1
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-2
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-3
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-4
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-5
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-6
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-7
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-8
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-9
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-10
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-12
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-13
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-14
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-15
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-16
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-17
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-18
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-19
POST /kibana_sample_data_logs/_clone/kibana_sample_data_logs-20

@cjcenizal cjcenizal added bug Fixes for quality problems that affect the customer experience v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI v7.16.0 v7.16.1 labels Nov 8, 2021
@cjcenizal cjcenizal requested a review from a team as a code owner November 8, 2021 21:49
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-management (Team:Stack Management)

{cleanup.error
? JSON.stringify(cleanup.error)
: i18n.translate('xpack.snapshotRestore.repositoryDetails.cleanupUnknownError', {
defaultMessage: '503: Unknown error',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm missing the background on this particular error scenario. @cuff-links Do you recall any details around this or how we could reproduce?

@cjcenizal cjcenizal added v8.1.0 and removed v7.16.1 labels Nov 8, 2021
@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
snapshotRestore 258.7KB 258.6KB -165.0B

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

Copy link
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

Thanks for patching this up @cjcenizal! code changes lgtm, I was only able to test it locally by forcing an error on the POST handler for cleanup repository x-pack/plugins/snapshot_restore/server/routes/api/repositories.ts


type HomeTestSubjects = TestSubjects | ThreeLevelDepth | NonVisibleTestSubjects;

type NonVisibleTestSubjects =
Copy link
Member

Choose a reason for hiding this comment

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

🔥

@cjcenizal cjcenizal added the release_note:skip Skip the PR/issue when compiling release notes label Nov 9, 2021
@cjcenizal cjcenizal merged commit 69c0e6c into elastic:main Nov 9, 2021
cjcenizal added a commit to cjcenizal/kibana that referenced this pull request Nov 9, 2021
cjcenizal added a commit to cjcenizal/kibana that referenced this pull request Nov 9, 2021
# Conflicts:
#	x-pack/plugins/snapshot_restore/server/routes/api/repositories.ts
@cjcenizal cjcenizal deleted the snapshot-restore/error-handling branch November 9, 2021 17:20
cjcenizal added a commit that referenced this pull request Nov 9, 2021
# Conflicts:
#	x-pack/plugins/snapshot_restore/server/routes/api/repositories.ts
kpatticha pushed a commit to kpatticha/kibana that referenced this pull request Nov 10, 2021
fkanout pushed a commit to fkanout/kibana that referenced this pull request Nov 17, 2021
roeehub pushed a commit to build-security/kibana that referenced this pull request Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience 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 v7.16.0 v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants