K8SPG-1057: Allow using etcd as patroni DCS#1647
Conversation
|
|
|
I will wait for the jira ticket to open the PR in the helm charts repo |
|
and another note - this is my first OSS contribute so be gentel 😄 |
egegunes
left a comment
egegunes
left a comment
There was a problem hiding this comment.
@yoav-katz the implementation looks good to me in general. but we definitely need an e2e test that deploys etcd and configures PerconaPGCluster to use it.
|
would love to see this merged! |
…erator into etcd-dcs
|
It seems to fail on cluster deletion due to timeout. I doubt it's something caused by these changes. I retriggered it. |
|
@mayankshah1607 |
… etcd in-place restore
…erator into etcd-dcs
| if cluster.UsesExternalDCS() { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
I think returning both nil here is going to confuse the caller
There was a problem hiding this comment.
I traced every consumer of this return value to be sure. The short version: (nil, nil) is safe here, but it did surface a real (unrelated) stale error message, which I'll fix.
I'm doing the same thing in reconcilePatroniDistributedConfiguration - each DCS-aware reconciler self-guards on UsesExternalDCS() as its first line and returns the zero value when there's nothing to create.
Once you know one of these functions can no-op under external DCS, they all read the same way - so the nil isn't a surprise, it's the documented contract ("Otherwise, it is nil").
The alternative is hoisting the if !UsesExternalDCS() up to the orchestrator - but that just relocates the branch and adds a DCS fork to the one file that currently has none, so my lean is to keep the self-guard for consistency with the sibling reconcilers. but I'm happy to move the check if you feel strongly about it.
|
@yoav-katz friendly ping, we'll release v3.1.0 soon and it'd be great to include this |
|
Hopefully I will get to it this weekend |
…erator into etcd-dcs
# Conflicts: # percona/controller/pgcluster/controller.go # pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go
commit: 7ff5a52 |
|
@egegunes Im not sure if the tests are failuing because a change I did. |
CHANGE DESCRIPTION
Problem:
Patroni supports multiple DCS backends, but the operator hardcodes Kubernetes Endpoints as the only option. This blocks clusters on managed Kubernetes platforms where workloads cannot reach the control plane API.
Cause:
The kubernetes: stanza was hardcoded in the generated Patroni config with no mechanism to select a different backend.
Several other pieces of the operator also assumed k8s DCS: RBAC rules unconditionally granted Endpoints permissions, the primary service routed through Patroni-managed Endpoints objects, and pod role labels/annotations were expected to be set by Patroni itself (which only happens with k8s DCS).
Solution:
Add a spec.patroni.dcs field (type: kubernetes default, type: etcd alternative). The field is immutable after cluster creation, enforced by a CEL validation rule on the CRD.
When type: etcd, the operator:
Reconciliation model (event-driven → periodic):
With k8s DCS the operator is event-driven: Patroni writes leader election, bootstrap (initialize), and pending-restart state into Endpoints objects and pod annotations, and the operator's watches turn those writes into reconciles. Under etcd DCS none of those k8s writes happen, so those watch events never fire. The operator compensates in three ways:
In-place restore DCS cleanup:
An in-place restore must clear stale Patroni state before re-bootstrapping. Under k8s DCS this is done by deleting the Patroni Endpoints objects. Those don't exist under etcd, so instead a one-shot Job runs patronictl remove against etcd to clear the leader lock, member keys, and initialize key. It runs only after Patroni is confirmed stopped (so no live process re-registers itself), and the operator gates the restore state machine on the Job (create → wait → retry-on-failure). The Job reuses the restore's affinity, tolerations, resources, and priorityClassName.
The Kubernetes DCS path is unchanged.
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability