From 8d275e69cc21875e8cc4585458dcf54cd6fbe111 Mon Sep 17 00:00:00 2001 From: Ralthos <161431341+Ralthos@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:12:53 +0530 Subject: [PATCH] Print n/a for unset timestamps in backup and restore get (#10206) velero backup get prints in CREATED for a backup that never started, and velero restore get prints it in both STARTED and COMPLETED. The timestamps are *metav1.Time and are appended to the row unformatted, so a nil pointer reaches the user as Go's nil literal. This is reachable in ordinary use. A backup that fails validation never starts, so StartTimestamp is never set, and a restore that fails validation gets neither timestamp. formatTimestamp returns n/a for an unset value, matching humanReadableTimeFromNow, which already handles a zero expiration in the same row. A set timestamp is unchanged. Adds tests for both printers, which had no row-level coverage. Signed-off-by: saral --- changelogs/unreleased/10201-Ralthos | 1 + pkg/cmd/util/output/backup_printer.go | 2 +- pkg/cmd/util/output/output.go | 14 ++ pkg/cmd/util/output/printer_timestamp_test.go | 130 ++++++++++++++++++ pkg/cmd/util/output/restore_printer.go | 4 +- 5 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/10201-Ralthos create mode 100644 pkg/cmd/util/output/printer_timestamp_test.go diff --git a/changelogs/unreleased/10201-Ralthos b/changelogs/unreleased/10201-Ralthos new file mode 100644 index 000000000..f41b37a2d --- /dev/null +++ b/changelogs/unreleased/10201-Ralthos @@ -0,0 +1 @@ +Show n/a instead of for unset timestamps in velero backup get and velero restore get diff --git a/pkg/cmd/util/output/backup_printer.go b/pkg/cmd/util/output/backup_printer.go index 873bc9fc3..53a950828 100644 --- a/pkg/cmd/util/output/backup_printer.go +++ b/pkg/cmd/util/output/backup_printer.go @@ -107,7 +107,7 @@ func printBackup(backup *velerov1api.Backup) []metav1.TableRow { status, backup.Status.Errors, backup.Status.Warnings, - backup.Status.StartTimestamp, + formatTimestamp(backup.Status.StartTimestamp), humanReadableTimeFromNow(expiration), backup.Spec.StorageLocation, queuePosition(backup.Status.QueuePosition), diff --git a/pkg/cmd/util/output/output.go b/pkg/cmd/util/output/output.go index 9dfca040b..9c46030f2 100644 --- a/pkg/cmd/util/output/output.go +++ b/pkg/cmd/util/output/output.go @@ -248,3 +248,17 @@ func NewPrinter(cmd *cobra.Command) (printers.ResourcePrinter, error) { return printer, nil } + +// formatTimestamp renders an optional timestamp for a table cell. +// +// Appending a nil *metav1.Time to a row prints "", which reaches the user +// for any object that has not reached the phase that sets the field: a backup +// that failed validation never gets a start time, and a restore that failed +// validation gets neither a start nor a completion time. An unset timestamp +// shows as "n/a" instead, matching humanReadableTimeFromNow in the same row. +func formatTimestamp(t *metav1.Time) string { + if t == nil || t.IsZero() { + return "n/a" + } + return t.String() +} diff --git a/pkg/cmd/util/output/printer_timestamp_test.go b/pkg/cmd/util/output/printer_timestamp_test.go new file mode 100644 index 000000000..f967b3b3f --- /dev/null +++ b/pkg/cmd/util/output/printer_timestamp_test.go @@ -0,0 +1,130 @@ +/* +Copyright the Velero contributors. + +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. +*/ + +package output + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" +) + +func TestFormatTimestamp(t *testing.T) { + set := metav1.NewTime(time.Date(2026, 8, 8, 21, 6, 28, 0, time.UTC)) + + tests := []struct { + name string + input *metav1.Time + want string + }{ + { + name: "nil renders as n/a", + input: nil, + want: "n/a", + }, + { + name: "zero value renders as n/a", + input: &metav1.Time{}, + want: "n/a", + }, + { + name: "a set timestamp is unchanged", + input: &set, + want: set.String(), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, formatTimestamp(tc.input)) + }) + } +} + +// A backup that fails validation never starts, so StartTimestamp stays nil. +func TestPrintBackupWithoutStartTimestamp(t *testing.T) { + backup := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{Name: "failed-validation"}, + Status: velerov1api.BackupStatus{ + Phase: velerov1api.BackupPhaseFailedValidation, + }, + } + + rows := printBackup(backup) + require.Len(t, rows, 1) + + // Name, Status, Errors, Warnings, Created, ... + assert.Equal(t, "n/a", rows[0].Cells[4], "unset start time should not print as ") + assert.Equal(t, string(velerov1api.BackupPhaseFailedValidation), rows[0].Cells[1]) +} + +func TestPrintBackupWithStartTimestamp(t *testing.T) { + started := metav1.NewTime(time.Date(2026, 8, 8, 21, 6, 28, 0, time.UTC)) + backup := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{Name: "completed"}, + Status: velerov1api.BackupStatus{ + Phase: velerov1api.BackupPhaseCompleted, + StartTimestamp: &started, + }, + } + + rows := printBackup(backup) + require.Len(t, rows, 1) + assert.Equal(t, started.String(), rows[0].Cells[4]) +} + +// A restore that fails validation gets neither timestamp. +func TestPrintRestoreWithoutTimestamps(t *testing.T) { + restore := &velerov1api.Restore{ + ObjectMeta: metav1.ObjectMeta{Name: "failed-validation"}, + Spec: velerov1api.RestoreSpec{BackupName: "does-not-exist"}, + Status: velerov1api.RestoreStatus{ + Phase: velerov1api.RestorePhaseFailedValidation, + }, + } + + rows := printRestore(restore) + require.Len(t, rows, 1) + + // Name, Backup, Status, Started, Completed, ... + assert.Equal(t, "n/a", rows[0].Cells[3], "unset start time should not print as ") + assert.Equal(t, "n/a", rows[0].Cells[4], "unset completion time should not print as ") +} + +func TestPrintRestoreWithTimestamps(t *testing.T) { + started := metav1.NewTime(time.Date(2026, 8, 8, 21, 9, 40, 0, time.UTC)) + completed := metav1.NewTime(time.Date(2026, 8, 8, 21, 9, 41, 0, time.UTC)) + + restore := &velerov1api.Restore{ + ObjectMeta: metav1.ObjectMeta{Name: "completed"}, + Spec: velerov1api.RestoreSpec{BackupName: "nightly-1"}, + Status: velerov1api.RestoreStatus{ + Phase: velerov1api.RestorePhaseCompleted, + StartTimestamp: &started, + CompletionTimestamp: &completed, + }, + } + + rows := printRestore(restore) + require.Len(t, rows, 1) + assert.Equal(t, started.String(), rows[0].Cells[3]) + assert.Equal(t, completed.String(), rows[0].Cells[4]) +} diff --git a/pkg/cmd/util/output/restore_printer.go b/pkg/cmd/util/output/restore_printer.go index 782eb3485..d9b35a3cb 100644 --- a/pkg/cmd/util/output/restore_printer.go +++ b/pkg/cmd/util/output/restore_printer.go @@ -62,8 +62,8 @@ func printRestore(restore *v1.Restore) []metav1.TableRow { restore.Name, restore.Spec.BackupName, status, - restore.Status.StartTimestamp, - restore.Status.CompletionTimestamp, + formatTimestamp(restore.Status.StartTimestamp), + formatTimestamp(restore.Status.CompletionTimestamp), restore.Status.Errors, restore.Status.Warnings, restore.CreationTimestamp.Time,