chore: review

Signed-off-by: Leonardo Cecchi <[email protected]>

Author: leonardoce

hack/build-dev-image.sh

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	barmanBackup "github.com/cloudnative-pg/barman-cloud/pkg/backup"
+	barmanCommand "github.com/cloudnative-pg/barman-cloud/pkg/command"
 	barmanCredentials "github.com/cloudnative-pg/barman-cloud/pkg/credentials"
 	"github.com/cloudnative-pg/cloudnative-pg/pkg/postgres"
 	"github.com/cloudnative-pg/cnpg-i/pkg/backup"
@@ -114,6 +115,43 @@ func (b BackupServiceImplementation) Backup(
 	}
 
 	contextLogger.Info("Backup completed", "backup", executedBackupInfo.ID)
+
+	// Refresh the recovery window
+	contextLogger.Info(
+		"Refreshing the recovery window",
+		"backupName", executedBackupInfo.BackupName,
+	)
+	backupList, err := barmanCommand.GetBackupList(
+		ctx,
+		&objectStore.Spec.Configuration,
+		configuration.ServerName,
+		env,
+	)
+	if err != nil {
+		contextLogger.Error(err, "while reading the backup list")
+		return nil, err
+	}
+
+	if err := updateRecoveryWindow(
+		ctx,
+		b.Client,
+		backupList,
+		&objectStore,
+		configuration.ServerName,
+	); err != nil {
+		contextLogger.Error(
+			err,
+			"Error while updating the recovery window in the ObjectStore status stanza. Skipping.",
+			"backupName", executedBackupInfo.BackupName,
+		)
+	} else {
+		contextLogger.Debug(
+			"backupName", executedBackupInfo.BackupName,
+			"Updated the recovery window in the ObjectStore status stanza",
+			"serverRecoveryWindow", objectStore.Status.ServerRecoveryWindow,
+		)
+	}
+
 	return &backup.BackupResult{
 		BackupId:   executedBackupInfo.ID,
 		BackupName: executedBackupInfo.BackupName,

internal/cnpgi/instance/backup.go

@@ -35,7 +35,18 @@ func Start(ctx context.Context) error {
 	controllerOptions := ctrl.Options{
 		Scheme: scheme,
 		Client: client.Options{
-			// NOTE WE WILL MODIFY THE GENERATE CLIENT WITH A CUSTOM CACHE
+			// Important: the caching options below are used by
+			// controller-runtime only.
+			// The plugin code uses an enhanced client with a
+			// custom caching strategy specifically for ObjectStores
+			// and Clusters.
+			//
+			// This custom strategy is necessary because we lack
+			// permission to list these resources at the namespace
+			// level. Additionally, controller-runtime does not
+			// support caching a closed (explicit) set of objects
+			// within a namespace - it can only cache either individual
+			// objects or all objects in a namespace.
 			Cache: &client.CacheOptions{
 				DisableFor: []client.Object{
 					&corev1.Secret{},

internal/cnpgi/instance/manager.go

@@ -3,21 +3,20 @@ package instance
 import (
 	"context"
 	"fmt"
-	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
-	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata"
-	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"strings"
 
 	"github.com/cloudnative-pg/cnpg-i/pkg/metrics"
 	"github.com/cloudnative-pg/machinery/pkg/log"
-)
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
-var (
-	// Sanitize the plugin name to be a valid Prometheus metric namespace
-	metricsDomain = strings.NewReplacer(".", "_", "-", "_").Replace(metadata.PluginName)
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/metadata"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config"
 )
 
+// Sanitize the plugin name to be a valid Prometheus metric namespace
+var metricsDomain = strings.NewReplacer(".", "_", "-", "_").Replace(metadata.PluginName)
+
 type metricsImpl struct {
 	// important the client should be one with a underlying cache
 	Client client.Client
@@ -32,7 +31,6 @@ func buildFqName(name string) string {
 var (
 	firstRecoverabilityPointMetricName     = buildFqName("first_recoverability_point")
 	lastAvailableBackupTimestampMetricName = buildFqName("last_available_backup_timestamp")
-	testMetricName                         = buildFqName("test_metric")
 )
 
 func (m metricsImpl) GetCapabilities(

internal/cnpgi/instance/metrics.go

@@ -98,7 +98,7 @@ var _ = Describe("Metrics Collect method", func() {
 		res, err := m.Collect(ctx, req)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(res).ToNot(BeNil())
-		Expect(res.Metrics).To(HaveLen(3))
+		Expect(res.Metrics).To(HaveLen(2))
 
 		// Verify the metrics
 		metricsMap := make(map[string]float64)

internal/cnpgi/instance/metrics_test.go

@@ -0,0 +1,43 @@
+package instance
+
+import (
+	"context"
+	"time"
+
+	"github.com/cloudnative-pg/barman-cloud/pkg/catalog"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+)
+
+// updateRecoveryWindow updates the recovery window inside the object
+// store status subresource
+func updateRecoveryWindow(
+	ctx context.Context,
+	c client.Client,
+	backupList *catalog.Catalog,
+	objectStore *barmancloudv1.ObjectStore,
+	serverName string,
+) error {
+	// Set the recovery window inside the barman object store object
+	convertTime := func(t *time.Time) *metav1.Time {
+		if t == nil {
+			return nil
+		}
+		return ptr.To(metav1.NewTime(*t))
+	}
+
+	recoveryWindow := barmancloudv1.RecoveryWindow{
+		FirstRecoverabilityPoint: convertTime(backupList.GetFirstRecoverabilityPoint()),
+		LastSuccessfulBackupTime: convertTime(backupList.GetLastSuccessfulBackupTime()),
+	}
+
+	if objectStore.Status.ServerRecoveryWindow == nil {
+		objectStore.Status.ServerRecoveryWindow = make(map[string]barmancloudv1.RecoveryWindow)
+	}
+	objectStore.Status.ServerRecoveryWindow[serverName] = recoveryWindow
+
+	return c.Status().Update(ctx, objectStore)
+}

internal/cnpgi/instance/recovery_window.go

@@ -7,15 +7,12 @@ import (
 	"slices"
 	"time"
 
-	"github.com/cloudnative-pg/barman-cloud/pkg/catalog"
 	barmanCommand "github.com/cloudnative-pg/barman-cloud/pkg/command"
 	barmanCredentials "github.com/cloudnative-pg/barman-cloud/pkg/credentials"
 	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
 	"github.com/cloudnative-pg/machinery/pkg/log"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
@@ -157,36 +154,7 @@ func (c *CatalogMaintenanceRunnable) maintenance(
 		return err
 	}
 
-	return c.updateRecoveryWindow(ctx, backupList, objectStore, configuration.ServerName)
-}
-
-// updateRecoveryWindow updates the recovery window inside the object
-// store status subresource
-func (c *CatalogMaintenanceRunnable) updateRecoveryWindow(
-	ctx context.Context,
-	backupList *catalog.Catalog,
-	objectStore *barmancloudv1.ObjectStore,
-	serverName string,
-) error {
-	// Set the recovery window inside the barman object store object
-	convertTime := func(t *time.Time) *metav1.Time {
-		if t == nil {
-			return nil
-		}
-		return ptr.To(metav1.NewTime(*t))
-	}
-
-	recoveryWindow := barmancloudv1.RecoveryWindow{
-		FirstRecoverabilityPoint: convertTime(backupList.GetFirstRecoverabilityPoint()),
-		LastSuccessfulBackupTime: convertTime(backupList.GetLastSuccessfulBackupTime()),
-	}
-
-	if objectStore.Status.ServerRecoveryWindow == nil {
-		objectStore.Status.ServerRecoveryWindow = make(map[string]barmancloudv1.RecoveryWindow)
-	}
-	objectStore.Status.ServerRecoveryWindow[serverName] = recoveryWindow
-
-	return c.Client.Status().Update(ctx, objectStore)
+	return updateRecoveryWindow(ctx, c.Client, backupList, objectStore, configuration.ServerName)
 }
 
 // deleteBackupsNotInCatalog deletes all Backup objects pointing to the given cluster that are not