add the possibility to create a standby cluster that streams from a remote primary #1544
Conversation
machine424
requested review from
CyberDem0n,
erthalion,
FxKu,
Jan-M,
RafiaSabih and
sdudoladov
as code owners
|
I ping you @FxKu as you've started reviewing the other PR |
|
@Jan-M maybe? |
|
@machine424 thanks for the work! This is a great patroni feature you're bringing here, looking forward to it. Are you planning to continue working on it? |
machine424
force-pushed
the
stand
branch
4 times, most recently
from
30bcb26
to
eb990fd
Compare
|
I just figured out I never pushed my changes. I've just rebased. Before adjusting the user's doc, we'll need, in another PR I think, to add support of Services for As a workaround (we run this machine424@02c415a on prod since july 2021), we create an extra Service that points to |
|
|
||
| * **s3_wal_path** | ||
| the url to S3 bucket containing the WAL archive of the remote primary. | ||
| Optional, but `s3_wal_path` or `gs_wal_path` is required. | ||
| Required when the `standby` section is present even when `standby_host` is set. |
There was a problem hiding this comment.
Hm, it should not be required if host or gs_wal_path is set. We can work with oneOf in the CRD schema validation, I think.
standby:
type: object
properties:
s3_wal_path:
type: string
gs_wal_path:
type: string
standby_host:
type: string
standby_port:
type: string
oneOf:
- required:
- s3_wal_path
- required:
- gs_wal_path
- required:
- standby_host
| spec.StandbyCluster.GSWalPath == "" { | ||
| return nil, fmt.Errorf("one of s3_wal_path or gs_wal_path must be set for standby cluster") | ||
| if spec.StandbyCluster != nil { | ||
| if spec.StandbyCluster.S3WalPath == "" && spec.StandbyCluster.GSWalPath == "" && spec.StandbyCluster.StandbyHost == "" { |
There was a problem hiding this comment.
when one of the three options is required, I guess this extra if would be obsolete then
| envPos: 0, | ||
| }, | ||
| { | ||
| subTest: "from remote primary", |
There was a problem hiding this comment.
could add here that this also tests host taking precedence over wal path setting. Maybe make it an extra test with right comment.
|
Thanks @machine424. Would be happy to merge it for the next release. Have a look at my comments. There seems to be one conflict left. |
|
@machine424 can you resolve the conflict and reflect my comments? |
|
Hello @FxKu, I've started looking at this but I discovered you already took care of it here #1830 Thanks and sorry I didn't had time before. Should I open an issue for the limitation I talked about in #1544 (comment)? (of document it?) I'd try to address it when I have time. You can close this PR when you want. |
|
@machine424 thanks for getting back. Regarding the extra service: Is it necessary?I know that the operator just labels the standby leader pod / service / endpoint as master. |
|
Yes, the operator will create two services for the standby cluster: The standby cluster Pod will not edit the Endpoints of Am I wrong? |
Hello,
I don't know if we should add any tests for this. -> I'll add some unit tests (DONE)
Still need to adjust the user's doc.
Do without the blocking point in here: #804
We suppose both clusters have the same standby's password for now.