-
Notifications
You must be signed in to change notification settings - Fork 1k
Support for GCS WAL-E backups #620
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
Conversation
I've updated the scripts to check for Darwin (MacOS) and removed mac specific things. I changed some things that require your eyes :
Other then that everything seems to be working and tests are passing. Do you think we can merge this PR in now? |
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? |
The default is empty. | ||
|
||
* **gcp_credentials** | ||
Used to set the GOOGLE_APPLICATION_CREDENTIALS environment variable for the pods. |
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 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.
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 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?
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.
@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. |
I will try to do it this weekend. Sorry for the late reply. |
is there any update on this? @alfredw33 |
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? |
@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. |
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? |
@FxKu I think I got most of the asks in. Let me know if there is anything else you want me to do. Thanks! |
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 |
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.
# GCP Credentials for setting the GOOGLE_APPLICATION_CREDNETIALS environment variable | |
# GCP credentials for setting the GOOGLE_APPLICATION_CREDENTIALS environment variable |
charts/postgres-operator/values.yaml
Outdated
@@ -200,6 +200,12 @@ configAwsOrGcp: | |||
# 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 |
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.
# GCS Bucket to use for shipping WAL segments with WAL-E | |
# GCS bucket to use for shipping WAL segments with WAL-E |
@@ -91,6 +91,8 @@ configuration: | |||
# kube_iam_role: "" | |||
# log_s3_bucket: "" | |||
# wal_s3_bucket: "" | |||
# wal_gs_bucket: "" | |||
# gcp_credentials: "" |
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.
Could you add these examples also in the configmap? But please with alphabetical sorting there.
Thanks @alfredw33 ! Few minor things:
|
@FxKu - I think I completed what you asked for. Let me know if I've missed anything. |
👍 |
@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. |
👍 |
Thanks @alfredw33 for your contribution and patience ;) |
@FxKu It's my pleasure! 😃 |
Added the ability to support the following environment variables:
to be passed down to the spilo images. This should address #198.