Skip to content

Support for GCS WAL-E backups - #620

Merged
FxKu merged 8 commits into
zalando:masterfrom
alfredw33:master
Jun 3, 2020
Merged

Support for GCS WAL-E backups#620
FxKu merged 8 commits into
zalando:masterfrom
alfredw33:master

Conversation

@alfredw33

Copy link
Copy Markdown
Contributor

Added the ability to support the following environment variables:

  • WAL_GS_BUCKET
  • GOOGLE_APPLICATION_CREDENTIALS

to be passed down to the spilo images. This should address #198.

@alfredw33

Copy link
Copy Markdown
Contributor Author

I've updated the scripts to check for Darwin (MacOS) and removed mac specific things.

I changed some things that require your eyes :

  • Moved the -r option from the Makefile to right after the cp command.
  • Added the requirement for greadlink if you are on Mac to use run.sh (If you are okay with that)

Other then that everything seems to be working and tests are passing. Do you think we can merge this PR in now?

@FxKu

FxKu commented Aug 16, 2019

Copy link
Copy Markdown
Contributor

Thanks for your contribution and also for providing tests. I'm fine with the support for two more Spilo env variables, but would ask you to move all the changes on the e2e setup to another PR to have a cleaner separation. Is that ok for you?

Comment thread docs/reference/operator_parameters.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't immedately see where you in fact use the secret mount part?

Having the secrets in pod env var like this exposes them to a lot of readers, would be a proper secret ref be the better choice.

I thought we support all required google fields already.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought so too and I could be wrong on what it would take to get this working in GCP without my changes but from reading the documentation it wasn't clear on how to get this to work for GCP.

  • GOOGLE_APPLICATION_CREDENTIALS env variable will get populated with a file path where a credentials.json file (or whatever you call it) to the service account who has the IAM rules to push to the GCS configured bucket. This service account is then used be SPILO to push the WAL-E information the GCS bucket.

  • We then use the file mount features to mount the credientails.json to the same path as the path specified by the GOOGLE_APPLICATION_CREDENTIALS

We are using this in production for our applications in GCP. Would it be helpful on giving an example on how we did this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, it might be worth to add a sub-chapter here in the admin docs. Wondering if #946 could also be used in the future, but for now, let's take the setup which you use in production

@FxKu

FxKu commented Nov 27, 2019

Copy link
Copy Markdown
Contributor

@alfredw33 could you rebase and remove the changes in Makefile and run.sh script of e2e tests, so that this PR is only about introducing GCE config parameters? We have switched to go modules in the meantime, which also simplified the e2e setup.

Sorry, for the long pause.

@alfredw33

Copy link
Copy Markdown
Contributor Author

I will try to do it this weekend. Sorry for the late reply.

@schmitch

Copy link
Copy Markdown

is there any update on this? @alfredw33

@alfredw33

Copy link
Copy Markdown
Contributor Author

Hey,

I'll take a look at how to split these out into two different commits this weekend. I will commit to delivering the makefile / run.sh changes to support Mac development. If I have time I'll get the other commit in.

Should I Cancel this PR and make two separate ones? Does that make more sense?

@FxKu

FxKu commented Mar 26, 2020

Copy link
Copy Markdown
Contributor

@alfredw33 we are now using go modules to install kind so the makefile changes should not apply anymore. Simply remove these changes and rebase with master.

@alfredw33

Copy link
Copy Markdown
Contributor Author

Hey,

I've rebased and made the changes to get my tests to run. One thing I'm struggling with right now is that I'm getting errors on running the e2e tests where it complains about the go-client for k8. There seems to be some issues around this and considering I haven't updated any dependencies I'm not sure why this is not working for me.

Is there a specific go version I should be using. I've upgraded to 1.14.2 but maybe I need to use 1.14?

@alfredw33

Copy link
Copy Markdown
Contributor Author

@FxKu I think I got most of the asks in. Let me know if there is anything else you want me to do.

Thanks!

Comment thread charts/postgres-operator/values.yaml Outdated
# GCS Bucket to use for shipping WAL segments with WAL-E
# wal_gs_bucket: ""

# GCP Credentials for setting the GOOGLE_APPLICATION_CREDNETIALS environment variable

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
# GCP Credentials for setting the GOOGLE_APPLICATION_CREDNETIALS environment variable
# GCP credentials for setting the GOOGLE_APPLICATION_CREDENTIALS environment variable

Comment thread charts/postgres-operator/values.yaml Outdated
# S3 bucket to use for shipping WAL segments with WAL-E
# wal_s3_bucket: ""

# GCS Bucket to use for shipping WAL segments with WAL-E

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
# GCS Bucket to use for shipping WAL segments with WAL-E
# GCS bucket to use for shipping WAL segments with WAL-E

# log_s3_bucket: ""
# wal_s3_bucket: ""
# wal_gs_bucket: ""
# gcp_credentials: ""

@FxKu FxKu May 13, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add these examples also in the configmap? But please with alphabetical sorting there.

@FxKu

FxKu commented May 13, 2020

Copy link
Copy Markdown
Contributor

Thanks @alfredw33 ! Few minor things:

  1. Could you cover the two new configuration fields in the validation of the OperatorConfiguration CRD, too? Here's a guide to the files, you have to change.
  2. Please, do not change the go.mod and go.sum files. Reset your changes there. There's still one small change in the e2e run shellscript, which isn't necessary.

@FxKu FxKu added this to the 1.6 milestone May 13, 2020
@alfredw33

Copy link
Copy Markdown
Contributor Author

Thanks @alfredw33 ! Few minor things:

  1. Could you cover the two new configuration fields in the validation of the OperatorConfiguration CRD, too? Here's a guide to the files, you have to change.
  2. Please, do not change the go.mod and go.sum files. Reset your changes there. There's still one small change in the e2e run shellscript, which isn't necessary.

@FxKu - I think I completed what you asked for. Let me know if I've missed anything.

Comment thread docs/administrator.md Outdated
Comment thread docs/administrator.md Outdated
Comment thread e2e/run.sh Outdated
@FxKu

FxKu commented May 22, 2020

Copy link
Copy Markdown
Contributor

👍

@kungfuchicken

kungfuchicken commented May 29, 2020

Copy link
Copy Markdown

@alfredw33 @FxKu any progress on this? I realize it's only a week since last update, but I'm holding on a deployment pending the merge ;)

@alfredw33

Copy link
Copy Markdown
Contributor Author

@alfredw33 @FxKu any progress on this? I realize it's only a week since last update, but I'm holding on a deployment pending the merge ;)

I wasn't aware that I needed to do something else. Did I miss anything that is preventing you to merge. Looks like 1/2 approvals have been given.

@Jan-M

Jan-M commented Jun 3, 2020

Copy link
Copy Markdown
Contributor

👍

@FxKu
FxKu merged commit 2b0def5 into zalando:master Jun 3, 2020
@FxKu

FxKu commented Jun 3, 2020

Copy link
Copy Markdown
Contributor

Thanks @alfredw33 for your contribution and patience ;)

@alfredw33

Copy link
Copy Markdown
Contributor Author

@FxKu It's my pleasure! 😃

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.

5 participants