From 7efb2cdf5667dd18fa12a189517f68d57f9fd8e7 Mon Sep 17 00:00:00 2001 From: Marco Nenciarini Date: Thu, 16 Jul 2026 18:05:28 +0200 Subject: [PATCH] feat: honor the operator's check_empty_wal_archive decision Archive() and the restore job hook each re-derived, on their own, whether to verify the WAL archive destination is empty, by reading a Cluster annotation and, for Archive, an on-disk marker file. That decision belongs to the operator, which already tracks both the annotation and the marker file's lifecycle. Honor cnpg-i's new WALArchiveRequest/RestoreRequest field CheckEmptyWalArchive when the operator sets it: obey it directly, without re-inspecting the marker file. Only fall back to the previous annotation-and-marker-file logic when talking to an operator that predates this field. Signed-off-by: Marco Nenciarini --- internal/cnpgi/common/wal.go | 20 +++++++++++++++----- internal/cnpgi/restore/restore.go | 16 ++++++++++++++-- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/internal/cnpgi/common/wal.go b/internal/cnpgi/common/wal.go index 1d4fd30..222a17b 100644 --- a/internal/cnpgi/common/wal.go +++ b/internal/cnpgi/common/wal.go @@ -156,13 +156,23 @@ func (w WALServiceImplementation) Archive( return nil, err } - // Step 2: Check if the archive location is safe to perform archiving - checkFileExisting, err := fileutils.FileExists(emptyWalArchiveFile) - if err != nil { - return nil, fmt.Errorf("while checking for empty wal archive check file %q: %w", emptyWalArchiveFile, err) + // Step 2: Check if the archive location is safe to perform archiving. + // The operator owns the marker file's lifecycle, so a non-nil decision + // already accounts for it and is obeyed as-is. A nil value means an + // operator that predates this field; fall back to checking the marker + // file and Cluster annotation directly. + var checkEmptyWalArchive bool + if request.CheckEmptyWalArchive != nil { + checkEmptyWalArchive = *request.CheckEmptyWalArchive + } else { + checkFileExisting, err := fileutils.FileExists(emptyWalArchiveFile) + if err != nil { + return nil, fmt.Errorf("while checking for empty wal archive check file %q: %w", emptyWalArchiveFile, err) + } + checkEmptyWalArchive = utils.IsEmptyWalArchiveCheckEnabled(&configuration.Cluster.ObjectMeta) && checkFileExisting } - if utils.IsEmptyWalArchiveCheckEnabled(&configuration.Cluster.ObjectMeta) && checkFileExisting { + if checkEmptyWalArchive { if err := CheckBackupDestination( ctx, &objectStore.Spec.Configuration, diff --git a/internal/cnpgi/restore/restore.go b/internal/cnpgi/restore/restore.go index f286565..0b46f44 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,8 +290,18 @@ func (impl *JobHookImpl) checkBackupDestination( } } - // Check if we're ok to archive in the desired destination - if utils.IsEmptyWalArchiveCheckEnabled(&cluster.ObjectMeta) { + // Check if we're ok to restore from the desired destination. Unlike + // archiving, restore is a one-shot operation that has never been gated + // on the first-archive marker file, so the only fallback needed here + // (for an operator that predates this field) is the Cluster annotation. + var checkEmptyWalArchive bool + if operatorCheckEmptyWalArchive != nil { + checkEmptyWalArchive = *operatorCheckEmptyWalArchive + } else { + checkEmptyWalArchive = utils.IsEmptyWalArchiveCheckEnabled(&cluster.ObjectMeta) + } + + if checkEmptyWalArchive { return common.CheckBackupDestination(ctx, barmanConfiguration, walArchiver, serverName) }