diff --git a/go.mod b/go.mod index 5a95b0eb..999c45a8 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/cloudnative-pg/api v1.30.0 github.com/cloudnative-pg/barman-cloud v0.5.2-0.20260709152604-43158c204df1 github.com/cloudnative-pg/cloudnative-pg v1.30.0 - github.com/cloudnative-pg/cnpg-i v0.5.0 + github.com/cloudnative-pg/cnpg-i v0.5.1-0.20260716144117-db806aee0db1 github.com/cloudnative-pg/cnpg-i-machinery v0.4.2 github.com/cloudnative-pg/machinery v0.5.0 github.com/onsi/ginkgo/v2 v2.32.0 diff --git a/go.sum b/go.sum index ada804db..f0fc6762 100644 --- a/go.sum +++ b/go.sum @@ -26,6 +26,8 @@ github.com/cloudnative-pg/cloudnative-pg v1.30.0 h1:fnhVq44xXx97MNiuvJsPrX1vSjYb github.com/cloudnative-pg/cloudnative-pg v1.30.0/go.mod h1:QkolwBOWZ+GvAiJt6KpDSymwkpf0K19/p4Q6MQlTM8U= github.com/cloudnative-pg/cnpg-i v0.5.0 h1:/TOzpNT6cwNgrpftTtrnLKdoHgMwd+88vZgXjlVgXeE= github.com/cloudnative-pg/cnpg-i v0.5.0/go.mod h1:7Gh4+UzhBpGhr4DreB1GN9wGYfvxwXCXZUyVt3zE/3I= +github.com/cloudnative-pg/cnpg-i v0.5.1-0.20260716144117-db806aee0db1 h1:HO7S6jJ21DhSS6oQg7Ok1+uT23hF6I58im1IIRMPufk= +github.com/cloudnative-pg/cnpg-i v0.5.1-0.20260716144117-db806aee0db1/go.mod h1:4kcpLAj+feMTnh0TVadXRASdVnEhBytNSm/BvktLgmI= github.com/cloudnative-pg/cnpg-i-machinery v0.4.2 h1:0reS9MtyLYINHXQ/MfxJ9jp39hhBf8e3Qdj+T5Nsq6I= github.com/cloudnative-pg/cnpg-i-machinery v0.4.2/go.mod h1:gvrKabgxXq0zGthXGucemDdsxakLEQDMxn43M4HLW30= github.com/cloudnative-pg/machinery v0.5.0 h1:hhTnkzn+AiN3NmbjCQ6RXj5rfqV3K6arzq6kdXAzcnQ= diff --git a/internal/cnpgi/common/wal.go b/internal/cnpgi/common/wal.go index bb7d3e08..9bdc22ce 100644 --- a/internal/cnpgi/common/wal.go +++ b/internal/cnpgi/common/wal.go @@ -155,13 +155,17 @@ func (w WALServiceImplementation) Archive( return nil, err } - // Step 2: Check if the archive location is safe to perform archiving - checkFileExisting, err := fileutils.FileExists(emptyWalArchiveFile) + // Step 2: Check if the archive location is safe to perform archiving. + checkEmptyWalArchive, err := resolveArchiveEmptyWalArchiveCheck( + request.CheckEmptyWalArchive, + configuration.Cluster, + emptyWalArchiveFile, + ) if err != nil { - return nil, fmt.Errorf("while checking for empty wal archive check file %q: %w", emptyWalArchiveFile, err) + return nil, err } - if utils.IsEmptyWalArchiveCheckEnabled(&configuration.Cluster.ObjectMeta) && checkFileExisting { + if checkEmptyWalArchive { if err := CheckBackupDestination( ctx, &objectStore.Spec.Configuration, @@ -223,6 +227,31 @@ func (w WALServiceImplementation) Archive( return &wal.WALArchiveResult{}, nil } +// resolveArchiveEmptyWalArchiveCheck reports whether the WAL archive +// destination must be verified before archiving this segment. +// +// The operator owns the marker file's lifecycle, so when it sets the decision +// (non-nil) that value already accounts for the marker and is obeyed as-is. A +// nil value comes from an operator that predates this field, so we fall back to +// the previous logic: the Cluster annotation combined with the on-disk marker +// file. +func resolveArchiveEmptyWalArchiveCheck( + operatorDecision *bool, + cluster *cnpgv1.Cluster, + markerFilePath string, +) (bool, error) { + if operatorDecision != nil { + return *operatorDecision, nil + } + + markerFilePresent, err := fileutils.FileExists(markerFilePath) + if err != nil { + return false, fmt.Errorf("while checking for empty wal archive check file %q: %w", markerFilePath, err) + } + + return utils.IsEmptyWalArchiveCheckEnabled(&cluster.ObjectMeta) && markerFilePresent, nil +} + // Restore implements the WALService interface func (w WALServiceImplementation) Restore( ctx context.Context, diff --git a/internal/cnpgi/common/wal_test.go b/internal/cnpgi/common/wal_test.go index 92d836c0..4e66bb3c 100644 --- a/internal/cnpgi/common/wal_test.go +++ b/internal/cnpgi/common/wal_test.go @@ -20,12 +20,18 @@ SPDX-License-Identifier: Apache-2.0 package common import ( + "os" + "path/filepath" + 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" + "k8s.io/utils/ptr" + "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata" "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) var _ = Describe("resolveRestoreObjectStore", func() { @@ -96,3 +102,65 @@ var _ = Describe("resolveRestoreObjectStore", func() { "cluster-server", "cluster-store"), ) }) + +var _ = Describe("resolveArchiveEmptyWalArchiveCheck", func() { + // skipAnnotation mirrors the unexported constant in cloudnative-pg's + // pkg/utils; hard-coding the literal makes a divergence surface as a + // failing test rather than silently disabling the check. + const skipAnnotation = "cnpg.io/skipEmptyWalArchiveCheck" + + clusterWith := func(annotationValue *string) *cnpgv1.Cluster { + cluster := &cnpgv1.Cluster{} + if annotationValue != nil { + cluster.Annotations = map[string]string{skipAnnotation: *annotationValue} + } + return cluster + } + + // markerPath returns the marker file path inside a fresh temp dir, + // creating the file there when present is true. + markerPath := func(present bool) string { + filePath := filepath.Join(GinkgoT().TempDir(), metadata.CheckEmptyWalArchiveFile) + if present { + Expect(os.WriteFile(filePath, []byte{}, 0o600)).To(Succeed()) + } + return filePath + } + + When("the operator sets the decision", func() { + It("obeys true, ignoring the annotation and the marker file", func() { + // annotation would skip the check and the marker is absent, yet the + // operator's explicit true must still win. + got, err := resolveArchiveEmptyWalArchiveCheck( + ptr.To(true), clusterWith(ptr.To("enabled")), markerPath(false)) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(BeTrue()) + }) + + It("obeys false, ignoring the annotation and the marker file", func() { + // annotation would keep the check on and the marker is present, yet the + // operator's explicit false must still win. + got, err := resolveArchiveEmptyWalArchiveCheck( + ptr.To(false), clusterWith(nil), markerPath(true)) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(BeFalse()) + }) + }) + + When("the operator predates the field (nil decision)", func() { + DescribeTable( + "falls back to the annotation combined with the marker file", + func(annotationValue *string, markerPresent bool, expected bool) { + got, err := resolveArchiveEmptyWalArchiveCheck( + nil, clusterWith(annotationValue), markerPath(markerPresent)) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(expected)) + }, + Entry("no annotation and marker present: check runs", nil, true, true), + Entry("no annotation and marker absent: check skipped", nil, false, false), + Entry("opt-out annotation and marker present: check skipped", ptr.To("enabled"), true, false), + Entry("unrelated annotation value and marker present: check runs", ptr.To("something-else"), true, true), + Entry("empty annotation value and marker present: check runs", ptr.To(""), true, true), + ) + }) +}) diff --git a/internal/cnpgi/restore/restore.go b/internal/cnpgi/restore/restore.go index f286565e..9ee73548 100644 --- a/internal/cnpgi/restore/restore.go +++ b/internal/cnpgi/restore/restore.go @@ -113,6 +113,7 @@ func (impl JobHookImpl) Restore( configuration.Cluster, &targetObjectStore.Spec.Configuration, targetObjectStore.Name, + req.CheckEmptyWalArchive, ); err != nil { return nil, err } @@ -250,6 +251,7 @@ func (impl *JobHookImpl) checkBackupDestination( cluster *cnpgv1.Cluster, barmanConfiguration *cnpgv1.BarmanObjectStoreConfiguration, objectStoreName string, + operatorCheckEmptyWalArchive *bool, ) error { // Get environment from cache env, err := barmanCredentials.EnvSetCloudCredentialsAndCertificates(ctx, @@ -288,14 +290,26 @@ func (impl *JobHookImpl) checkBackupDestination( } } - // Check if we're ok to archive in the desired destination - if utils.IsEmptyWalArchiveCheckEnabled(&cluster.ObjectMeta) { + if resolveRestoreEmptyWalArchiveCheck(operatorCheckEmptyWalArchive, cluster) { return common.CheckBackupDestination(ctx, barmanConfiguration, walArchiver, serverName) } return nil } +// resolveRestoreEmptyWalArchiveCheck reports whether the destination must be +// verified before restoring. When the operator sets the decision (non-nil) it +// is obeyed as-is; a nil value comes from an operator that predates this field, +// so we fall back to the Cluster annotation. Unlike archiving, restore is a +// one-shot operation that has never been gated on the first-archive marker +// file, so the annotation is the only fallback needed. +func resolveRestoreEmptyWalArchiveCheck(operatorDecision *bool, cluster *cnpgv1.Cluster) bool { + if operatorDecision != nil { + return *operatorDecision + } + return utils.IsEmptyWalArchiveCheckEnabled(&cluster.ObjectMeta) +} + // restoreCustomWalDir moves the current pg_wal data to the specified custom wal dir and applies the symlink // returns indicating if any changes were made and any error encountered in the process func (impl JobHookImpl) restoreCustomWalDir(ctx context.Context) (bool, error) { diff --git a/internal/cnpgi/restore/restore_test.go b/internal/cnpgi/restore/restore_test.go new file mode 100644 index 00000000..a88a7f39 --- /dev/null +++ b/internal/cnpgi/restore/restore_test.go @@ -0,0 +1,66 @@ +/* +Copyright © contributors to CloudNativePG, established as +CloudNativePG a Series of LF Projects, LLC. + +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. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package restore + +import ( + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + "k8s.io/utils/ptr" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("resolveRestoreEmptyWalArchiveCheck", func() { + // skipAnnotation mirrors the unexported constant in cloudnative-pg's + // pkg/utils; hard-coding the literal makes a divergence surface as a + // failing test rather than silently disabling the check. + const skipAnnotation = "cnpg.io/skipEmptyWalArchiveCheck" + + clusterWith := func(annotationValue *string) *cnpgv1.Cluster { + cluster := &cnpgv1.Cluster{} + if annotationValue != nil { + cluster.Annotations = map[string]string{skipAnnotation: *annotationValue} + } + return cluster + } + + When("the operator sets the decision", func() { + It("obeys true even when the annotation would skip the check", func() { + Expect(resolveRestoreEmptyWalArchiveCheck(ptr.To(true), clusterWith(ptr.To("enabled")))).To(BeTrue()) + }) + + It("obeys false even when the annotation would keep the check on", func() { + Expect(resolveRestoreEmptyWalArchiveCheck(ptr.To(false), clusterWith(nil))).To(BeFalse()) + }) + }) + + When("the operator predates the field (nil decision)", func() { + DescribeTable( + "falls back to the Cluster annotation, never to a marker file", + func(annotationValue *string, expected bool) { + Expect(resolveRestoreEmptyWalArchiveCheck(nil, clusterWith(annotationValue))).To(Equal(expected)) + }, + Entry("no annotation: check runs", nil, true), + Entry("opt-out annotation: check skipped", ptr.To("enabled"), false), + Entry("unrelated annotation value: check runs", ptr.To("something-else"), true), + Entry("empty annotation value: check runs", ptr.To(""), true), + ) + }) +}) diff --git a/internal/cnpgi/restore/suite_test.go b/internal/cnpgi/restore/suite_test.go new file mode 100644 index 00000000..b110bef4 --- /dev/null +++ b/internal/cnpgi/restore/suite_test.go @@ -0,0 +1,32 @@ +/* +Copyright © contributors to CloudNativePG, established as +CloudNativePG a Series of LF Projects, LLC. + +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. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package restore + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestRestore(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Restore job hook test suite") +}