Skip to content

Conversation

vitobotta
Copy link
Contributor

Hi! Following the example of #628 I've added a setting for the region when configuring logical backups. With some S3-compatible services like Scaleway you need to specify the region as well as the endpoint in the aws upload command, otherwise the upload fails. I found this while investigating with the dump.sh script and found that I had to add --region <region> to the aws command for the upload to succeed. Tested many times.Hope this is OK. Thanks!

@FxKu
Copy link
Member

FxKu commented Feb 4, 2020

Thanks for this PR.
This new option is not yet covered by CRD validation. Please add it (in alphabetical order).

I think at some point we should switch to a map to pass all these arguments. Cannot create a new field every time a user needs another s3 cp option.

@vitobotta
Copy link
Contributor Author

Hi @FxKu ! I've added the missing bit. Hopefully it's OK now. :) I'm still new to this stuff. Thanks

@FxKu
Copy link
Member

FxKu commented Feb 5, 2020

Validation is still missing. Let me point you to the correct places (insert before the line I've linked):
CRD manifest
Manifest copy for Helm chart
CRD validation in Go code

@vitobotta
Copy link
Contributor Author

Hi @FxKu is it OK now or is there anything else missing? Thank you for your patience :)

@vitobotta
Copy link
Contributor Author

Looks like the last commit failed. Something else missing perhaps? Sorry, I'm not familiar with Go etc yet.

@FxKu
Copy link
Member

FxKu commented Feb 5, 2020

Retriggered the travis build and now it succeeds. Can you update my last suggestions. Sorry missed them before. Then it's done 😃

vitobotta and others added 3 commits February 5, 2020 18:54
Co-Authored-By: Felix Kunde <felix-kunde@gmx.de>
Co-Authored-By: Felix Kunde <felix-kunde@gmx.de>
Co-Authored-By: Felix Kunde <felix-kunde@gmx.de>
@vitobotta
Copy link
Contributor Author

@FxKu Done. Hopefully it's OK this time :D

@FxKu
Copy link
Member

FxKu commented Feb 6, 2020

👍

1 similar comment
@hjacobs
Copy link
Contributor

hjacobs commented Feb 6, 2020

👍

@hjacobs
Copy link
Contributor

hjacobs commented Feb 6, 2020

@vitobotta thanks for your blog post!

@vitobotta
Copy link
Contributor Author

@hjacobs Thank you guys for open sourcing this! :)

@Jan-M
Copy link
Member

Jan-M commented Feb 7, 2020

Just FYI on your blog post: We do intentionally not use wal-g for shipping backups despite the fact that it is significantly fast. We do though use it for restoring.

@vitobotta
Copy link
Contributor Author

Hi @Jan-M ! I must have missed that that. Are there any problems with using wal-g for backups as well? Do you recommend that I change my clusters because of this?

@FxKu FxKu merged commit a660d75 into zalando:master Feb 10, 2020
@vitobotta
Copy link
Contributor Author

Thanks for merging!

@FxKu FxKu added this to the 1.4 milestone Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants