Skip to content

Commit d70ead9

Browse files
Joeavaikathclaude
Joeavaikath
and
claude
authored
General fixes (#112)
* Fix gzip decompression for backup downloads and enhance describe command This commit addresses issues with compressed backup data and adds new functionality for detailed backup information: Download improvements: - Fix gzip detection to check file magic bytes (0x1f 0x8b) in addition to Content-Encoding header. Object storage signed URLs often serve .gz files without the Content-Encoding header, causing decompressed content to be displayed as binary gibberish - Add comprehensive debug logging to download functions for easier troubleshooting - Use buffered reader with Peek() to detect gzip format without consuming the stream Describe command enhancements: - Add --details flag to fetch additional backup information including volume snapshots, resource lists, backup results, and item operations - Exclude describe command from output wrapper (similar to logs command) to prevent buffering issues with large downloads - Update tests to reflect describe command exclusion Installation improvements: - Enhance make install output formatting with clearer status messages - Show version info during installation verification Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com> * Refactor download request logic to eliminate code duplication Extract common download request creation and polling logic into a new shared function CreateAndWaitForDownloadURL. This eliminates ~53 lines of duplicated code between the logs and describe commands. Key changes: - Add CreateAndWaitForDownloadURL function to handle request creation and polling with optional progress callbacks - Add OnProgress callback to DownloadRequestOptions for progress indication - Refactor logs command to use shared function (75 lines → 22 lines) - Simplify printDetailedBackupInfo to accept backup name string instead of full NonAdminBackup object - Remove debug log statements from download.go - Clean up unused imports in logs.go This maintains the key behavioral difference between commands: - logs: streams content for low memory usage - describe --details: buffers content for formatting Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com> * Improve nonadmin backup describe output formatting Align nonadmin backup describe output with admin backup format: - Move Resource List before Backup Volumes section to match admin order - Parse and format Resource List JSON as hierarchical structure with bullets - Skip printing sections with no data (empty snapshots, no errors/warnings) - Parse and format Volume Snapshot Details, Backup Results, and Item Operations as readable JSON - Split Backup Results into separate Errors and Warnings subsections Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com> * Fix ineffectual assignment in backup describe command Remove unused assignment to hasOutput variable in printDetailedBackupInfo function. The assignment at line 447 had no effect since it was in the last section and the variable was never used afterward. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Joseph <jvaikath@redhat.com> --------- Signed-off-by: Joseph <jvaikath@redhat.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f8a17ce commit d70ead9

6 files changed

Lines changed: 404 additions & 111 deletions

File tree

‎Makefile‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,17 @@ install: build ## Build and install the kubectl plugin to ~/.local/bin (no sudo
228228
echo " ├─ Temporarily updating PATH for verification"; \
229229
if PATH="$(INSTALL_PATH):$$PATH" command -v kubectl >/dev/null 2>&1; then \
230230
if PATH="$(INSTALL_PATH):$$PATH" kubectl plugin list 2>/dev/null | grep -q "kubectl-oadp"; then \
231-
echo " ├─ ✅ Installation verified: kubectl oadp plugin is accessible"; \
232-
PATH="$(INSTALL_PATH):$$PATH" kubectl oadp version 2>/dev/null || echo " │ └─ (Note: version command requires cluster access)"; \
231+
echo " └─ ✅ Installation verified: kubectl oadp plugin is accessible"; \
232+
echo ""; \
233+
echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \
234+
echo "🎉 Installation complete!"; \
235+
echo ""; \
236+
echo " ⚠️ To use in this terminal session, run:"; \
237+
echo " export PATH=\"$(INSTALL_PATH):$$PATH\""; \
238+
echo ""; \
239+
echo " Quick start:"; \
240+
echo " • kubectl oadp --help # Show available commands"; \
241+
echo " • kubectl oadp backup get # List backups"; \
233242
else \
234243
echo " ├─ ❌ Installation verification failed: kubectl oadp plugin not found"; \
235244
echo " │ └─ Try running: export PATH=\"$(INSTALL_PATH):$$PATH\""; \
@@ -242,9 +251,22 @@ install: build ## Build and install the kubectl plugin to ~/.local/bin (no sudo
242251
if command -v kubectl >/dev/null 2>&1; then \
243252
if kubectl plugin list 2>/dev/null | grep -q "kubectl-oadp"; then \
244253
echo " ├─ ✅ Installation verified: kubectl oadp plugin is accessible"; \
245-
echo " ├─ Running version command..."; \
254+
echo " └─ Running version command..."; \
255+
echo ""; \
256+
version_output=$$(kubectl oadp version 2>&1 | grep -v "WARNING: the client version does not match"); \
257+
if [ $$? -eq 0 ] && [ -n "$$version_output" ]; then \
258+
echo "$$version_output" | sed 's/^/ /'; \
259+
else \
260+
echo " (Note: version command requires cluster access)"; \
261+
fi; \
262+
echo ""; \
263+
echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"; \
264+
echo "🎉 Installation complete!"; \
246265
echo ""; \
247-
kubectl oadp version 2>/dev/null || echo " │ └─ (Note: version command requires cluster access)"; \
266+
echo " Quick start:"; \
267+
echo " • kubectl oadp --help # Show available commands"; \
268+
echo " • kubectl oadp backup get # List backups"; \
269+
echo " • kubectl oadp version # Show version info"; \
248270
else \
249271
echo " └─ ❌ Installation verification failed: kubectl oadp plugin not found"; \
250272
fi; \

‎cmd/non-admin/backup/describe.go‎

Lines changed: 242 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package backup
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
67
"sort"
78
"strings"
@@ -18,7 +19,10 @@ import (
1819
)
1920

2021
func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
21-
var requestTimeout time.Duration
22+
var (
23+
requestTimeout time.Duration
24+
details bool
25+
)
2226

2327
c := &cobra.Command{
2428
Use: use + " NAME",
@@ -68,15 +72,24 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
6872
}
6973

7074
// Print in Velero-style format
71-
printNonAdminBackupDetails(cmd, &nab)
75+
printNonAdminBackupDetails(cmd, &nab, kbClient, backupName, userNamespace, effectiveTimeout)
76+
77+
// Add detailed output if --details flag is set
78+
if details {
79+
if err := printDetailedBackupInfo(cmd, kbClient, backupName, userNamespace, effectiveTimeout); err != nil {
80+
return fmt.Errorf("failed to fetch detailed backup information: %w", err)
81+
}
82+
}
7283

7384
return nil
7485
},
7586
Example: ` kubectl oadp nonadmin backup describe my-backup
76-
kubectl oadp nonadmin backup describe my-backup --request-timeout=30m`,
87+
kubectl oadp nonadmin backup describe my-backup --details
88+
kubectl oadp nonadmin backup describe my-backup --details --request-timeout=30m`,
7789
}
7890

7991
c.Flags().DurationVar(&requestTimeout, "request-timeout", 0, fmt.Sprintf("The length of time to wait before giving up on a single server request (e.g., 30s, 5m, 1h). Overrides %s env var. Default: %v", shared.TimeoutEnvVar, shared.DefaultHTTPTimeout))
92+
c.Flags().BoolVar(&details, "details", false, "Display additional backup details including volume snapshots, resource lists, and item operations")
8093

8194
output.BindFlags(c.Flags())
8295
output.ClearOutputFlagDefault(c)
@@ -85,7 +98,7 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
8598
}
8699

87100
// printNonAdminBackupDetails prints backup details in Velero admin describe format
88-
func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBackup) {
101+
func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBackup, kbClient kbclient.Client, backupName string, userNamespace string, timeout time.Duration) {
89102
out := cmd.OutOrStdout()
90103

91104
// Get Velero backup reference if available
@@ -309,6 +322,25 @@ func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBac
309322

310323
fmt.Fprintf(out, "\n")
311324

325+
// Fetch and display Resource List
326+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
327+
defer cancel()
328+
329+
resourceList, err := shared.ProcessDownloadRequest(ctx, kbClient, shared.DownloadRequestOptions{
330+
BackupName: backupName,
331+
DataType: "BackupResourceList",
332+
Namespace: userNamespace,
333+
HTTPTimeout: timeout,
334+
})
335+
336+
if err == nil && resourceList != "" {
337+
if formattedList := formatResourceList(resourceList); formattedList != "" {
338+
fmt.Fprintf(out, "Resource List:\n")
339+
fmt.Fprintf(out, "%s\n", formattedList)
340+
fmt.Fprintf(out, "\n")
341+
}
342+
}
343+
312344
// Backup Volumes
313345
fmt.Fprintf(out, "Backup Volumes:\n")
314346

@@ -347,6 +379,212 @@ func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBac
347379
}
348380
}
349381

382+
// printDetailedBackupInfo fetches and displays additional backup details when --details flag is used.
383+
// It uses NonAdminDownloadRequest to fetch:
384+
// - BackupVolumeInfos (snapshot details)
385+
// - BackupResults (errors, warnings)
386+
// - BackupItemOperations (plugin operations)
387+
func printDetailedBackupInfo(cmd *cobra.Command, kbClient kbclient.Client, backupName string, userNamespace string, timeout time.Duration) error {
388+
out := cmd.OutOrStdout()
389+
390+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
391+
defer cancel()
392+
393+
hasOutput := false
394+
395+
// 1. Fetch BackupVolumeInfos
396+
volumeInfo, err := shared.ProcessDownloadRequest(ctx, kbClient, shared.DownloadRequestOptions{
397+
BackupName: backupName,
398+
DataType: "BackupVolumeInfos",
399+
Namespace: userNamespace,
400+
HTTPTimeout: timeout,
401+
})
402+
403+
if err == nil && volumeInfo != "" {
404+
if formattedInfo := formatVolumeInfo(volumeInfo); formattedInfo != "" {
405+
if !hasOutput {
406+
fmt.Fprintf(out, "\n")
407+
hasOutput = true
408+
}
409+
fmt.Fprintf(out, "Volume Snapshot Details:\n")
410+
fmt.Fprintf(out, "%s\n", formattedInfo)
411+
fmt.Fprintf(out, "\n")
412+
}
413+
}
414+
415+
// 2. Fetch BackupResults
416+
results, err := shared.ProcessDownloadRequest(ctx, kbClient, shared.DownloadRequestOptions{
417+
BackupName: backupName,
418+
DataType: "BackupResults",
419+
Namespace: userNamespace,
420+
HTTPTimeout: timeout,
421+
})
422+
423+
if err == nil && results != "" {
424+
if formattedResults := formatBackupResults(results); formattedResults != "" {
425+
if !hasOutput {
426+
fmt.Fprintf(out, "\n")
427+
hasOutput = true
428+
}
429+
fmt.Fprintf(out, "Backup Results:\n")
430+
fmt.Fprintf(out, "%s\n", formattedResults)
431+
fmt.Fprintf(out, "\n")
432+
}
433+
}
434+
435+
// 3. Fetch BackupItemOperations
436+
itemOps, err := shared.ProcessDownloadRequest(ctx, kbClient, shared.DownloadRequestOptions{
437+
BackupName: backupName,
438+
DataType: "BackupItemOperations",
439+
Namespace: userNamespace,
440+
HTTPTimeout: timeout,
441+
})
442+
443+
if err == nil && itemOps != "" {
444+
if formattedOps := formatItemOperations(itemOps); formattedOps != "" {
445+
if !hasOutput {
446+
fmt.Fprintf(out, "\n")
447+
}
448+
fmt.Fprintf(out, "Backup Item Operations:\n")
449+
fmt.Fprintf(out, "%s\n", formattedOps)
450+
fmt.Fprintf(out, "\n")
451+
}
452+
}
453+
454+
return nil
455+
}
456+
457+
// formatVolumeInfo formats volume snapshot information for display
458+
func formatVolumeInfo(volumeInfo string) string {
459+
if strings.TrimSpace(volumeInfo) == "" {
460+
return ""
461+
}
462+
463+
// Try to parse as JSON array
464+
var snapshots []interface{}
465+
if err := json.Unmarshal([]byte(volumeInfo), &snapshots); err != nil {
466+
// If parsing fails, fall back to indented output
467+
return indent(volumeInfo, " ")
468+
}
469+
470+
// If empty array, return empty string (will show "<none>")
471+
if len(snapshots) == 0 {
472+
return ""
473+
}
474+
475+
// Format as indented JSON for readability
476+
formatted, err := json.MarshalIndent(snapshots, " ", " ")
477+
if err != nil {
478+
return indent(volumeInfo, " ")
479+
}
480+
return indent(string(formatted), " ")
481+
}
482+
483+
// formatResourceList formats the resource list for display
484+
func formatResourceList(resourceList string) string {
485+
if strings.TrimSpace(resourceList) == "" {
486+
return ""
487+
}
488+
489+
// Try to parse as JSON map
490+
var resources map[string][]string
491+
if err := json.Unmarshal([]byte(resourceList), &resources); err != nil {
492+
// If parsing fails, fall back to indented output
493+
return indent(resourceList, " ")
494+
}
495+
496+
// Sort the keys (GroupVersionKind)
497+
keys := make([]string, 0, len(resources))
498+
for k := range resources {
499+
keys = append(keys, k)
500+
}
501+
sort.Strings(keys)
502+
503+
// Build formatted output
504+
var output strings.Builder
505+
for _, gvk := range keys {
506+
items := resources[gvk]
507+
output.WriteString(fmt.Sprintf(" %s:\n", gvk))
508+
for _, item := range items {
509+
output.WriteString(fmt.Sprintf(" - %s\n", item))
510+
}
511+
}
512+
513+
return strings.TrimSuffix(output.String(), "\n")
514+
}
515+
516+
// formatBackupResults formats backup results (errors/warnings) for display
517+
func formatBackupResults(results string) string {
518+
if strings.TrimSpace(results) == "" {
519+
return ""
520+
}
521+
522+
// Try to parse as JSON object with errors and warnings
523+
var resultsObj struct {
524+
Errors map[string]interface{} `json:"errors"`
525+
Warnings map[string]interface{} `json:"warnings"`
526+
}
527+
if err := json.Unmarshal([]byte(results), &resultsObj); err != nil {
528+
// If parsing fails, fall back to indented output
529+
return indent(results, " ")
530+
}
531+
532+
// If both are empty, return empty string so section won't be printed
533+
if len(resultsObj.Errors) == 0 && len(resultsObj.Warnings) == 0 {
534+
return ""
535+
}
536+
537+
// Format nicely
538+
var output strings.Builder
539+
540+
// Show errors
541+
output.WriteString(" Errors:\n")
542+
if len(resultsObj.Errors) > 0 {
543+
formatted, _ := json.MarshalIndent(resultsObj.Errors, " ", " ")
544+
output.WriteString(indent(string(formatted), " "))
545+
} else {
546+
output.WriteString(" <none>")
547+
}
548+
output.WriteString("\n\n")
549+
550+
// Show warnings
551+
output.WriteString(" Warnings:\n")
552+
if len(resultsObj.Warnings) > 0 {
553+
formatted, _ := json.MarshalIndent(resultsObj.Warnings, " ", " ")
554+
output.WriteString(indent(string(formatted), " "))
555+
} else {
556+
output.WriteString(" <none>")
557+
}
558+
559+
return strings.TrimSuffix(output.String(), "\n")
560+
}
561+
562+
// formatItemOperations formats backup item operations for display
563+
func formatItemOperations(itemOps string) string {
564+
if strings.TrimSpace(itemOps) == "" {
565+
return ""
566+
}
567+
568+
// Try to parse as JSON array
569+
var operations []interface{}
570+
if err := json.Unmarshal([]byte(itemOps), &operations); err != nil {
571+
// If parsing fails, fall back to indented output
572+
return indent(itemOps, " ")
573+
}
574+
575+
// If empty array, return empty string (will show "<none>")
576+
if len(operations) == 0 {
577+
return ""
578+
}
579+
580+
// Format as indented JSON for readability
581+
formatted, err := json.MarshalIndent(operations, " ", " ")
582+
if err != nil {
583+
return indent(itemOps, " ")
584+
}
585+
return indent(string(formatted), " ")
586+
}
587+
350588
// colorizePhase returns the phase string with ANSI color codes
351589
func colorizePhase(phase string) string {
352590
const (

0 commit comments

Comments
 (0)