Add configurable timeout to backup logs command

cmd/non-admin/backup/backup_test.go

@@ -60,6 +60,8 @@ func TestNonAdminBackupCommands(t *testing.T) {
 			args: []string{"nonadmin", "backup", "describe", "--help"},
 			expectContains: []string{
 				"Describe a non-admin backup",
+				"--timeout",
+				"Maximum time to wait for backup details to be available",
 			},
 		},
 		{
@@ -82,6 +84,8 @@ func TestNonAdminBackupCommands(t *testing.T) {
 			args: []string{"nonadmin", "backup", "logs", "--help"},
 			expectContains: []string{
 				"Show logs for a non-admin backup",
+				"--timeout",
+				"Maximum time to wait for logs to be available",
 			},
 		},
 		{
@@ -128,6 +132,7 @@ func TestNonAdminBackupCommands(t *testing.T) {
 			expectContains: []string{
 				"Describe non-admin resources",
 				"backup",
+				"--timeout",
 			},
 		},
 		{
@@ -136,6 +141,7 @@ func TestNonAdminBackupCommands(t *testing.T) {
 			expectContains: []string{
 				"Get logs for non-admin resources",
 				"backup",
+				"--timeout",
 			},
 		},
 		// Shorthand verb-noun order tests

cmd/non-admin/backup/describe.go

@@ -17,12 +17,22 @@ import (
 	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const (
+	// defaultDescribeTimeout is the default timeout for fetching backup details
+	defaultDescribeTimeout = 5 * time.Minute
+)
+
 func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
 	c := &cobra.Command{
 		Use:   use + " NAME",
 		Short: "Describe a non-admin backup",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			timeout, err := cmd.Flags().GetDuration("timeout")
+			if err != nil {
+				return fmt.Errorf("failed to get timeout flag: %w", err)
+			}
+
 			backupName := args[0]
 
 			// Get the current namespace from kubectl context
@@ -51,21 +61,23 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
 			}
 
 			// Print in Velero-style format
-			printNonAdminBackupDetails(cmd, &nab)
+			printNonAdminBackupDetails(cmd, &nab, kbClient, userNamespace, timeout)
 
 			return nil
 		},
 		Example: `  kubectl oadp nonadmin backup describe my-backup`,
 	}
 
+	c.Flags().Duration("timeout", defaultDescribeTimeout, "Maximum time to wait for backup details to be available")
+
 	output.BindFlags(c.Flags())
 	output.ClearOutputFlagDefault(c)
 
 	return c
 }
 
 // printNonAdminBackupDetails prints backup details in Velero admin describe format
-func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBackup) {
+func printNonAdminBackupDetails(cmd *cobra.Command, nab *nacv1alpha1.NonAdminBackup, kbClient kbclient.Client, userNamespace string, timeout time.Duration) {
 	out := cmd.OutOrStdout()
 
 	// Get Velero backup reference if available
@@ -350,8 +362,8 @@ func colorizePhase(phase string) string {
 
 // NonAdminDescribeBackup mirrors Velero's output.DescribeBackup functionality
 // but works within non-admin RBAC boundaries using NonAdminDownloadRequest
-func NonAdminDescribeBackup(cmd *cobra.Command, kbClient kbclient.Client, nab *nacv1alpha1.NonAdminBackup, userNamespace string) error {
-	ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
+func NonAdminDescribeBackup(cmd *cobra.Command, kbClient kbclient.Client, nab *nacv1alpha1.NonAdminBackup, userNamespace string, timeout time.Duration) error {
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
 	defer cancel()
 
 	// Print basic backup information

cmd/non-admin/backup/logs.go

@@ -30,13 +30,23 @@ import (
 	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const (
+	// defaultLogsTimeout is the default timeout for waiting for backup logs to be available
+	defaultLogsTimeout = 5 * time.Minute
+)
+
 func NewLogsCommand(f client.Factory, use string) *cobra.Command {
-	return &cobra.Command{
+	c := &cobra.Command{
 		Use:   use + " NAME",
 		Short: "Show logs for a non-admin backup",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
+			timeout, err := cmd.Flags().GetDuration("timeout")
+			if err != nil {
+				return fmt.Errorf("failed to get timeout flag: %w", err)
+			}
+
+			ctx, cancel := context.WithTimeout(context.Background(), timeout)
 			defer cancel()
 
 			// Get the current namespace from kubectl context
@@ -97,20 +107,26 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command {
 				_ = kbClient.Delete(deleteCtx, req)
 			}()
 
-			fmt.Fprintf(cmd.OutOrStdout(), "Waiting for backup logs to be processed...\n")
+			fmt.Fprintf(cmd.OutOrStdout(), "Waiting for backup logs to be processed (timeout: %v)...\n", timeout)
 
-			// Wait for the download request to be processed using shared utility
-			// Note: We create a custom waiting implementation here to provide user feedback
-			timeout := time.After(120 * time.Second)
-			tick := time.Tick(2 * time.Second)
+			// Wait for the download request to be processed
+			// Note: We implement custom polling here instead of using shared.ProcessDownloadRequest
+			// because we want to show progress feedback (dots) to the user
+			ticker := time.NewTicker(2 * time.Second)
+			defer ticker.Stop()
 
 			var signedURL string
 		Loop:
 			for {
 				select {
-				case <-timeout:
-					return fmt.Errorf("timed out waiting for NonAdminDownloadRequest to be processed")
-				case <-tick:
+				case <-ctx.Done():
+					// Check if context was cancelled due to timeout or other reason
+					if ctx.Err() == context.DeadlineExceeded {
+						return shared.FormatDownloadRequestTimeoutError(kbClient, req, timeout)
+					}
+					// Context cancelled for other reason (e.g., user interruption)
+					return fmt.Errorf("operation cancelled: %w", ctx.Err())
+				case <-ticker.C:
 					fmt.Fprintf(cmd.OutOrStdout(), ".")
 					var updated nacv1alpha1.NonAdminDownloadRequest
 					if err := kbClient.Get(ctx, kbclient.ObjectKey{
@@ -149,4 +165,8 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command {
 		},
 		Example: `  kubectl oadp nonadmin backup logs my-backup`,
 	}
+
+	c.Flags().Duration("timeout", defaultLogsTimeout, "Maximum time to wait for logs to be available")
+
+	return c
 }

cmd/shared/download.go

@@ -31,6 +31,12 @@ import (
 	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const (
+	// defaultStatusCheckTimeout is the timeout for retrieving status information
+	// when formatting timeout error messages
+	defaultStatusCheckTimeout = 5 * time.Second
+)
+
 // DownloadRequestOptions holds configuration for creating and processing NonAdminDownloadRequests
 type DownloadRequestOptions struct {
 	// BackupName is the name of the backup to download data for
@@ -103,10 +109,15 @@ func waitForDownloadURL(ctx context.Context, kbClient kbclient.Client, req *nacv
 	for {
 		select {
 		case <-timeoutCtx.Done():
-			return "", fmt.Errorf("timed out waiting for NonAdminDownloadRequest to be processed")
+			// Check if context was cancelled due to timeout or other reason
+			if timeoutCtx.Err() == context.DeadlineExceeded {
+				return "", FormatDownloadRequestTimeoutError(kbClient, req, timeout)
+			}
+			// Context cancelled for other reason (e.g., user interruption)
+			return "", fmt.Errorf("operation cancelled: %w", timeoutCtx.Err())
 		case <-ticker.C:
 			var updated nacv1alpha1.NonAdminDownloadRequest
-			if err := kbClient.Get(ctx, kbclient.ObjectKey{
+			if err := kbClient.Get(timeoutCtx, kbclient.ObjectKey{
 				Namespace: req.Namespace,
 				Name:      req.Name,
 			}, &updated); err != nil {
@@ -198,3 +209,35 @@ func StreamDownloadContent(url string, writer io.Writer) error {
 
 	return nil
 }
+
+// FormatDownloadRequestTimeoutError creates an informative timeout error message
+// by retrieving the current status of the NonAdminDownloadRequest.
+// Note: This function uses context.Background() for the status check because it's typically
+// called after the main context has been cancelled (timeout), and we still want to retrieve
+// the final status for error reporting.
+func FormatDownloadRequestTimeoutError(kbClient kbclient.Client, req *nacv1alpha1.NonAdminDownloadRequest, timeout time.Duration) error {
+	// Get the latest status to provide helpful error message
+	// Use a fresh context since the caller's context may already be cancelled
+	statusCtx, cancel := context.WithTimeout(context.Background(), defaultStatusCheckTimeout)
+	defer cancel()
+
+	var updated nacv1alpha1.NonAdminDownloadRequest
+	if err := kbClient.Get(statusCtx, kbclient.ObjectKey{
+		Namespace: req.Namespace,
+		Name:      req.Name,
+	}, &updated); err == nil {
+		// Show what state the request is in
+		var statusInfo string
+		if len(updated.Status.Conditions) > 0 {
+			var conditions []string
+			for _, cond := range updated.Status.Conditions {
+				conditions = append(conditions, fmt.Sprintf("%s=%s", cond.Type, cond.Status))
+			}
+			statusInfo = fmt.Sprintf("NonAdminDownloadRequest conditions: %s", strings.Join(conditions, ", "))
+		} else {
+			statusInfo = "NonAdminDownloadRequest has no status conditions yet"
+		}
+		return fmt.Errorf("timed out after %v waiting for NonAdminDownloadRequest %q to be processed. %s", timeout, req.Name, statusInfo)
+	}
+	return fmt.Errorf("timed out after %v waiting for NonAdminDownloadRequest %q to be processed", timeout, req.Name)
+}