Skip to content

Commit

Permalink
feat: add .spec.replicaCluster.primary field (cloudnative-pg#4388)
Browse files Browse the repository at this point in the history 
This patch introduces changes to the handling of the
`.spec.replica.enabled` field, allowing it to be set to `nil`. Prior to
this patch, only `true` and `false` were permitted, with `false` being
the default. When the `.spec.replica.enabled` field is set to `nil`, the
cluster's classification as a primary or replica cluster will be
determined by two new fields, `.spec.replica.primary` and
`.spec.replica.self`.

The `.spec.replica.self` field specifies the name of the current
cluster, which will be compared with the `.spec.externalClusters[].name`
field. The `.spec.replica.primary` field specifies the name of the
current primary cluster, which will be searched in the same list.
Under the new behavior, when the `.spec.replica.enabled` field is set to
`nil` and the `.spec.replica.primary` and `.spec.replica.self` fields
contain the same value, the cluster is classified as a primary cluster.

Conversely, when the `.spec.replica.enabled` field is `nil` and the
values of `.spec.replica.primary` and `.spec.replica.self` differ, the
cluster will be classified as a replica cluster.

The purpose of this patch is to streamline the configuration process for
a distributed PostgreSQL topology, where the `.spec.externalClusters`
section is synchronized across different data centers.

Closes cloudnative-pg#3942

Signed-off-by: Armando Ruocco <[email protected]>
Signed-off-by: Leonardo Cecchi <[email protected]>
Signed-off-by: Francesco Canovai <[email protected]>
Co-authored-by: Leonardo Cecchi <[email protected]>
Co-authored-by: Francesco Canovai <[email protected]>
  • Loading branch information
@armru @leonardoce @fcanovai
3 people authored Jun 24, 2024
1 parent a68a520 commit ae9a127
Show file tree
Hide file tree
Showing 17 changed files with 548 additions and 113 deletions.
36 changes: 33 additions & 3 deletions api/v1/cluster_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1084,8 +1084,15 @@ type PgBouncerIntegrationStatus struct {

// ReplicaClusterConfiguration encapsulates the configuration of a replica
// cluster
// +kubebuilder:validation:XValidation:rule="!has(self.promotionToken) || size(self.promotionToken) == 0 || !self.enabled",message=Promotion token must be empty on replica clusters
type ReplicaClusterConfiguration struct {
// Self defines the name of this cluster. It is used to determine if this is a primary
// or a replica cluster, comparing it with `primary`
Self string `json:"self,omitempty"`

// Primary defines which Cluster is defined to be the primary in the distributed PostgreSQL cluster, based on the
// topology specified in externalClusters
Primary string `json:"primary,omitempty"`

// The name of the external cluster which is the replication origin
// +kubebuilder:validation:MinLength=1
Source string `json:"source"`
Expand All @@ -1094,7 +1101,7 @@ type ReplicaClusterConfiguration struct {
// existing cluster. Replica cluster can be created from a recovery
// object store or via streaming through pg_basebackup.
// Refer to the Replica clusters page of the documentation for more information.
Enabled bool `json:"enabled"`
Enabled *bool `json:"enabled,omitempty"`

// A demotion token generated by an external cluster used to
// check if the promotion requirements are met.
Expand Down Expand Up @@ -3243,7 +3250,30 @@ func (cluster Cluster) ExternalCluster(name string) (ExternalCluster, bool) {

// IsReplica checks if this is a replica cluster or not
func (cluster Cluster) IsReplica() bool {
return cluster.Spec.ReplicaCluster != nil && cluster.Spec.ReplicaCluster.Enabled
// Before introducing the "primary" field, the
// "enabled" parameter was declared as a "boolean"
// and was not declared "omitempty".
//
// Legacy replica clusters will have the "replica" stanza
// and the "enabled" field set explicitly to true.
//
// The following code is designed to not change the
// previous semantics.
r := cluster.Spec.ReplicaCluster
if r == nil {
return false
}

if r.Enabled != nil {
return *r.Enabled
}

clusterName := r.Self
if len(clusterName) == 0 {
clusterName = cluster.Name
}

return clusterName != r.Primary
}

var slotNameNegativeRegex = regexp.MustCompile("[^a-z0-9_]+")
Expand Down
136 changes: 129 additions & 7 deletions api/v1/cluster_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ var _ = Describe("Barman Endpoint CA for replica cluster", func() {
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Source: "testSource",
Enabled: true,
Enabled: ptr.To(true),
},
},
}
Expand Down Expand Up @@ -817,7 +817,7 @@ var _ = Describe("Barman Endpoint CA for replica cluster", func() {
},
ReplicaCluster: &ReplicaClusterConfiguration{
Source: "testReplica",
Enabled: true,
Enabled: ptr.To(true),
},
},
}
Expand Down Expand Up @@ -1008,7 +1008,7 @@ var _ = Describe("Cluster ShouldRecoveryCreateApplicationDatabase", func() {
})

It("should return false if the cluster is a replica", func() {
cluster.Spec.ReplicaCluster = &ReplicaClusterConfiguration{Enabled: true}
cluster.Spec.ReplicaCluster = &ReplicaClusterConfiguration{Enabled: ptr.To(true)}
result := cluster.ShouldRecoveryCreateApplicationDatabase()
Expect(result).To(BeFalse())
})
Expand Down Expand Up @@ -1238,7 +1238,7 @@ var _ = Describe("ShouldPromoteFromReplicaCluster", func() {
cluster := &Cluster{
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Enabled: true,
Enabled: ptr.To(true),
PromotionToken: "ABC",
},
},
Expand All @@ -1250,7 +1250,7 @@ var _ = Describe("ShouldPromoteFromReplicaCluster", func() {
cluster := &Cluster{
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Enabled: true,
Enabled: ptr.To(true),
},
},
}
Expand All @@ -1270,7 +1270,7 @@ var _ = Describe("ShouldPromoteFromReplicaCluster", func() {
cluster := &Cluster{
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Enabled: true,
Enabled: ptr.To(true),
PromotionToken: "ABC",
},
},
Expand All @@ -1285,7 +1285,7 @@ var _ = Describe("ShouldPromoteFromReplicaCluster", func() {
cluster := &Cluster{
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Enabled: true,
Enabled: ptr.To(true),
PromotionToken: "ABC",
},
},
Expand All @@ -1296,3 +1296,125 @@ var _ = Describe("ShouldPromoteFromReplicaCluster", func() {
Expect(cluster.ShouldPromoteFromReplicaCluster()).To(BeTrue())
})
})

var _ = Describe("IsReplica", func() {
Describe("using the legacy API", func() {
replicaClusterOldAPI := &Cluster{
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Enabled: ptr.To(true),
Source: "source-cluster",
},
},
}

primaryClusterOldAPI := &Cluster{
Spec: ClusterSpec{
ReplicaCluster: nil,
},
}

primaryClusterOldAPIExplicit := &Cluster{
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Enabled: ptr.To(false),
Source: "source-cluster",
},
},
}

DescribeTable(
"doesn't change the semantics",
func(resource *Cluster, isReplica bool) {
Expect(resource.IsReplica()).To(Equal(isReplica))
},
Entry(
"replica cluster with the old API",
replicaClusterOldAPI, true),
Entry(
"primary cluster with the old API",
primaryClusterOldAPI, false),
Entry(
"primary cluster with the old API, explicitly disabling replica",
primaryClusterOldAPIExplicit, false),
)
})

Describe("using the new API, with an implicit self", func() {
primaryClusterNewAPI := &Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-1",
},
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Primary: "cluster-1",
Enabled: nil,
Source: "source-cluster",
},
},
}

replicaClusterNewAPI := &Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-1",
},
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Primary: "cluster-2",
Enabled: nil,
Source: "source-cluster",
},
},
}

DescribeTable(
"uses the primary cluster name",
func(resource *Cluster, isReplica bool) {
Expect(resource.IsReplica()).To(Equal(isReplica))
},
Entry(
"primary cluster",
primaryClusterNewAPI, false),
Entry(
"replica cluster",
replicaClusterNewAPI, true),
)
})

Describe("using the new API, with an explicit self", func() {
primaryClusterNewAPI := &Cluster{
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Self: "cluster-1",
Primary: "cluster-1",
Enabled: nil,
Source: "source-cluster",
},
},
}

replicaClusterNewAPI := &Cluster{
Spec: ClusterSpec{
ReplicaCluster: &ReplicaClusterConfiguration{
Self: "cluster-1",
Primary: "cluster-2",
Enabled: nil,
Source: "source-cluster",
},
},
}

DescribeTable(
"uses the primary cluster name",
func(resource *Cluster, isReplica bool) {
Expect(resource.IsReplica()).To(Equal(isReplica))
},
Entry(
"primary cluster",
primaryClusterNewAPI, false),
Entry(
"replica cluster",
replicaClusterNewAPI, true),
)
})
})
Loading

0 comments on commit ae9a127

Please sign in to comment.