Compare backup completion time to time in pvc annotation.

Author: dsessler7

internal/controller/postgrescluster/snapshots.go

@@ -219,12 +219,21 @@ func (r *Reconciler) reconcileDedicatedSnapshotVolume(
 		return pvc, nil
 	}
 
-	// If pvc has been annotated with latest backup job's completion time, then restore was
-	// successful and there's nothing left to do.
-	backupJobCompletionTime := backupJob.Status.CompletionTime.String()
-	// TODO: change this to check whether backupJobCompletionTime is more recent than pvc annotation.
-	if pvc.GetAnnotations()[naming.PGBackRestBackupJobCompletion] == backupJobCompletionTime {
-		return pvc, nil
+	// If the pvc has not been annotated, it has not been restored and we want to proceed
+	// with a restore.
+	pvcAnnotationTimestampString, pvcAnnotationExists := pvc.GetAnnotations()[naming.PGBackRestBackupJobCompletion]
+	if pvcAnnotationExists {
+		// If pvc has been annotated, we want to compare its timestamp to the latest backup job's
+		// timestamp. If the pvc's timestamp is older than the latest backup job's
+		// completion time, then we want to do a restore.
+		backupJobCompletionTime := backupJob.Status.CompletionTime.Time
+		pvcAnnotationTime, err := time.Parse(time.RFC3339, pvcAnnotationTimestampString)
+		if err != nil {
+			return pvc, err
+		}
+		if backupJobCompletionTime.Compare(pvcAnnotationTime) <= 0 {
+			return pvc, nil
+		}
 	}
 
 	// If we've made it here, the pvc has not been restored with latest backup.
@@ -409,7 +418,7 @@ func (r *Reconciler) dedicatedSnapshotVolumeRestore(ctx context.Context,
 
 	addTMPEmptyDir(&restoreJob.Spec.Template)
 
-	restoreJob.Annotations[naming.PGBackRestBackupJobCompletion] = backupJob.Status.CompletionTime.String()
+	restoreJob.Annotations[naming.PGBackRestBackupJobCompletion] = backupJob.Status.CompletionTime.Format(time.RFC3339)
 	return errors.WithStack(r.apply(ctx, restoreJob))
 }
 

internal/controller/postgrescluster/snapshots_test.go

@@ -7,6 +7,7 @@ package postgrescluster
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/pkg/errors"
 	"gotest.tools/v3/assert"
@@ -561,7 +562,7 @@ func TestReconcileDedicatedSnapshotVolume(t *testing.T) {
 		// Create successful restore job
 		restoreJob := testRestoreJob(cluster)
 		restoreJob.Annotations = map[string]string{
-			naming.PGBackRestBackupJobCompletion: backupJob.Status.CompletionTime.String(),
+			naming.PGBackRestBackupJobCompletion: backupJob.Status.CompletionTime.Format(time.RFC3339),
 		}
 		err = errors.WithStack(r.setControllerReference(cluster, restoreJob))
 		assert.NilError(t, err)
@@ -603,7 +604,7 @@ func TestReconcileDedicatedSnapshotVolume(t *testing.T) {
 		assert.Equal(t, len(restoreJobs.Items), 0)
 
 		// Assert pvc was annotated
-		assert.Equal(t, pvc.GetAnnotations()[naming.PGBackRestBackupJobCompletion], backupJob.Status.CompletionTime.String())
+		assert.Equal(t, pvc.GetAnnotations()[naming.PGBackRestBackupJobCompletion], backupJob.Status.CompletionTime.Format(time.RFC3339))
 	})
 
 	t.Run("SnapshotsEnabledFailedRestoreExists", func(t *testing.T) {
@@ -646,7 +647,7 @@ func TestReconcileDedicatedSnapshotVolume(t *testing.T) {
 		// Create failed restore job
 		restoreJob := testRestoreJob(cluster)
 		restoreJob.Annotations = map[string]string{
-			naming.PGBackRestBackupJobCompletion: backupJob.Status.CompletionTime.String(),
+			naming.PGBackRestBackupJobCompletion: backupJob.Status.CompletionTime.Format(time.RFC3339),
 		}
 		err = errors.WithStack(r.setControllerReference(cluster, restoreJob))
 		assert.NilError(t, err)
@@ -758,7 +759,7 @@ func TestDedicatedSnapshotVolumeRestore(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Equal(t, len(jobs.Items), 1)
 	assert.Equal(t, jobs.Items[0].Annotations[naming.PGBackRestBackupJobCompletion],
-		backupJob.Status.CompletionTime.String())
+		backupJob.Status.CompletionTime.Format(time.RFC3339))
 }
 
 func TestGenerateSnapshotOfDedicatedSnapshotVolume(t *testing.T) {

internal/naming/annotations.go

@@ -23,9 +23,9 @@ const (
 
 	// PGBackRestBackupJobCompletion is the annotation that is added to restore jobs, pvcs, and
 	// VolumeSnapshots that are involved in the volume snapshot creation process. The annotation
-	// holds a timestamp that corresponds to the completion time of the associated backup job. A
-	// backup is always taken right before a delta restore is performed on the dedicated snapshot
-	// volume, and then a VolumeSnapshot is taken of the dedicated volume. The value is a timestamp.
+	// holds a RFC3339 formatted timestamp that corresponds to the completion time of the associated
+	// backup job. A backup is always taken right before a delta restore is performed on the
+	// dedicated snapshot volume, and then a VolumeSnapshot is taken of the dedicated volume.
 	PGBackRestBackupJobCompletion = annotationPrefix + "pgbackrest-backup-job-completion"
 
 	// PGBackRestConfigHash is an annotation used to specify the hash value associated with a