use pointer types for metav1.Time fields (#1951)

* use pointer types for metav1.Time fields

Signed-off-by: Adnan Abdulhussein <aadnan@vmware.com>

* simpler metav1.Time ptrs

Signed-off-by: Adnan Abdulhussein <aadnan@vmware.com>

* remove test debug println

Signed-off-by: Adnan Abdulhussein <aadnan@vmware.com>

pkg/controller/backup_controller.go

@@ -198,7 +198,7 @@ func (c *backupController) processBackup(key string) error {
 		request.Status.Phase = velerov1api.BackupPhaseFailedValidation
 	} else {
 		request.Status.Phase = velerov1api.BackupPhaseInProgress
-		request.Status.StartTimestamp.Time = c.clock.Now()
+		request.Status.StartTimestamp = &metav1.Time{Time: c.clock.Now()}
 	}
 
 	// update status
@@ -289,7 +289,7 @@ func (c *backupController) prepareBackupRequest(backup *velerov1api.Backup) *pkg
 	}
 
 	// calculate expiration
-	request.Status.Expiration = metav1.NewTime(c.clock.Now().Add(request.Spec.TTL.Duration))
+	request.Status.Expiration = &metav1.Time{Time: c.clock.Now().Add(request.Spec.TTL.Duration)}
 
 	// default storage location if not specified
 	if request.Spec.StorageLocation == "" {
@@ -485,7 +485,7 @@ func (c *backupController) runBackup(backup *pkgbackup.Request) error {
 	exists, err := backupStore.BackupExists(backup.StorageLocation.Spec.StorageType.ObjectStorage.Bucket, backup.Name)
 	if exists || err != nil {
 		backup.Status.Phase = velerov1api.BackupPhaseFailed
-		backup.Status.CompletionTimestamp.Time = c.clock.Now()
+		backup.Status.CompletionTimestamp = &metav1.Time{Time: c.clock.Now()}
 		if err != nil {
 			return errors.Wrapf(err, "error checking if backup already exists in object storage")
 		}
@@ -499,7 +499,7 @@ func (c *backupController) runBackup(backup *pkgbackup.Request) error {
 
 	// Mark completion timestamp before serializing and uploading.
 	// Otherwise, the JSON file in object storage has a CompletionTimestamp of 'null'.
-	backup.Status.CompletionTimestamp.Time = c.clock.Now()
+	backup.Status.CompletionTimestamp = &metav1.Time{Time: c.clock.Now()}
 
 	backup.Status.VolumeSnapshotsAttempted = len(backup.VolumeSnapshots)
 	for _, snap := range backup.VolumeSnapshots {

pkg/controller/backup_controller_test.go

@@ -305,7 +305,7 @@ func TestDefaultBackupTTL(t *testing.T) {
 			res := c.prepareBackupRequest(test.backup)
 			assert.NotNil(t, res)
 			assert.Equal(t, test.expectedTTL, res.Spec.TTL)
-			assert.Equal(t, test.expectedExpiration, res.Status.Expiration)
+			assert.Equal(t, test.expectedExpiration, *res.Status.Expiration)
 		})
 	}
 }
@@ -316,6 +316,7 @@ func TestProcessBackupCompletions(t *testing.T) {
 	now, err := time.Parse(time.RFC1123Z, time.RFC1123Z)
 	require.NoError(t, err)
 	now = now.Local()
+	timestamp := metav1.NewTime(now)
 
 	tests := []struct {
 		name                string
@@ -348,9 +349,9 @@ func TestProcessBackupCompletions(t *testing.T) {
 				Status: velerov1api.BackupStatus{
 					Phase:               velerov1api.BackupPhaseCompleted,
 					Version:             1,
-					StartTimestamp:      metav1.NewTime(now),
-					CompletionTimestamp: metav1.NewTime(now),
-					Expiration:          metav1.NewTime(now),
+					StartTimestamp:      &timestamp,
+					CompletionTimestamp: &timestamp,
+					Expiration:          &timestamp,
 				},
 			},
 		},
@@ -376,9 +377,9 @@ func TestProcessBackupCompletions(t *testing.T) {
 				Status: velerov1api.BackupStatus{
 					Phase:               velerov1api.BackupPhaseCompleted,
 					Version:             1,
-					StartTimestamp:      metav1.NewTime(now),
-					CompletionTimestamp: metav1.NewTime(now),
-					Expiration:          metav1.NewTime(now),
+					StartTimestamp:      &timestamp,
+					CompletionTimestamp: &timestamp,
+					Expiration:          &timestamp,
 				},
 			},
 		},
@@ -407,9 +408,9 @@ func TestProcessBackupCompletions(t *testing.T) {
 				Status: velerov1api.BackupStatus{
 					Phase:               velerov1api.BackupPhaseCompleted,
 					Version:             1,
-					StartTimestamp:      metav1.NewTime(now),
-					CompletionTimestamp: metav1.NewTime(now),
-					Expiration:          metav1.NewTime(now),
+					StartTimestamp:      &timestamp,
+					CompletionTimestamp: &timestamp,
+					Expiration:          &timestamp,
 				},
 			},
 		},
@@ -436,9 +437,9 @@ func TestProcessBackupCompletions(t *testing.T) {
 				Status: velerov1api.BackupStatus{
 					Phase:               velerov1api.BackupPhaseCompleted,
 					Version:             1,
-					Expiration:          metav1.NewTime(now.Add(10 * time.Minute)),
-					StartTimestamp:      metav1.NewTime(now),
-					CompletionTimestamp: metav1.NewTime(now),
+					Expiration:          &metav1.Time{now.Add(10 * time.Minute)},
+					StartTimestamp:      &timestamp,
+					CompletionTimestamp: &timestamp,
 				},
 			},
 		},
@@ -465,9 +466,9 @@ func TestProcessBackupCompletions(t *testing.T) {
 				Status: velerov1api.BackupStatus{
 					Phase:               velerov1api.BackupPhaseCompleted,
 					Version:             1,
-					StartTimestamp:      metav1.NewTime(now),
-					CompletionTimestamp: metav1.NewTime(now),
-					Expiration:          metav1.NewTime(now),
+					StartTimestamp:      &timestamp,
+					CompletionTimestamp: &timestamp,
+					Expiration:          &timestamp,
 				},
 			},
 		},
@@ -496,9 +497,9 @@ func TestProcessBackupCompletions(t *testing.T) {
 				Status: velerov1api.BackupStatus{
 					Phase:               velerov1api.BackupPhaseFailed,
 					Version:             1,
-					StartTimestamp:      metav1.NewTime(now),
-					CompletionTimestamp: metav1.NewTime(now),
-					Expiration:          metav1.NewTime(now),
+					StartTimestamp:      &timestamp,
+					CompletionTimestamp: &timestamp,
+					Expiration:          &timestamp,
 				},
 			},
 		},
@@ -525,9 +526,9 @@ func TestProcessBackupCompletions(t *testing.T) {
 				Status: velerov1api.BackupStatus{
 					Phase:               velerov1api.BackupPhaseFailed,
 					Version:             1,
-					StartTimestamp:      metav1.NewTime(now),
-					CompletionTimestamp: metav1.NewTime(now),
-					Expiration:          metav1.NewTime(now),
+					StartTimestamp:      &timestamp,
+					CompletionTimestamp: &timestamp,
+					Expiration:          &timestamp,
 				},
 			},
 		},

pkg/controller/download_request_controller.go

@@ -184,7 +184,7 @@ func (c *downloadRequestController) generatePreSignedURL(downloadRequest *v1.Dow
 	}
 
 	update.Status.Phase = v1.DownloadRequestPhaseProcessed
-	update.Status.Expiration = metav1.NewTime(c.clock.Now().Add(persistence.DownloadURLTTL))
+	update.Status.Expiration = &metav1.Time{Time: c.clock.Now().Add(persistence.DownloadURLTTL)}
 
 	_, err = patchDownloadRequest(downloadRequest, update, c.downloadRequestClient)
 	return errors.WithStack(err)

pkg/controller/download_request_controller_test.go

@@ -252,9 +252,9 @@ func TestProcessDownloadRequest(t *testing.T) {
 			// clock time, it's easier to do this here than as part of the test case definitions.
 			if tc.downloadRequest != nil && tc.downloadRequest.Status.Phase == v1.DownloadRequestPhaseProcessed {
 				if tc.expired {
-					tc.downloadRequest.Status.Expiration.Time = harness.controller.clock.Now().Add(-1 * time.Minute)
+					tc.downloadRequest.Status.Expiration = &metav1.Time{Time: harness.controller.clock.Now().Add(-1 * time.Minute)}
 				} else {
-					tc.downloadRequest.Status.Expiration.Time = harness.controller.clock.Now().Add(time.Minute)
+					tc.downloadRequest.Status.Expiration = &metav1.Time{Time: harness.controller.clock.Now().Add(time.Minute)}
 				}
 			}
 

pkg/controller/gc_controller.go

@@ -123,14 +123,13 @@ func (c *gcController) processQueueItem(key string) error {
 	log = c.logger.WithFields(
 		logrus.Fields{
 			"backup":     key,
-			"expiration": backup.Status.Expiration.Time,
+			"expiration": backup.Status.Expiration,
 		},
 	)
 
 	now := c.clock.Now()
 
-	expiration := backup.Status.Expiration.Time
-	if expiration.IsZero() || expiration.After(now) {
+	if backup.Status.Expiration == nil || backup.Status.Expiration.After(now) {
 		log.Debug("Backup has not expired yet, skipping")
 		return nil
 	}

pkg/controller/pod_volume_backup_controller.go

@@ -27,6 +27,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/clock"
@@ -181,7 +182,7 @@ func (c *podVolumeBackupController) processBackup(req *velerov1api.PodVolumeBack
 	// update status to InProgress
 	req, err = c.patchPodVolumeBackup(req, func(r *velerov1api.PodVolumeBackup) {
 		r.Status.Phase = velerov1api.PodVolumeBackupPhaseInProgress
-		r.Status.StartTimestamp.Time = c.clock.Now()
+		r.Status.StartTimestamp = &metav1.Time{Time: c.clock.Now()}
 	})
 	if err != nil {
 		log.WithError(err).Error("Error setting PodVolumeBackup StartTimestamp and phase to InProgress")
@@ -277,7 +278,7 @@ func (c *podVolumeBackupController) processBackup(req *velerov1api.PodVolumeBack
 		r.Status.Path = path
 		r.Status.Phase = velerov1api.PodVolumeBackupPhaseCompleted
 		r.Status.SnapshotID = snapshotID
-		r.Status.CompletionTimestamp.Time = c.clock.Now()
+		r.Status.CompletionTimestamp = &metav1.Time{Time: c.clock.Now()}
 		if emptySnapshot {
 			r.Status.Message = "volume was empty so no snapshot was taken"
 		}
@@ -376,7 +377,7 @@ func (c *podVolumeBackupController) fail(req *velerov1api.PodVolumeBackup, msg s
 	if _, err := c.patchPodVolumeBackup(req, func(r *velerov1api.PodVolumeBackup) {
 		r.Status.Phase = velerov1api.PodVolumeBackupPhaseFailed
 		r.Status.Message = msg
-		r.Status.CompletionTimestamp.Time = c.clock.Now()
+		r.Status.CompletionTimestamp = &metav1.Time{Time: c.clock.Now()}
 	}); err != nil {
 		log.WithError(err).Error("Error setting PodVolumeBackup phase to Failed")
 		return err

pkg/controller/pod_volume_restore_controller.go

@@ -29,6 +29,7 @@ import (
 	"github.com/sirupsen/logrus"
 	corev1api "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/clock"
@@ -265,7 +266,7 @@ func (c *podVolumeRestoreController) processRestore(req *velerov1api.PodVolumeRe
 	// update status to InProgress
 	req, err = c.patchPodVolumeRestore(req, func(r *velerov1api.PodVolumeRestore) {
 		r.Status.Phase = velerov1api.PodVolumeRestorePhaseInProgress
-		r.Status.StartTimestamp.Time = c.clock.Now()
+		r.Status.StartTimestamp = &metav1.Time{Time: c.clock.Now()}
 	})
 	if err != nil {
 		log.WithError(err).Error("Error setting PodVolumeRestore startTimestamp and phase to InProgress")
@@ -301,7 +302,7 @@ func (c *podVolumeRestoreController) processRestore(req *velerov1api.PodVolumeRe
 	// update status to Completed
 	if _, err = c.patchPodVolumeRestore(req, func(r *velerov1api.PodVolumeRestore) {
 		r.Status.Phase = velerov1api.PodVolumeRestorePhaseCompleted
-		r.Status.CompletionTimestamp.Time = c.clock.Now()
+		r.Status.CompletionTimestamp = &metav1.Time{Time: c.clock.Now()}
 	}); err != nil {
 		log.WithError(err).Error("Error setting PodVolumeRestore completionTimestamp and phase to Completed")
 		return err
@@ -408,7 +409,7 @@ func (c *podVolumeRestoreController) failRestore(req *velerov1api.PodVolumeResto
 	if _, err := c.patchPodVolumeRestore(req, func(pvr *velerov1api.PodVolumeRestore) {
 		pvr.Status.Phase = velerov1api.PodVolumeRestorePhaseFailed
 		pvr.Status.Message = msg
-		pvr.Status.CompletionTimestamp.Time = c.clock.Now()
+		pvr.Status.CompletionTimestamp = &metav1.Time{Time: c.clock.Now()}
 	}); err != nil {
 		log.WithError(err).Error("Error setting PodVolumeRestore phase to Failed")
 		return err

pkg/controller/restic_repository_controller.go

@@ -190,7 +190,7 @@ func (c *resticRepositoryController) initializeRepo(req *v1.ResticRepository, lo
 
 	return c.patchResticRepository(req, func(req *v1.ResticRepository) {
 		req.Status.Phase = v1.ResticRepositoryPhaseReady
-		req.Status.LastMaintenanceTime = metav1.Time{Time: time.Now()}
+		req.Status.LastMaintenanceTime = &metav1.Time{Time: time.Now()}
 	})
 }
 
@@ -238,7 +238,7 @@ func (c *resticRepositoryController) runMaintenanceIfDue(req *v1.ResticRepositor
 	}
 
 	return c.patchResticRepository(req, func(req *v1.ResticRepository) {
-		req.Status.LastMaintenanceTime = metav1.Time{Time: now}
+		req.Status.LastMaintenanceTime = &metav1.Time{Time: now}
 	})
 }
 

pkg/controller/restore_controller.go

@@ -373,7 +373,15 @@ func backupXorScheduleProvided(restore *api.Restore) bool {
 func mostRecentCompletedBackup(backups []*api.Backup) *api.Backup {
 	sort.Slice(backups, func(i, j int) bool {
 		// Use .After() because we want descending sort.
-		return backups[i].Status.StartTimestamp.After(backups[j].Status.StartTimestamp.Time)
+
+		var iStartTime, jStartTime time.Time
+		if backups[i].Status.StartTimestamp != nil {
+			iStartTime = backups[i].Status.StartTimestamp.Time
+		}
+		if backups[j].Status.StartTimestamp != nil {
+			jStartTime = backups[j].Status.StartTimestamp.Time
+		}
+		return iStartTime.After(jStartTime)
 	})
 
 	for _, backup := range backups {

pkg/controller/restore_controller_test.go

@@ -781,7 +781,7 @@ func TestMostRecentCompletedBackup(t *testing.T) {
 		},
 		Status: api.BackupStatus{
 			Phase:          api.BackupPhaseCompleted,
-			StartTimestamp: metav1.Time{Time: now},
+			StartTimestamp: &metav1.Time{Time: now},
 		},
 	})
 
@@ -791,7 +791,7 @@ func TestMostRecentCompletedBackup(t *testing.T) {
 		},
 		Status: api.BackupStatus{
 			Phase:          api.BackupPhaseCompleted,
-			StartTimestamp: metav1.Time{Time: now.Add(time.Second)},
+			StartTimestamp: &metav1.Time{Time: now.Add(time.Second)},
 		},
 	}
 	backups = append(backups, expected)

pkg/controller/schedule_controller.go

@@ -266,7 +266,7 @@ func (c *scheduleController) submitBackupIfDue(item *api.Schedule, cronSchedule
 	original := item
 	schedule := item.DeepCopy()
 
-	schedule.Status.LastBackup = metav1.NewTime(now)
+	schedule.Status.LastBackup = &metav1.Time{Time: now}
 
 	if _, err := patchSchedule(original, schedule, c.schedulesClient); err != nil {
 		return errors.Wrapf(err, "error updating Schedule's LastBackup time to %v", schedule.Status.LastBackup)
@@ -278,7 +278,10 @@ func (c *scheduleController) submitBackupIfDue(item *api.Schedule, cronSchedule
 func getNextRunTime(schedule *api.Schedule, cronSchedule cron.Schedule, asOf time.Time) (bool, time.Time) {
 	// get the latest run time (if the schedule hasn't run yet, this will be the zero value which will trigger
 	// an immediate backup)
-	lastBackupTime := schedule.Status.LastBackup.Time
+	var lastBackupTime time.Time
+	if schedule.Status.LastBackup != nil {
+		lastBackupTime = schedule.Status.LastBackup.Time
+	}
 
 	nextRunTime := cronSchedule.Next(lastBackupTime)
 

pkg/controller/schedule_controller_test.go

@@ -174,7 +174,7 @@ func TestProcessSchedule(t *testing.T) {
 							t.Logf("error parsing status.lastBackup: %s\n", err)
 							return false, nil, err
 						}
-						res.Status.LastBackup = metav1.Time{Time: parsed}
+						res.Status.LastBackup = &metav1.Time{Time: parsed}
 					}
 
 					return true, res, nil
@@ -318,14 +318,21 @@ func TestGetNextRunTime(t *testing.T) {
 				offsetDuration, err := time.ParseDuration(test.lastRanOffset)
 				require.NoError(t, err, "unable to parse test.lastRanOffset: %v", err)
 
-				test.schedule.Status.LastBackup = metav1.Time{Time: testClock.Now().Add(-offsetDuration)}
+				test.schedule.Status.LastBackup = &metav1.Time{Time: testClock.Now().Add(-offsetDuration)}
 			}
 
 			nextRunTimeOffset, err := time.ParseDuration(test.expectedNextRunTimeOffset)
 			if err != nil {
 				panic(err)
 			}
-			expectedNextRunTime := test.schedule.Status.LastBackup.Add(nextRunTimeOffset)
+
+			// calculate expected next run time (if the schedule hasn't run yet, this
+			// will be the zero value which will trigger an immediate backup)
+			var baseTime time.Time
+			if test.lastRanOffset != "" {
+				baseTime = test.schedule.Status.LastBackup.Time
+			}
+			expectedNextRunTime := baseTime.Add(nextRunTimeOffset)
 
 			due, nextRunTime := getNextRunTime(test.schedule, cronSchedule, testClock.Now())
 
@@ -371,7 +378,7 @@ func TestParseCronSchedule(t *testing.T) {
 	assert.Equal(t, time.Date(2017, 8, 11, 9, 0, 0, 0, time.UTC), next)
 
 	// record backup time
-	s.Status.LastBackup = metav1.NewTime(now)
+	s.Status.LastBackup = &metav1.Time{Time: now}
 
 	// advance clock 1 minute, make sure we're not due and next backup is tomorrow at 9am
 	now = time.Date(2017, 8, 11, 9, 2, 0, 0, time.UTC)