From 01312ec3957ab08b3d97c80364afddb112c39ee7 Mon Sep 17 00:00:00 2001 From: melancholictheory <61789920+melancholictheory@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:22:00 +0300 Subject: [PATCH] feat: support taking backups from a standby (pgBackRest multi-host TLS) Backups that CloudNativePG schedules on a standby currently fail at stanza-create with pgBackRest exit 56 (unable to find primary cluster), because the plugin only ever configures a single local pgBackRest host. This forces multi-instance clusters back to target: primary and puts all backup I/O on the primary. This adds an opt-in backupStandby configuration on the Archive: an "enabled" switch, "injectSAN" and "injectService" opt-outs for the injection the plugin will perform, and a "serviceName" override defaulting to -pgbackrest. When a backup runs on a standby, the current primary is configured as a second pgBackRest host over TLS (pg2-host-type=tls) reachable through that service, and --backup-standby is passed, so pgBackRest coordinates pg_backup_start/stop on the primary while copying data from the standby. The pgBackRest TLS server port is not configurable: a user-provided service has to expose the default one. Whether a given backup runs on a standby is decided by CloudNativePG via the backup target, not by the Archive: the plugin only checks whether it is running on a replica, which needs nothing but currentPrimary from the Cluster. A cluster without a current primary is reported as an error. stanza-create is also given the primary peer so it can initialise from a standby, without --backup-standby. The service and certificate SAN injection and the pgBackRest TLS server lifecycle are not implemented yet. Enabling the feature while the plugin is expected to inject them fails with an explicit error, so the incomplete path cannot be used by accident, and the README documents how to try it with both provided out of band. Refs: #103 Signed-off-by: melancholictheory <61789920+melancholictheory@users.noreply.github.com> --- README.md | 25 +++ .../pgbackrest.cnpg.opera.com_archives.yaml | 30 ++++ internal/cnpgi/instance/backup.go | 70 ++++++++ internal/cnpgi/instance/backup_test.go | 116 +++++++++++++ internal/cnpgi/instance/suite_test.go | 30 ++++ internal/pgbackrest/api/config.go | 71 ++++++++ .../pgbackrest/api/zz_generated.deepcopy.go | 30 ++++ internal/pgbackrest/backup/backup.go | 25 +++ internal/pgbackrest/backup/backup_test.go | 72 ++++++++ internal/pgbackrest/command/standby.go | 159 ++++++++++++++++++ internal/pgbackrest/command/standby_test.go | 93 ++++++++++ manifest.yaml | 30 ++++ 12 files changed, 751 insertions(+) create mode 100644 internal/cnpgi/instance/backup_test.go create mode 100644 internal/cnpgi/instance/suite_test.go create mode 100644 internal/pgbackrest/command/standby.go create mode 100644 internal/pgbackrest/command/standby_test.go diff --git a/README.md b/README.md index 83cb41e..6a457ef 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,31 @@ This configuration enables both WAL archiving and data directory backups. > Archiving will only start working after at least one backup is created. That's due to > the stanza creation process which currently is only executed on backups. +### Backups From a Standby (experimental, incomplete) + +`backupStandby` offloads backup I/O to a standby using pgBackRest multi-host TLS. +Whether a backup runs on a standby is decided by CloudNativePG through the backup +target; the plugin adds the primary as a second pgBackRest host when it finds +itself on a replica. + +```yaml +spec: + configuration: + backupStandby: + enabled: true + injectService: false + injectSAN: false + # serviceName: my-pgbackrest # defaults to -pgbackrest +``` + +> [!WARNING] +> This is incomplete. The plugin does not yet inject the headless service or the +> certificate SAN, and it does not yet run the pgBackRest TLS server on the +> instances. Enabling it with `injectService` or `injectSAN` left at their default +> fails with an explicit error. To try it, provide the service (exposing the +> pgBackRest TLS server port) and the SAN yourself, run the server, and set both +> options to `false`. See [issue #103](https://github.com/operasoftware/cnpg-plugin-pgbackrest/issues/103). + ### Performing a Base Backup Once WAL archiving is enabled, the cluster is ready for backups. To create a diff --git a/config/crd/bases/pgbackrest.cnpg.opera.com_archives.yaml b/config/crd/bases/pgbackrest.cnpg.opera.com_archives.yaml index f9b60dd..99cad2d 100644 --- a/config/crd/bases/pgbackrest.cnpg.opera.com_archives.yaml +++ b/config/crd/bases/pgbackrest.cnpg.opera.com_archives.yaml @@ -43,6 +43,36 @@ spec: description: PgbackrestConfiguration is the configuration of all pgBackRest operations properties: + backupStandby: + description: |- + BackupStandby, when enabled, offloads backups to a standby instance using + pgBackRest multi-host TLS. See BackupStandbyConfiguration. + properties: + enabled: + description: Enabled turns on backup-from-standby. + type: boolean + injectSAN: + default: true + description: |- + InjectSAN controls whether the plugin adds the pgBackRest service DNS name to + the cluster server certificate (serverAltDNSNames). Defaults to true; set to + false to manage the SAN yourself. + type: boolean + injectService: + default: true + description: |- + InjectService controls whether the plugin injects the headless service that + exposes the pgBackRest TLS server port on the instances. Defaults to true; set + to false to manage that service yourself. + type: boolean + serviceName: + description: |- + ServiceName is the headless service that resolves to the primary's pgBackRest + TLS server. Defaults to "-pgbackrest", the service the plugin manages; + set it when you provide the service yourself. The port is not configurable: a + user-provided service must expose DefaultServerPort. + type: string + type: object compression: description: |- Compress a WAL file before sending it to the object store. Available diff --git a/internal/cnpgi/instance/backup.go b/internal/cnpgi/instance/backup.go index 979d0b3..0b820ca 100644 --- a/internal/cnpgi/instance/backup.go +++ b/internal/cnpgi/instance/backup.go @@ -19,9 +19,11 @@ package instance import ( "context" + "errors" "fmt" "time" + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" "github.com/cloudnative-pg/cloudnative-pg/pkg/postgres" "github.com/cloudnative-pg/cnpg-i-machinery/pkg/pluginhelper/decoder" "github.com/cloudnative-pg/cnpg-i/pkg/backup" @@ -33,8 +35,10 @@ import ( pgbackrestv1 "github.com/operasoftware/cnpg-plugin-pgbackrest/api/v1" "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/cnpgi/metadata" "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/cnpgi/operator/config" + pgbackrestApi "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/api" pgbackrestBackup "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/backup" "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/catalog" + pgbackrestCommand "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/command" pgbackrestCredentials "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/credentials" "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/utils" ) @@ -102,6 +106,20 @@ func (b BackupServiceImplementation) Backup( b.PGDataPath, ) + // When backup-from-standby is enabled and this instance is a standby, point + // pgBackRest at the current primary so both stanza-create and the backup can + // coordinate control operations there. + standbyTopology, err := b.resolveStandbyTopology(ctx, configuration.Cluster, &archive.Spec.Configuration) + if err != nil { + contextLogger.Error(err, "while resolving backup-from-standby topology") + return nil, err + } + if standbyTopology != nil { + contextLogger.Info("Taking backup from standby", + "primaryHost", standbyTopology.PrimaryHost) + backupCmd = backupCmd.WithStandbyBackup(standbyTopology) + } + // We need to connect to PostgreSQL and to do that we need // PGHOST (and the like) to be available osEnvironment := utils.SanitizedEnviron() @@ -167,3 +185,55 @@ func (b BackupServiceImplementation) Backup( }, }, nil } + +// errNoStandbyInjection is returned when backup-from-standby is enabled while the +// plugin is still expected to inject the service and the certificate SAN, which is +// not implemented yet. +var errNoStandbyInjection = errors.New( + "backup-from-standby is experimental and incomplete: the plugin does not inject the pgBackRest " + + "service and certificate SAN yet. Provide both yourself and set injectService and injectSAN to false") + +// resolveStandbyTopology returns the topology needed to take this backup from a +// standby, or nil when a normal (local/primary) backup should be taken. It +// returns nil when the feature is disabled, or when this instance is the +// primary (or no primary is known yet). When this instance is a standby with a +// known primary, it resolves the primary pod's IP so pgBackRest can reach the +// primary's TLS server. +func (b BackupServiceImplementation) resolveStandbyTopology( + ctx context.Context, + cluster *cnpgv1.Cluster, + cfg *pgbackrestApi.PgbackrestConfiguration, +) (*pgbackrestCommand.StandbyBackupTopology, error) { + contextLogger := log.FromContext(ctx) + + enabled := cfg.IsBackupStandbyEnabled() + currentPrimary := cluster.Status.CurrentPrimary + onStandby, err := pgbackrestCommand.ShouldConfigurePrimaryPeer(enabled, currentPrimary, b.InstanceName) + if err != nil { + return nil, err + } + if !onStandby { + if enabled { + contextLogger.Info( + "backup-from-standby enabled but this instance is not a standby; taking a local backup", + "instance", b.InstanceName, "currentPrimary", currentPrimary) + } + return nil, nil + } + + // The service and SAN injection is not implemented yet, so the feature only works + // when both are provided out of band. Fail fast instead of letting pgBackRest fail + // with a connection or certificate error. + if cfg.BackupStandby.ShouldInjectService() || cfg.BackupStandby.ShouldInjectSAN() { + return nil, errNoStandbyInjection + } + + return &pgbackrestCommand.StandbyBackupTopology{ + PrimaryHost: cfg.BackupStandby.GetServiceName(cluster.Name), + PrimaryPort: pgbackrestCommand.DefaultServerPort, + PrimaryPGData: b.PGDataPath, + CertFile: pgbackrestCommand.DefaultTLSCertFile, + KeyFile: pgbackrestCommand.DefaultTLSKeyFile, + CAFile: pgbackrestCommand.DefaultTLSCAFile, + }, nil +} diff --git a/internal/cnpgi/instance/backup_test.go b/internal/cnpgi/instance/backup_test.go new file mode 100644 index 0000000..cfad224 --- /dev/null +++ b/internal/cnpgi/instance/backup_test.go @@ -0,0 +1,116 @@ +/* +Copyright The CloudNativePG Contributors +Copyright 2025, Opera Norway AS + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package instance + +import ( + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + pgbackrestApi "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/api" + pgbackrestCommand "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/command" +) + +var _ = Describe("resolveStandbyTopology", func() { + const ( + ns = "test-ns" + primaryPod = "cluster-1" + standby = "cluster-2" + pgData = "/var/lib/postgresql/data/pgdata" + ) + + newCluster := func(currentPrimary string) *cnpgv1.Cluster { + c := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: "cluster"}} + c.Status.CurrentPrimary = currentPrimary + return c + } + + newImpl := func(instanceName string, objs ...client.Object) BackupServiceImplementation { + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + return BackupServiceImplementation{Client: fakeClient, InstanceName: instanceName, PGDataPath: pgData} + } + + // The injection is not implemented yet, so a usable configuration opts out of it. + cfg := func(enabled bool) *pgbackrestApi.PgbackrestConfiguration { + no := false + return &pgbackrestApi.PgbackrestConfiguration{ + BackupStandby: &pgbackrestApi.BackupStandbyConfiguration{ + Enabled: enabled, + InjectService: &no, + InjectSAN: &no, + }, + } + } + + It("returns nil when the feature is disabled", func(ctx SpecContext) { + impl := newImpl(standby) + topo, err := impl.resolveStandbyTopology(ctx, newCluster(primaryPod), cfg(false)) + Expect(err).ToNot(HaveOccurred()) + Expect(topo).To(BeNil()) + }) + + It("returns nil (local backup) when this instance is the primary", func(ctx SpecContext) { + impl := newImpl(primaryPod) + topo, err := impl.resolveStandbyTopology(ctx, newCluster(primaryPod), cfg(true)) + Expect(err).ToNot(HaveOccurred()) + Expect(topo).To(BeNil()) + }) + + It("errors when no primary is known", func(ctx SpecContext) { + impl := newImpl(standby) + _, err := impl.resolveStandbyTopology(ctx, newCluster(""), cfg(true)) + Expect(err).To(MatchError(pgbackrestCommand.ErrNoCurrentPrimary)) + }) + + It("builds the topology from the configured service when on a standby", func(ctx SpecContext) { + impl := newImpl(standby) + topo, err := impl.resolveStandbyTopology(ctx, newCluster(primaryPod), cfg(true)) + Expect(err).ToNot(HaveOccurred()) + Expect(topo).ToNot(BeNil()) + Expect(*topo).To(Equal(pgbackrestCommand.StandbyBackupTopology{ + PrimaryHost: "cluster" + pgbackrestApi.ServiceNameSuffix, + PrimaryPort: pgbackrestCommand.DefaultServerPort, + PrimaryPGData: pgData, + CertFile: pgbackrestCommand.DefaultTLSCertFile, + KeyFile: pgbackrestCommand.DefaultTLSKeyFile, + CAFile: pgbackrestCommand.DefaultTLSCAFile, + })) + }) + + It("honours an explicit service name", func(ctx SpecContext) { + conf := cfg(true) + conf.BackupStandby.ServiceName = "my-pgbackrest" + impl := newImpl(standby) + topo, err := impl.resolveStandbyTopology(ctx, newCluster(primaryPod), conf) + Expect(err).ToNot(HaveOccurred()) + Expect(topo.PrimaryHost).To(Equal("my-pgbackrest")) + }) + + It("fails fast while the service and SAN injection is not implemented", func(ctx SpecContext) { + conf := &pgbackrestApi.PgbackrestConfiguration{ + BackupStandby: &pgbackrestApi.BackupStandbyConfiguration{Enabled: true}, + } + impl := newImpl(standby) + _, err := impl.resolveStandbyTopology(ctx, newCluster(primaryPod), conf) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("injectService")) + }) +}) diff --git a/internal/cnpgi/instance/suite_test.go b/internal/cnpgi/instance/suite_test.go new file mode 100644 index 0000000..dc4846e --- /dev/null +++ b/internal/cnpgi/instance/suite_test.go @@ -0,0 +1,30 @@ +/* +Copyright The CloudNativePG Contributors +Copyright 2025, Opera Norway AS + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package instance + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestInstance(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Instance suite") +} diff --git a/internal/pgbackrest/api/config.go b/internal/pgbackrest/api/config.go index e56423f..b4e6bde 100644 --- a/internal/pgbackrest/api/config.go +++ b/internal/pgbackrest/api/config.go @@ -351,6 +351,68 @@ const ( StanzaCreateDisabled StanzaCreatePolicy = "Disabled" ) +// BackupStandbyConfiguration configures taking backups from a standby instead of +// the primary, offloading the backup I/O from the primary. It relies on a +// pgBackRest TLS server running on the instances (pgBackRest multi-host TLS). When +// enabled, the plugin provisions the resources this needs unless the injection +// opt-outs below turn that off. +// +// Whether a given backup actually runs on a standby is decided by CloudNativePG +// via the Backup/ScheduledBackup target, not here: the plugin adds the primary as a +// second host and passes --backup-standby only when the backup lands on a replica. +// +// Experimental, see https://github.com/operasoftware/cnpg-plugin-pgbackrest/issues/103 +type BackupStandbyConfiguration struct { + // Enabled turns on backup-from-standby. + // +optional + Enabled bool `json:"enabled,omitempty"` + + // InjectService controls whether the plugin injects the headless service that + // exposes the pgBackRest TLS server port on the instances. Defaults to true; set + // to false to manage that service yourself. + // +kubebuilder:default=true + // +optional + InjectService *bool `json:"injectService,omitempty"` + + // InjectSAN controls whether the plugin adds the pgBackRest service DNS name to + // the cluster server certificate (serverAltDNSNames). Defaults to true; set to + // false to manage the SAN yourself. + // +kubebuilder:default=true + // +optional + InjectSAN *bool `json:"injectSAN,omitempty"` + + // ServiceName is the headless service that resolves to the primary's pgBackRest + // TLS server. Defaults to "-pgbackrest", the service the plugin manages; + // set it when you provide the service yourself. The port is not configurable: a + // user-provided service must expose DefaultServerPort. + // +optional + ServiceName string `json:"serviceName,omitempty"` +} + +// ServiceNameSuffix is appended to the cluster name to build the default pgBackRest +// service name. +const ServiceNameSuffix = "-pgbackrest" + +// GetServiceName returns the pgBackRest service name for the given cluster. +func (s *BackupStandbyConfiguration) GetServiceName(clusterName string) string { + if s == nil || s.ServiceName == "" { + return clusterName + ServiceNameSuffix + } + return s.ServiceName +} + +// ShouldInjectService reports whether the plugin should inject the pgBackRest +// headless service (defaults to true when unset). +func (s *BackupStandbyConfiguration) ShouldInjectService() bool { + return s == nil || s.InjectService == nil || *s.InjectService +} + +// ShouldInjectSAN reports whether the plugin should inject the pgBackRest service +// SAN into the cluster server certificate (defaults to true when unset). +func (s *BackupStandbyConfiguration) ShouldInjectSAN() bool { + return s == nil || s.InjectSAN == nil || *s.InjectSAN +} + // PgbackrestConfiguration is the configuration of all pgBackRest operations type PgbackrestConfiguration struct { Repositories []PgbackrestRepository `json:"repositories"` @@ -398,6 +460,10 @@ type PgbackrestConfiguration struct { // +kubebuilder:default=OnFirstArchive // +optional CreateStanza StanzaCreatePolicy `json:"createStanza,omitempty"` + // BackupStandby, when enabled, offloads backups to a standby instance using + // pgBackRest multi-host TLS. See BackupStandbyConfiguration. + // +optional + BackupStandby *BackupStandbyConfiguration `json:"backupStandby,omitempty"` } // GetCreateStanzaPolicy returns the configured stanza creation policy, defaulting to @@ -420,6 +486,11 @@ func (c *PgbackrestConfiguration) ShouldCreateStanzaOnBackup() bool { return c.GetCreateStanzaPolicy() != StanzaCreateDisabled } +// IsBackupStandbyEnabled reports whether backup-from-standby is enabled. +func (c *PgbackrestConfiguration) IsBackupStandbyEnabled() bool { + return c.BackupStandby != nil && c.BackupStandby.Enabled +} + // ArePopulated checks if the passed set of credentials contains // something func (credentials PgbackrestCredentials) ArePopulated() bool { diff --git a/internal/pgbackrest/api/zz_generated.deepcopy.go b/internal/pgbackrest/api/zz_generated.deepcopy.go index 7866706..92a1336 100644 --- a/internal/pgbackrest/api/zz_generated.deepcopy.go +++ b/internal/pgbackrest/api/zz_generated.deepcopy.go @@ -24,6 +24,31 @@ import ( pkgapi "github.com/cloudnative-pg/machinery/pkg/api" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BackupStandbyConfiguration) DeepCopyInto(out *BackupStandbyConfiguration) { + *out = *in + if in.InjectService != nil { + in, out := &in.InjectService, &out.InjectService + *out = new(bool) + **out = **in + } + if in.InjectSAN != nil { + in, out := &in.InjectSAN, &out.InjectSAN + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BackupStandbyConfiguration. +func (in *BackupStandbyConfiguration) DeepCopy() *BackupStandbyConfiguration { + if in == nil { + return nil + } + out := new(BackupStandbyConfiguration) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DataBackupConfiguration) DeepCopyInto(out *DataBackupConfiguration) { *out = *in @@ -126,6 +151,11 @@ func (in *PgbackrestConfiguration) DeepCopyInto(out *PgbackrestConfiguration) { *out = new(LogConfiguration) **out = **in } + if in.BackupStandby != nil { + in, out := &in.BackupStandby, &out.BackupStandby + *out = new(BackupStandbyConfiguration) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PgbackrestConfiguration. diff --git a/internal/pgbackrest/backup/backup.go b/internal/pgbackrest/backup/backup.go index e815f66..2b5e80a 100644 --- a/internal/pgbackrest/backup/backup.go +++ b/internal/pgbackrest/backup/backup.go @@ -40,6 +40,11 @@ type Command struct { configuration *pgbackrestApi.PgbackrestConfiguration backupConfig *cnpgApiV1.BackupPluginConfiguration pgDataDirectory string + // standbyTopology, when non-nil, makes the backup run from a standby: the + // current primary is configured as a second pgBackRest host over TLS and + // --backup-standby is passed. When nil the command targets the local + // instance exactly as before. + standbyTopology *pgbackrestCommand.StandbyBackupTopology } // NewBackupCommand creates a new pgbackrest backup command @@ -55,6 +60,14 @@ func NewBackupCommand( } } +// WithStandbyBackup configures the command to take the backup from a standby, +// coordinating control operations on the primary described by topology. When +// not called, the command targets the local instance as before. +func (b *Command) WithStandbyBackup(topology *pgbackrestCommand.StandbyBackupTopology) *Command { + b.standbyTopology = topology + return b +} + // GetDataConfiguration gets the configuration in the `Data` object of the pgbackrest configuration func (b *Command) GetDataConfiguration( options []string, @@ -146,6 +159,11 @@ func (b *Command) GetPgbackrestBackupOptions( return nil, err } + if b.standbyTopology != nil { + options = pgbackrestCommand.AppendStandbyPrimaryHostOptions(options, *b.standbyTopology) + options = append(options, pgbackrestCommand.BackupStandbyOption()) + } + options, err = pgbackrestCommand.AppendLogOptionsFromConfiguration(ctx, options, b.configuration) if err != nil { return nil, err @@ -199,6 +217,13 @@ func (b *Command) getStanzaCreateOptions( return nil, err } + // When taking the backup from a standby, stanza-create must also be able to + // reach the primary, so add it as a second host. --backup-standby is a + // backup-only option and is intentionally not added here. + if b.standbyTopology != nil { + options = pgbackrestCommand.AppendStandbyPrimaryHostOptions(options, *b.standbyTopology) + } + options, err = pgbackrestCommand.AppendLogOptionsFromConfiguration( ctx, options, diff --git a/internal/pgbackrest/backup/backup_test.go b/internal/pgbackrest/backup/backup_test.go index 2df9921..d79e970 100644 --- a/internal/pgbackrest/backup/backup_test.go +++ b/internal/pgbackrest/backup/backup_test.go @@ -27,6 +27,7 @@ import ( "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/cnpgi/metadata" pgbackrestApi "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/api" pgbackrestCatalog "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/catalog" + pgbackrestCommand "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/command" ) var _ = Describe("GetPgbackrestBackupOptions", func() { @@ -144,4 +145,75 @@ var _ = Describe("GetPgbackrestBackupOptions", func() { ContainSubstring(" --repo1-retention-history 9"), ) }) + + It("should configure the primary peer and --backup-standby for a backup from a standby", func(ctx SpecContext) { + backupConfig := cnpgApiV1.BackupPluginConfiguration{Name: metadata.PluginName} + topology := &pgbackrestCommand.StandbyBackupTopology{ + PrimaryHost: "10.0.0.5", + PrimaryPort: 8432, + PrimaryPGData: pgDataDir, + CertFile: "/certs/tls.crt", + KeyFile: "/certs/tls.key", + CAFile: "/certs/ca.crt", + } + command := NewBackupCommand(pluginConfig, &backupConfig, pgDataDir).WithStandbyBackup(topology) + + options, err := command.GetPgbackrestBackupOptions(ctx, backupName, stanza) + + Expect(err).ToNot(HaveOccurred()) + Expect(strings.Join(options, " ")). + To( + Equal( + fmt.Sprintf("backup --annotation %s=%s --repo1-type s3 --repo1-s3-bucket bucket-name --repo1-path / --pg1-path %s --pg1-user postgres --pg1-socket-path /controller/run/ --pg2-host 10.0.0.5 --pg2-host-type tls --pg2-host-port 8432 --pg2-host-cert-file /certs/tls.crt --pg2-host-key-file /certs/tls.key --pg2-host-ca-file /certs/ca.crt --pg2-path %s --pg2-user postgres --pg2-socket-path /controller/run/ --backup-standby=y --log-level-stderr warn --log-level-console off --stanza %s --lock-path /controller/tmp/pgbackrest --no-archive-check", pgbackrestCatalog.BackupNameAnnotation, backupName, pgDataDir, pgDataDir, stanza), + )) + }) + + It("should not add standby options for a normal (primary) backup", func(ctx SpecContext) { + backupConfig := cnpgApiV1.BackupPluginConfiguration{Name: metadata.PluginName} + command := NewBackupCommand(pluginConfig, &backupConfig, pgDataDir) + + options, err := command.GetPgbackrestBackupOptions(ctx, backupName, stanza) + + Expect(err).ToNot(HaveOccurred()) + joined := strings.Join(options, " ") + Expect(joined).ToNot(ContainSubstring("--pg2-")) + Expect(joined).ToNot(ContainSubstring("--backup-standby")) + }) + + It("should add the primary peer to stanza-create but not --backup-standby", func(ctx SpecContext) { + backupConfig := cnpgApiV1.BackupPluginConfiguration{Name: metadata.PluginName} + topology := &pgbackrestCommand.StandbyBackupTopology{ + PrimaryHost: "10.0.0.5", + PrimaryPort: 8432, + PrimaryPGData: pgDataDir, + CertFile: "/certs/tls.crt", + KeyFile: "/certs/tls.key", + CAFile: "/certs/ca.crt", + } + command := NewBackupCommand(pluginConfig, &backupConfig, pgDataDir).WithStandbyBackup(topology) + + options, err := command.getStanzaCreateOptions(ctx, stanza) + + Expect(err).ToNot(HaveOccurred()) + Expect(strings.Join(options, " ")). + To( + Equal( + fmt.Sprintf("stanza-create --repo1-type s3 --repo1-s3-bucket bucket-name --repo1-path / --pg1-path %s --pg1-user postgres --pg1-socket-path /controller/run/ --pg2-host 10.0.0.5 --pg2-host-type tls --pg2-host-port 8432 --pg2-host-cert-file /certs/tls.crt --pg2-host-key-file /certs/tls.key --pg2-host-ca-file /certs/ca.crt --pg2-path %s --pg2-user postgres --pg2-socket-path /controller/run/ --log-level-stderr warn --log-level-console off --stanza %s --lock-path /controller/tmp/pgbackrest", pgDataDir, pgDataDir, stanza), + )) + Expect(strings.Join(options, " ")).ToNot(ContainSubstring("--backup-standby")) + }) + + It("should not add standby options to a normal stanza-create", func(ctx SpecContext) { + backupConfig := cnpgApiV1.BackupPluginConfiguration{Name: metadata.PluginName} + command := NewBackupCommand(pluginConfig, &backupConfig, pgDataDir) + + options, err := command.getStanzaCreateOptions(ctx, stanza) + + Expect(err).ToNot(HaveOccurred()) + joined := strings.Join(options, " ") + Expect(joined).To(HavePrefix("stanza-create ")) + Expect(joined).ToNot(ContainSubstring("--pg2-")) + Expect(joined).ToNot(ContainSubstring("--backup-standby")) + }) + }) diff --git a/internal/pgbackrest/command/standby.go b/internal/pgbackrest/command/standby.go new file mode 100644 index 0000000..194fbd6 --- /dev/null +++ b/internal/pgbackrest/command/standby.go @@ -0,0 +1,159 @@ +/* +Copyright The CloudNativePG Contributors +Copyright 2025, Opera Norway AS + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package command + +import ( + "errors" + "fmt" + "strconv" + + "github.com/operasoftware/cnpg-plugin-pgbackrest/internal/pgbackrest/utils" +) + +// Networking and filesystem defaults for the experimental "backup from +// standby" feature. These mirror pgBackRest's TLS server defaults and the +// sidecar's existing /controller layout. Certificate provisioning is +// intentionally left as a follow-up (see issue #103): the paths below are the +// contract the plugin expects the pgBackRest TLS certificates to be mounted at. +const ( + // DefaultServerPort is the port the pgBackRest TLS server listens on, and + // the port a standby uses to reach the primary's server. It matches + // pgBackRest's tls-server-port default. + DefaultServerPort = 8432 + + // DefaultTLSCertFile, DefaultTLSKeyFile and DefaultTLSCAFile are the mount + // paths the plugin expects the pgBackRest TLS client/server certificate + // material at inside every instance sidecar. + DefaultTLSCertFile = "/controller/certificates/pgbackrest/tls.crt" + DefaultTLSKeyFile = "/controller/certificates/pgbackrest/tls.key" + DefaultTLSCAFile = "/controller/certificates/pgbackrest/ca.crt" + + // localSocketPath is the unix socket every CNPG instance pod exposes for + // its local PostgreSQL. + localSocketPath = "/controller/run/" + + // primaryHostIndex is the zero-based pgBackRest pg-host index used for the + // remote primary. The local instance keeps index 0 (pg1); the primary is + // added as pg2. + primaryHostIndex = 1 +) + +// StandbyBackupTopology describes how a standby reaches the current primary's +// pgBackRest TLS server to coordinate a backup taken from the standby. +type StandbyBackupTopology struct { + // PrimaryHost is the address (pod IP or DNS) of the current primary running + // a pgBackRest TLS server. + PrimaryHost string + // PrimaryPort is the TLS server port on the primary. + PrimaryPort int + // PrimaryPGData is the PostgreSQL data directory on the primary. In CNPG + // this is identical to the local instance's data directory. + PrimaryPGData string + // CertFile, KeyFile and CAFile are the TLS client material this instance + // presents to the primary's pgBackRest server. + CertFile string + KeyFile string + CAFile string +} + +// ErrNoCurrentPrimary is returned when backup-from-standby is enabled but the +// cluster reports no current primary, which is not a healthy cluster state. +var ErrNoCurrentPrimary = errors.New("cluster has no current primary") + +// ShouldConfigurePrimaryPeer reports whether a backup running on instanceName +// should coordinate with the primary. Whether a backup lands on the primary or a +// standby is decided by CloudNativePG via the backup target, not here. +func ShouldConfigurePrimaryPeer(enabled bool, currentPrimary, instanceName string) (bool, error) { + if !enabled { + return false, nil + } + if currentPrimary == "" { + return false, ErrNoCurrentPrimary + } + return currentPrimary != instanceName, nil +} + +// AppendStandbyPrimaryHostOptions appends the pgBackRest options that describe +// the remote primary (pg2-*) so that a backup running on a standby can +// coordinate pg_backup_start/stop on the primary over a TLS connection. The +// local standby remains pg1 (added by AppendStanzaOptionsFromConfiguration). +func AppendStandbyPrimaryHostOptions(options []string, topology StandbyBackupTopology) []string { + return append( + options, + // how to reach the remote pgBackRest TLS server running on the primary + utils.FormatDbFlag(primaryHostIndex, "host"), topology.PrimaryHost, + utils.FormatDbFlag(primaryHostIndex, "host-type"), "tls", + utils.FormatDbFlag(primaryHostIndex, "host-port"), strconv.Itoa(topology.PrimaryPort), + utils.FormatDbFlag(primaryHostIndex, "host-cert-file"), topology.CertFile, + utils.FormatDbFlag(primaryHostIndex, "host-key-file"), topology.KeyFile, + utils.FormatDbFlag(primaryHostIndex, "host-ca-file"), topology.CAFile, + // how the primary's server reaches its local PostgreSQL + utils.FormatDbFlag(primaryHostIndex, "path"), topology.PrimaryPGData, + utils.FormatDbFlag(primaryHostIndex, "user"), "postgres", + utils.FormatDbFlag(primaryHostIndex, "socket-path"), localSocketPath, + ) +} + +// BackupStandbyOption returns the --backup-standby flag. A backup coordinated with +// the primary always runs on a standby, so the value is always "y". +func BackupStandbyOption() string { + return "--backup-standby=y" +} + +// ServerConfig configures the pgBackRest TLS server that must run in each +// instance sidecar for backup-from-standby to work. +type ServerConfig struct { + // Address is the interface the server binds to (pgBackRest tls-server-address). + Address string + // Port is the TLS server port (pgBackRest tls-server-port). + Port int + // PGData is the local PostgreSQL data directory. + PGData string + // CertFile, KeyFile and CAFile are the server's TLS material. + CertFile string + KeyFile string + CAFile string + // AuthClientCN is the client-certificate common name authorized to drive + // this server, mapped to all stanzas via tls-server-auth==*. + AuthClientCN string +} + +// PgbackrestServerOptions builds the argument list for `pgbackrest server` +// (TLS server mode). This long-running process lets a peer instance's +// pgBackRest client coordinate control commands (pg_backup_start/stop) against +// this instance's local PostgreSQL. +// +// Experimental: wiring this process into the sidecar lifecycle and providing +// its certificates is deferred pending maintainer direction (see issue #103). +func PgbackrestServerOptions(cfg ServerConfig) []string { + options := []string{ + "server", + "--tls-server-address", cfg.Address, + "--tls-server-port", strconv.Itoa(cfg.Port), + "--tls-server-cert-file", cfg.CertFile, + "--tls-server-key-file", cfg.KeyFile, + "--tls-server-ca-file", cfg.CAFile, + "--tls-server-auth", fmt.Sprintf("%s=*", cfg.AuthClientCN), + utils.FormatDbFlag(0, "path"), cfg.PGData, + utils.FormatDbFlag(0, "user"), "postgres", + utils.FormatDbFlag(0, "socket-path"), localSocketPath, + "--log-level-stderr", "warn", + "--log-level-console", "off", + } + return options +} diff --git a/internal/pgbackrest/command/standby_test.go b/internal/pgbackrest/command/standby_test.go new file mode 100644 index 0000000..9c74f41 --- /dev/null +++ b/internal/pgbackrest/command/standby_test.go @@ -0,0 +1,93 @@ +/* +Copyright The CloudNativePG Contributors +Copyright 2025, Opera Norway AS + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package command + +import ( + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Backup from standby options", func() { + topology := StandbyBackupTopology{ + PrimaryHost: "10.0.0.5", + PrimaryPort: 8432, + PrimaryPGData: "/pg/data", + CertFile: "/certs/tls.crt", + KeyFile: "/certs/tls.key", + CAFile: "/certs/ca.crt", + } + + Describe("AppendStandbyPrimaryHostOptions", func() { + It("adds the primary as a second pgBackRest host over TLS", func() { + options := AppendStandbyPrimaryHostOptions(nil, topology) + Expect(strings.Join(options, " ")).To(Equal( + "--pg2-host 10.0.0.5 --pg2-host-type tls --pg2-host-port 8432 " + + "--pg2-host-cert-file /certs/tls.crt --pg2-host-key-file /certs/tls.key " + + "--pg2-host-ca-file /certs/ca.crt " + + "--pg2-path /pg/data --pg2-user postgres --pg2-socket-path /controller/run/")) + }) + + It("preserves options already present", func() { + options := AppendStandbyPrimaryHostOptions([]string{"backup"}, topology) + Expect(options[0]).To(Equal("backup")) + Expect(strings.Join(options, " ")).To(ContainSubstring("--pg2-host 10.0.0.5")) + }) + }) + + Describe("BackupStandbyOption", func() { + It("formats the --backup-standby flag", func() { + Expect(BackupStandbyOption()).To(Equal("--backup-standby=y")) + }) + }) + + Describe("PgbackrestServerOptions", func() { + It("builds the pgbackrest TLS server invocation", func() { + options := PgbackrestServerOptions(ServerConfig{ + Address: "*", + Port: 8432, + PGData: "/pg/data", + CertFile: "/srv/tls.crt", + KeyFile: "/srv/tls.key", + CAFile: "/srv/ca.crt", + AuthClientCN: "pgbackrest-client", + }) + Expect(strings.Join(options, " ")).To(Equal( + "server --tls-server-address * --tls-server-port 8432 " + + "--tls-server-cert-file /srv/tls.crt --tls-server-key-file /srv/tls.key " + + "--tls-server-ca-file /srv/ca.crt --tls-server-auth pgbackrest-client=* " + + "--pg1-path /pg/data --pg1-user postgres --pg1-socket-path /controller/run/ " + + "--log-level-stderr warn --log-level-console off")) + }) + }) + + DescribeTable("ShouldConfigurePrimaryPeer", + func(enabled bool, currentPrimary, instanceName string, expected bool) { + Expect(ShouldConfigurePrimaryPeer(enabled, currentPrimary, instanceName)).To(Equal(expected)) + }, + Entry("disabled", false, "cluster-1", "cluster-2", false), + Entry("enabled but this instance is the primary", true, "cluster-1", "cluster-1", false), + Entry("enabled and this instance is a standby", true, "cluster-1", "cluster-2", true), + ) + + It("errors when enabled without a current primary", func() { + _, err := ShouldConfigurePrimaryPeer(true, "", "cluster-2") + Expect(err).To(MatchError(ErrNoCurrentPrimary)) + }) +}) diff --git a/manifest.yaml b/manifest.yaml index 7ad05bb..d704ad8 100644 --- a/manifest.yaml +++ b/manifest.yaml @@ -42,6 +42,36 @@ spec: description: PgbackrestConfiguration is the configuration of all pgBackRest operations properties: + backupStandby: + description: |- + BackupStandby, when enabled, offloads backups to a standby instance using + pgBackRest multi-host TLS. See BackupStandbyConfiguration. + properties: + enabled: + description: Enabled turns on backup-from-standby. + type: boolean + injectSAN: + default: true + description: |- + InjectSAN controls whether the plugin adds the pgBackRest service DNS name to + the cluster server certificate (serverAltDNSNames). Defaults to true; set to + false to manage the SAN yourself. + type: boolean + injectService: + default: true + description: |- + InjectService controls whether the plugin injects the headless service that + exposes the pgBackRest TLS server port on the instances. Defaults to true; set + to false to manage that service yourself. + type: boolean + serviceName: + description: |- + ServiceName is the headless service that resolves to the primary's pgBackRest + TLS server. Defaults to "-pgbackrest", the service the plugin manages; + set it when you provide the service yourself. The port is not configurable: a + user-provided service must expose DefaultServerPort. + type: string + type: object compression: description: |- Compress a WAL file before sending it to the object store. Available