dump.sh and k8sres.go utilize LOGICAL_BACKUP_S3_BUCKET, even on GCP

[rocket357]

• Which image of the operator are you using? registry.opensource.zalan.do/acid/postgres-operator:v1.7.1
• Where do you run it - cloud or metal? Kubernetes or OpenShift? GCP
• Are you running Postgres Operator in production? not yet (for GCP)
• Type of issue? Documentation (variable name confusion)

https://github.com/zalando/postgres-operator/blob/master/pkg/cluster/k8sres.go#L2144

Generated logical backup pod config uses "LOGICAL_BACKUP_S3_BUCKET" even on GCP (also: https://github.com/zalando/postgres-operator/blob/master/docker/logical-backup/dump.sh#L113). This seems confusing, as I would expect it to utilize LOGICAL_BACKUP_GS_BUCKET (following along with WALE_GS_PREFIX/WALG_GS_PREFIX in the spilo project).

It's simple enough to use LOGICAL_BACKUP_S3_BUCKET for now, but it seems it'd make more sense to migrate that to LOGICAL_BACKUP_GS_BUCKET at some point.

[FxKu]

Hm, sounds indeed like someone was too lazy back then implementing a correct set of environment variables. Feel free, to publish a PR where generateLogicalBackupPodEnvVars method reflects the config better - like generateSpiloPodEnvVars does it.

[rocket357]

Working on a PR.

[rocket357]

Apologies for the delays. I see Azure has been handled separately, so this issue and referencing PR will now just focus on GCS logical backup environment variables.

[Lxrdknows77]

@rocket357 @FxKu why its implemented in that way that all logical backup env variables are fetched from global operator Config, so that force to use same s3 for logical backups for all clusters. Just wondering why there is no option to fetch these variables from env variables of postgresql manifest itself (as its done for backup s3 bucket). Something like that: https://github.com/zalando/postgres-operator/pull/1973/files

Upd. I saw it has been added in to the road map for 2.0. Seems ok then. https://github.com/zalando/postgres-operator/milestone/15

[rocket357]

@Lxrdknows77 Ahh, good point. My bad on implementing in the global operator config. Your concerns are noted. (While my company had a use-case for logical backups, we wanted them consistent between regions (logicals for us-east1 go to this bucket, etc...) so I didn't think about that restriction being an issue...but you bring up a valid point).

That said, my company no longer has a use-case for logical backups, so we'll be disabling them in our configs. I've closed the PR for this since the implementation is bad and I'm not going to be working on a replacement implementation.