Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 133 additions & 17 deletions cmd/non-admin/backup/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,26 @@ import (
"github.com/spf13/cobra"
"github.com/vmware-tanzu/velero/pkg/client"
"github.com/vmware-tanzu/velero/pkg/cmd/util/output"
"gopkg.in/yaml.v2"
corev1 "k8s.io/api/core/v1"
kbclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"
)

// BackupVolumeInfo represents the backup volume information
type BackupVolumeInfo struct {
BackupMethod string `json:"backupMethod"`
Result string `json:"result"`
PVBInfo PodVolumeInfo `json:"pvbInfo,omitempty"`
}

// PodVolumeInfo represents pod volume backup information
type PodVolumeInfo struct {
PodName string `json:"podName"`
PodNamespace string `json:"podNamespace"`
VolumeName string `json:"volumeName"`
Size int64 `json:"size,omitempty"`
}

func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
var (
requestTimeout time.Duration
Expand Down Expand Up @@ -71,12 +86,28 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
return fmt.Errorf("NonAdminBackup %q not found in namespace %q: %w", backupName, userNamespace, err)
}

var volumeInfo string
if details {
ctx, cancel := context.WithTimeout(context.Background(), effectiveTimeout)
defer cancel()

fetchedInfo, err := shared.ProcessDownloadRequest(ctx, kbClient, shared.DownloadRequestOptions{
BackupName: backupName,
DataType: "BackupVolumeInfos",
Namespace: userNamespace,
HTTPTimeout: effectiveTimeout,
})
if err == nil {
volumeInfo = fetchedInfo
}
}

// Print in Velero-style format
printNonAdminBackupDetails(cmd, &nab, kbClient, backupName, userNamespace, effectiveTimeout, details)
printNonAdminBackupDetails(cmd, &nab, kbClient, backupName, userNamespace, effectiveTimeout, details, volumeInfo)

// Add detailed output if --details flag is set
if details {
if err := printDetailedBackupInfo(cmd, kbClient, backupName, userNamespace, effectiveTimeout); err != nil {
if err := printDetailedBackupInfo(cmd, kbClient, backupName, userNamespace, effectiveTimeout, volumeInfo); err != nil {
return fmt.Errorf("failed to fetch detailed backup information: %w", err)
}
}
Expand All @@ -98,7 +129,7 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
}

// printNonAdminBackupDetails prints backup details in Velero admin describe format
func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBackup, kbClient kbclient.Client, backupName string, userNamespace string, timeout time.Duration, showDetails bool) {
func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBackup, kbClient kbclient.Client, backupName string, userNamespace string, timeout time.Duration, showDetails bool, volumeInfo string) {
out := cmd.OutOrStdout()

// Get Velero backup reference if available
Expand Down Expand Up @@ -367,7 +398,31 @@ func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBac
fmt.Fprintf(out, "\n")

// Pod Volume Backups
fmt.Fprintf(out, " Pod Volume Backups: <none included>\n")
if nab.Status.FileSystemPodVolumeBackups != nil && nab.Status.FileSystemPodVolumeBackups.Total > 0 {
pvbStatus := nab.Status.FileSystemPodVolumeBackups
uploaderInfo := ""
if pvbStatus.UploaderType != "" {
uploaderInfo = fmt.Sprintf(" - %s", pvbStatus.UploaderType)
}

if showDetails {
// When --details is used, display compact volume breakdown using pre-fetched volumeInfo
fmt.Fprintf(out, " Pod Volume Backups%s:\n", uploaderInfo)

if compactInfo := formatPodVolumeBackupsCompact(volumeInfo); compactInfo != "" {
fmt.Fprintf(out, "%s", compactInfo)
} else {
// Fallback to count if parsing fails
fmt.Fprintf(out, " Completed: %d\n", pvbStatus.Completed)
}
} else {
// When --details is not used, show the hint
fmt.Fprintf(out, " Pod Volume Backups%s (specify --details for more information):\n", uploaderInfo)
fmt.Fprintf(out, " Completed: %d\n", pvbStatus.Completed)
}
} else {
fmt.Fprintf(out, " Pod Volume Backups: <none included>\n")
}

fmt.Fprintf(out, "\n")

Expand All @@ -386,28 +441,21 @@ func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBac
}
}

// printDetailedBackupInfo fetches and displays additional backup details when --details flag is used.
// printDetailedBackupInfo displays additional backup details when --details flag is used.
// It uses NonAdminDownloadRequest to fetch:
// - BackupVolumeInfos (snapshot details)
// - BackupVolumeInfos (snapshot details) - pre-fetched and passed in
// - BackupResults (errors, warnings)
// - BackupItemOperations (plugin operations)
func printDetailedBackupInfo(cmd *cobra.Command, kbClient kbclient.Client, backupName string, userNamespace string, timeout time.Duration) error {
func printDetailedBackupInfo(cmd *cobra.Command, kbClient kbclient.Client, backupName string, userNamespace string, timeout time.Duration, volumeInfo string) error {
out := cmd.OutOrStdout()

ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

hasOutput := false

// 1. Fetch BackupVolumeInfos
volumeInfo, err := shared.ProcessDownloadRequest(ctx, kbClient, shared.DownloadRequestOptions{
BackupName: backupName,
DataType: "BackupVolumeInfos",
Namespace: userNamespace,
HTTPTimeout: timeout,
})

if err == nil && volumeInfo != "" {
// 1. Use pre-fetched BackupVolumeInfos (already fetched to avoid duplicate request)
if volumeInfo != "" {
if formattedInfo := formatVolumeInfo(volumeInfo); formattedInfo != "" {
if !hasOutput {
fmt.Fprintf(out, "\n")
Expand Down Expand Up @@ -487,6 +535,74 @@ func formatVolumeInfo(volumeInfo string) string {
return indent(string(formatted), " ")
}

// formatPodVolumeBackupsCompact formats pod volume backups in a compact format
// Output format: podNamespace/podName: volume1 (size: X), volume2 (size: Y)
func formatPodVolumeBackupsCompact(volumeInfo string) string {
Comment thread
SharoonAustin06 marked this conversation as resolved.
if strings.TrimSpace(volumeInfo) == "" {
return ""
}

// Parse as JSON array of BackupVolumeInfo
var snapshots []BackupVolumeInfo
if err := json.Unmarshal([]byte(volumeInfo), &snapshots); err != nil {
return ""
}

// Group volumes by pod
type volumeDetail struct {
volumeName string
size int64
}
podVolumes := make(map[string][]volumeDetail)

for _, snapshot := range snapshots {
// Only process PodVolumeBackup entries with successful result
if snapshot.BackupMethod != "PodVolumeBackup" || snapshot.Result != "succeeded" {
continue
}

// Extract pod info
pvbInfo := snapshot.PVBInfo
if pvbInfo.PodName == "" || pvbInfo.PodNamespace == "" || pvbInfo.VolumeName == "" {
continue
}

podKey := fmt.Sprintf("%s/%s", pvbInfo.PodNamespace, pvbInfo.PodName)
podVolumes[podKey] = append(podVolumes[podKey], volumeDetail{
volumeName: pvbInfo.VolumeName,
size: pvbInfo.Size,
})
}

if len(podVolumes) == 0 {
return ""
}

// Sort pod keys for consistent output
podKeys := make([]string, 0, len(podVolumes))
for k := range podVolumes {
podKeys = append(podKeys, k)
}
sort.Strings(podKeys)

// Build formatted output
var output strings.Builder
fmt.Fprintf(&output, " Completed:\n")
for _, podKey := range podKeys {
volumes := podVolumes[podKey]
fmt.Fprintf(&output, " %s: ", podKey)

// Format volumes
volumeStrs := make([]string, len(volumes))
for i, vol := range volumes {
volumeStrs[i] = fmt.Sprintf("%s (size: %d)", vol.volumeName, vol.size)
}
fmt.Fprintf(&output, "%s\n", strings.Join(volumeStrs, ", "))
}

return output.String()
}

// formatResourceList formats the resource list for display
func formatResourceList(resourceList string) string {
if strings.TrimSpace(resourceList) == "" {
Expand Down
161 changes: 161 additions & 0 deletions cmd/non-admin/backup/describe_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
/*
Copyright 2025 The OADP CLI 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 backup

import (
"testing"
)

// TestFormatPodVolumeBackupsCompact tests the formatPodVolumeBackupsCompact function
func TestFormatPodVolumeBackupsCompact(t *testing.T) {
tests := []struct {
name string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run go fmt ./... and commit the result. The required lint check currently fails in the formatting step because describe_test.go is not gofmt-formatted.

volumeInfo string
expectedLines []string
shouldContain []string
shouldNotContain []string
}{
{
name: "empty volume info",
volumeInfo: "",
expectedLines: []string{},
},
{
name: "whitespace only volume info",
volumeInfo: " \n\t ",
expectedLines: []string{},
},
{
name: "malformed JSON",
volumeInfo: "not json at all",
expectedLines: []string{},
},
{
name: "single pod with single volume succeeded",
volumeInfo: `[{"backupMethod":"PodVolumeBackup","result":"succeeded","pvbInfo":{"podName":"test-pod","podNamespace":"default","volumeName":"vol1","size":1024}}]`,
shouldContain: []string{
"Completed:",
"default/test-pod:",
"vol1 (size: 1024)",
},
},
{
name: "single pod with multiple volumes",
volumeInfo: `[{"backupMethod":"PodVolumeBackup","result":"succeeded","pvbInfo":{"podName":"test-pod","podNamespace":"default","volumeName":"vol1","size":1024}},{"backupMethod":"PodVolumeBackup","result":"succeeded","pvbInfo":{"podName":"test-pod","podNamespace":"default","volumeName":"vol2","size":2048}}]`,
shouldContain: []string{
"Completed:",
"default/test-pod:",
"vol1 (size: 1024)",
"vol2 (size: 2048)",
},
},
{
name: "multiple pods with volumes",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this test compare the complete expected output for the multi-pod case? The current substring checks do not verify indentation, line layout, or the sorted pod order that this formatter is meant to provide.

volumeInfo: `[{"backupMethod":"PodVolumeBackup","result":"succeeded","pvbInfo":{"podName":"pod1","podNamespace":"ns1","volumeName":"vol1","size":1024}},{"backupMethod":"PodVolumeBackup","result":"succeeded","pvbInfo":{"podName":"pod2","podNamespace":"ns2","volumeName":"vol2","size":2048}}]`,
shouldContain: []string{
"Completed:",
"ns1/pod1:",
"ns2/pod2:",
"vol1 (size: 1024)",
"vol2 (size: 2048)",
},
},
{
name: "failed PVB should be excluded",
volumeInfo: `[{"backupMethod":"PodVolumeBackup","result":"succeeded","pvbInfo":{"podName":"pod1","podNamespace":"ns1","volumeName":"vol1","size":1024}},{"backupMethod":"PodVolumeBackup","result":"Failed","pvbInfo":{"podName":"pod2","podNamespace":"ns2","volumeName":"vol2","size":2048}}]`,
shouldContain: []string{
"Completed:",
"ns1/pod1:",
"vol1 (size: 1024)",
},
shouldNotContain: []string{
"ns2/pod2",
"vol2",
},
},
{
name: "non-PodVolumeBackup entries should be ignored",
volumeInfo: `[{"backupMethod":"VeleroSnapshot","result":"succeeded","pvbInfo":{"podName":"pod1","podNamespace":"ns1","volumeName":"vol1","size":1024}},{"backupMethod":"PodVolumeBackup","result":"succeeded","pvbInfo":{"podName":"pod2","podNamespace":"ns2","volumeName":"vol2","size":2048}}]`,
shouldContain: []string{
"Completed:",
"ns2/pod2:",
"vol2 (size: 2048)",
},
shouldNotContain: []string{
"ns1/pod1",
"vol1",
},
},
{
name: "empty array",
volumeInfo: `[]`,
expectedLines: []string{},
},
{
name: "missing pvbInfo should be skipped",
volumeInfo: `[{"backupMethod":"PodVolumeBackup","result":"Completed"}]`,
expectedLines: []string{},
},
{
name: "missing size field defaults to 0",
volumeInfo: `[{"backupMethod":"PodVolumeBackup","result":"succeeded","pvbInfo":{"podName":"pod1","podNamespace":"ns1","volumeName":"vol1"}}]`,
shouldContain: []string{
"ns1/pod1:",
"vol1 (size: 0)",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := formatPodVolumeBackupsCompact(tt.volumeInfo)

// Check empty results (when no expected lines and no "should contain")
if len(tt.expectedLines) == 0 && len(tt.shouldContain) == 0 && result != "" {
t.Errorf("expected empty output, got: %q", result)
}

// Check contains
for _, expected := range tt.shouldContain {
if !contains(result, expected) {
t.Errorf("expected output to contain %q, got:\n%s", expected, result)
}
}

// Check not contains
for _, notExpected := range tt.shouldNotContain {
if contains(result, notExpected) {
t.Errorf("expected output to NOT contain %q, got:\n%s", notExpected, result)
}
}
})
}
}

// Helper function for testing
func contains(text string, substring string) bool {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please replace this helper with strings.Contains. The current implementation slices text to len(substring) before checking that text is long enough, so a short failed output can panic instead of reporting the assertion failure.

return len(text) > 0 && (text == substring || len(substring) > 0 && (text[:len(substring)] == substring || len(text) > len(substring) && text[len(text)-len(substring):] == substring || findSubstring(text, substring)))
}

func findSubstring(text string, substring string) bool {
for i := 0; i <= len(text)-len(substring); i++ {
if text[i:i+len(substring)] == substring {
return true
}
}
return false
}
Loading
Loading