-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Upgrade Assistant] Show on prem backup step when found-snapshots repository not found #121060
[Upgrade Assistant] Show on prem backup step when found-snapshots repository not found #121060
Conversation
Pinging @elastic/kibana-stack-management (Team:Stack Management) |
} | ||
|
||
const isMissingFoundSnapshotsRepo = (error: ResponseError) => { | ||
return error.statusCode === 404 && error.message === `[${CLOUD_SNAPSHOT_REPOSITORY}] missing`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: can we future-proof this by checking that the message
includes the CLOUD_SNAPSHOT_REPOSITORY
instead of comparing against the entire string? If it's a 404
then I think this would be sufficient to infer that the repository is missing, without depending upon the specific language ES is using to communicate this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think even safer would be to add a test for how missing repo error is returned to UA api integration tests here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks both for having a look! I've updated this function to check if the message includes the repo name instead of matching against the entire string as suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for fixing this step for ECE, @sabarasaba!
I tested locally with xpack.cloud.id: 'test'
in kibana.dev.yml
file. The missing repo error was shown in the network tab and the correct on prem backup was displayed in the UI, great job 👍
…d of the full message
@elasticmachine merge upstream |
…ng_foundsnapshots
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @sabarasaba |
…ository not found (elastic#121060) * Show on prem stem when in cloud and missing found snapshots * Remove test code * Lets match against the error containing the missing repository instead of the full message Co-authored-by: Kibana Machine <[email protected]>
…shots repository not found (#121375) * [Upgrade Assistant] Show on prem backup step when found-snapshots repository not found (#121060) * Show on prem stem when in cloud and missing found snapshots * Remove test code * Lets match against the error containing the missing repository instead of the full message Co-authored-by: Kibana Machine <[email protected]> * commit using @elastic.co Co-authored-by: Kibana Machine <[email protected]>
…shots repository not found (elastic#121375) * [Upgrade Assistant] Show on prem backup step when found-snapshots repository not found (elastic#121060) * Show on prem stem when in cloud and missing found snapshots * Remove test code * Lets match against the error containing the missing repository instead of the full message Co-authored-by: Kibana Machine <[email protected]> * commit using @elastic.co Co-authored-by: Kibana Machine <[email protected]>
…shots repository not found (#121375) (#121450) * [Upgrade Assistant] Show on prem backup step when found-snapshots repository not found (#121060) * Show on prem stem when in cloud and missing found snapshots * Remove test code * Lets match against the error containing the missing repository instead of the full message Co-authored-by: Kibana Machine <[email protected]> * commit using @elastic.co Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Ignacio Rivas <[email protected]>
Summary
As described by ECE-Snapshots docs users might not have snapshots configured by default. The
cloud backup step
assumes that the installation has it and that thefound-snapshots
repository exists, as a consequence of this the user will be seeing an error about a missing snapshots repository.This PR changes the logic of the backup step to detect this edge case and default to the on prem step that will instead render a button that will take the user to the snapshots app.
How to test
yarn es snapshot --license=trial
and kibana withyarn start
isCloudEnabled
. This will force to load the cloud backup step while working locally.Stack Management
->Upgrade Assistant
cloud_backup_status
failed and that the backupstep is for on premNote: when this PR is merged, I'll create a new one for getting this change into main and 8.x
Screenshots
Error message