From ffbafcb670930308def4cba2342fefbde57266e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Dec 2025 16:50:06 +0000 Subject: [PATCH 1/9] Initial plan From dc5c4bbf7df404799eb76924e8ada28f5a9f8e07 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Dec 2025 16:58:53 +0000 Subject: [PATCH 2/9] Add configurable timeout to backup logs command with better error messages Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com> --- cmd/non-admin/backup/logs.go | 41 ++++++++++++++++++++++++++++++------ cmd/shared/download.go | 21 +++++++++++++++++- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/cmd/non-admin/backup/logs.go b/cmd/non-admin/backup/logs.go index ee8f555..622ef61 100644 --- a/cmd/non-admin/backup/logs.go +++ b/cmd/non-admin/backup/logs.go @@ -19,6 +19,7 @@ limitations under the License. import ( "context" "fmt" + "strings" "time" "github.com/migtools/oadp-cli/cmd/shared" @@ -31,12 +32,17 @@ import ( ) 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,19 +103,38 @@ 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) + timeoutChan := time.After(timeout) tick := time.Tick(2 * time.Second) var signedURL string Loop: for { select { - case <-timeout: - return fmt.Errorf("timed out waiting for NonAdminDownloadRequest to be processed") + case <-timeoutChan: + // Get the latest status to provide helpful error message + var updated nacv1alpha1.NonAdminDownloadRequest + if err := kbClient.Get(context.Background(), 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) case <-tick: fmt.Fprintf(cmd.OutOrStdout(), ".") var updated nacv1alpha1.NonAdminDownloadRequest @@ -149,4 +174,8 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command { }, Example: ` kubectl oadp nonadmin backup logs my-backup`, } + + c.Flags().Duration("timeout", 5*time.Minute, "Maximum time to wait for logs to be available") + + return c } diff --git a/cmd/shared/download.go b/cmd/shared/download.go index 22a1c8c..4a0fd95 100644 --- a/cmd/shared/download.go +++ b/cmd/shared/download.go @@ -103,7 +103,26 @@ 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") + // Get the latest status to provide helpful error message + var updated nacv1alpha1.NonAdminDownloadRequest + if err := kbClient.Get(context.Background(), 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) case <-ticker.C: var updated nacv1alpha1.NonAdminDownloadRequest if err := kbClient.Get(ctx, kbclient.ObjectKey{ From dfb77856e8a85a379ee11b5440bfbb94fa1d2f58 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Dec 2025 17:01:12 +0000 Subject: [PATCH 3/9] Update tests to verify timeout flag in logs command Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com> --- cmd/non-admin/backup/backup_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/non-admin/backup/backup_test.go b/cmd/non-admin/backup/backup_test.go index fe1da13..c783c76 100644 --- a/cmd/non-admin/backup/backup_test.go +++ b/cmd/non-admin/backup/backup_test.go @@ -82,6 +82,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", }, }, { @@ -136,6 +138,7 @@ func TestNonAdminBackupCommands(t *testing.T) { expectContains: []string{ "Get logs for non-admin resources", "backup", + "--timeout", }, }, // Shorthand verb-noun order tests From 21c258aa219084fb5b79a0d60ffc93099b0d8fb5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Dec 2025 17:07:24 +0000 Subject: [PATCH 4/9] Refactor timeout error handling into shared utility function Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com> --- cmd/non-admin/backup/logs.go | 22 +---------------- cmd/shared/download.go | 46 ++++++++++++++++++++---------------- 2 files changed, 27 insertions(+), 41 deletions(-) diff --git a/cmd/non-admin/backup/logs.go b/cmd/non-admin/backup/logs.go index 622ef61..e04efcb 100644 --- a/cmd/non-admin/backup/logs.go +++ b/cmd/non-admin/backup/logs.go @@ -19,7 +19,6 @@ limitations under the License. import ( "context" "fmt" - "strings" "time" "github.com/migtools/oadp-cli/cmd/shared" @@ -115,26 +114,7 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command { for { select { case <-timeoutChan: - // Get the latest status to provide helpful error message - var updated nacv1alpha1.NonAdminDownloadRequest - if err := kbClient.Get(context.Background(), 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) + return shared.FormatDownloadRequestTimeoutError(kbClient, req, timeout) case <-tick: fmt.Fprintf(cmd.OutOrStdout(), ".") var updated nacv1alpha1.NonAdminDownloadRequest diff --git a/cmd/shared/download.go b/cmd/shared/download.go index 4a0fd95..4312d61 100644 --- a/cmd/shared/download.go +++ b/cmd/shared/download.go @@ -103,26 +103,7 @@ func waitForDownloadURL(ctx context.Context, kbClient kbclient.Client, req *nacv for { select { case <-timeoutCtx.Done(): - // Get the latest status to provide helpful error message - var updated nacv1alpha1.NonAdminDownloadRequest - if err := kbClient.Get(context.Background(), 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) + return "", FormatDownloadRequestTimeoutError(kbClient, req, timeout) case <-ticker.C: var updated nacv1alpha1.NonAdminDownloadRequest if err := kbClient.Get(ctx, kbclient.ObjectKey{ @@ -217,3 +198,28 @@ 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. +func FormatDownloadRequestTimeoutError(kbClient kbclient.Client, req *nacv1alpha1.NonAdminDownloadRequest, timeout time.Duration) error { + // Get the latest status to provide helpful error message + var updated nacv1alpha1.NonAdminDownloadRequest + if err := kbClient.Get(context.Background(), 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) +} From 1cfcf03d685b7d27ec0ea912cab963de13a2cba1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Dec 2025 17:09:58 +0000 Subject: [PATCH 5/9] Fix context handling and eliminate duplicate timeout logic Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com> --- cmd/non-admin/backup/logs.go | 13 ++++++------- cmd/shared/download.go | 10 +++++++--- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/cmd/non-admin/backup/logs.go b/cmd/non-admin/backup/logs.go index e04efcb..6d9d890 100644 --- a/cmd/non-admin/backup/logs.go +++ b/cmd/non-admin/backup/logs.go @@ -104,18 +104,17 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command { 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 - timeoutChan := time.After(timeout) - tick := time.Tick(2 * time.Second) + // Wait for the download request to be processed + ticker := time.NewTicker(2 * time.Second) + defer ticker.Stop() var signedURL string Loop: for { select { - case <-timeoutChan: - return shared.FormatDownloadRequestTimeoutError(kbClient, req, timeout) - case <-tick: + case <-ctx.Done(): + return shared.FormatDownloadRequestTimeoutError(ctx, kbClient, req, timeout) + case <-ticker.C: fmt.Fprintf(cmd.OutOrStdout(), ".") var updated nacv1alpha1.NonAdminDownloadRequest if err := kbClient.Get(ctx, kbclient.ObjectKey{ diff --git a/cmd/shared/download.go b/cmd/shared/download.go index 4312d61..8da1c6e 100644 --- a/cmd/shared/download.go +++ b/cmd/shared/download.go @@ -103,7 +103,7 @@ func waitForDownloadURL(ctx context.Context, kbClient kbclient.Client, req *nacv for { select { case <-timeoutCtx.Done(): - return "", FormatDownloadRequestTimeoutError(kbClient, req, timeout) + return "", FormatDownloadRequestTimeoutError(ctx, kbClient, req, timeout) case <-ticker.C: var updated nacv1alpha1.NonAdminDownloadRequest if err := kbClient.Get(ctx, kbclient.ObjectKey{ @@ -201,10 +201,14 @@ func StreamDownloadContent(url string, writer io.Writer) error { // FormatDownloadRequestTimeoutError creates an informative timeout error message // by retrieving the current status of the NonAdminDownloadRequest. -func FormatDownloadRequestTimeoutError(kbClient kbclient.Client, req *nacv1alpha1.NonAdminDownloadRequest, timeout time.Duration) error { +func FormatDownloadRequestTimeoutError(ctx context.Context, kbClient kbclient.Client, req *nacv1alpha1.NonAdminDownloadRequest, timeout time.Duration) error { // Get the latest status to provide helpful error message + // Use a short-lived context to avoid blocking on this final status check + statusCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + var updated nacv1alpha1.NonAdminDownloadRequest - if err := kbClient.Get(context.Background(), kbclient.ObjectKey{ + if err := kbClient.Get(statusCtx, kbclient.ObjectKey{ Namespace: req.Namespace, Name: req.Name, }, &updated); err == nil { From b377a07110ad46146aaf15f0266bad9d23b8d4fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Dec 2025 17:12:34 +0000 Subject: [PATCH 6/9] Use context.Background() for timeout error status check Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com> --- cmd/non-admin/backup/logs.go | 2 +- cmd/shared/download.go | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cmd/non-admin/backup/logs.go b/cmd/non-admin/backup/logs.go index 6d9d890..91a5d1a 100644 --- a/cmd/non-admin/backup/logs.go +++ b/cmd/non-admin/backup/logs.go @@ -113,7 +113,7 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command { for { select { case <-ctx.Done(): - return shared.FormatDownloadRequestTimeoutError(ctx, kbClient, req, timeout) + return shared.FormatDownloadRequestTimeoutError(kbClient, req, timeout) case <-ticker.C: fmt.Fprintf(cmd.OutOrStdout(), ".") var updated nacv1alpha1.NonAdminDownloadRequest diff --git a/cmd/shared/download.go b/cmd/shared/download.go index 8da1c6e..cb3a110 100644 --- a/cmd/shared/download.go +++ b/cmd/shared/download.go @@ -103,7 +103,7 @@ func waitForDownloadURL(ctx context.Context, kbClient kbclient.Client, req *nacv for { select { case <-timeoutCtx.Done(): - return "", FormatDownloadRequestTimeoutError(ctx, kbClient, req, timeout) + return "", FormatDownloadRequestTimeoutError(kbClient, req, timeout) case <-ticker.C: var updated nacv1alpha1.NonAdminDownloadRequest if err := kbClient.Get(ctx, kbclient.ObjectKey{ @@ -201,10 +201,13 @@ func StreamDownloadContent(url string, writer io.Writer) error { // FormatDownloadRequestTimeoutError creates an informative timeout error message // by retrieving the current status of the NonAdminDownloadRequest. -func FormatDownloadRequestTimeoutError(ctx context.Context, kbClient kbclient.Client, req *nacv1alpha1.NonAdminDownloadRequest, timeout time.Duration) error { +// 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 short-lived context to avoid blocking on this final status check - statusCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + // Use a fresh context since the caller's context may already be cancelled + statusCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() var updated nacv1alpha1.NonAdminDownloadRequest From f08f3a1a58bb48e8f2ca5e1ceef7416910cdcb0f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Dec 2025 17:15:00 +0000 Subject: [PATCH 7/9] Distinguish timeout from user cancellation and use named constant Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com> --- cmd/non-admin/backup/logs.go | 7 ++++++- cmd/shared/download.go | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/cmd/non-admin/backup/logs.go b/cmd/non-admin/backup/logs.go index 91a5d1a..8b70be5 100644 --- a/cmd/non-admin/backup/logs.go +++ b/cmd/non-admin/backup/logs.go @@ -113,7 +113,12 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command { for { select { case <-ctx.Done(): - return shared.FormatDownloadRequestTimeoutError(kbClient, req, timeout) + // 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 diff --git a/cmd/shared/download.go b/cmd/shared/download.go index cb3a110..97acfd8 100644 --- a/cmd/shared/download.go +++ b/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,7 +109,12 @@ func waitForDownloadURL(ctx context.Context, kbClient kbclient.Client, req *nacv for { select { case <-timeoutCtx.Done(): - return "", FormatDownloadRequestTimeoutError(kbClient, req, timeout) + // 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{ @@ -207,7 +218,7 @@ func StreamDownloadContent(url string, writer io.Writer) error { 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(), 5*time.Second) + statusCtx, cancel := context.WithTimeout(context.Background(), defaultStatusCheckTimeout) defer cancel() var updated nacv1alpha1.NonAdminDownloadRequest From 9f3a3bedeb6212fc3faffb63a54ebb7f1fe0536e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Dec 2025 17:17:52 +0000 Subject: [PATCH 8/9] Use timeoutCtx for API calls and define default timeout constant Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com> --- cmd/non-admin/backup/logs.go | 9 ++++++++- cmd/shared/download.go | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cmd/non-admin/backup/logs.go b/cmd/non-admin/backup/logs.go index 8b70be5..65de474 100644 --- a/cmd/non-admin/backup/logs.go +++ b/cmd/non-admin/backup/logs.go @@ -30,6 +30,11 @@ 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 { c := &cobra.Command{ Use: use + " NAME", @@ -105,6 +110,8 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command { fmt.Fprintf(cmd.OutOrStdout(), "Waiting for backup logs to be processed (timeout: %v)...\n", timeout) // 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() @@ -159,7 +166,7 @@ func NewLogsCommand(f client.Factory, use string) *cobra.Command { Example: ` kubectl oadp nonadmin backup logs my-backup`, } - c.Flags().Duration("timeout", 5*time.Minute, "Maximum time to wait for logs to be available") + c.Flags().Duration("timeout", defaultLogsTimeout, "Maximum time to wait for logs to be available") return c } diff --git a/cmd/shared/download.go b/cmd/shared/download.go index 97acfd8..cb3e841 100644 --- a/cmd/shared/download.go +++ b/cmd/shared/download.go @@ -117,7 +117,7 @@ func waitForDownloadURL(ctx context.Context, kbClient kbclient.Client, req *nacv 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 { From d10b7dc0a72e0e13be83f6dead9d2a621e2acc4e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Dec 2025 16:25:20 +0000 Subject: [PATCH 9/9] Add configurable timeout to describe command Co-authored-by: kaovilai <11228024+kaovilai@users.noreply.github.com> --- cmd/non-admin/backup/backup_test.go | 3 +++ cmd/non-admin/backup/describe.go | 20 ++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/cmd/non-admin/backup/backup_test.go b/cmd/non-admin/backup/backup_test.go index c783c76..fae8cfb 100644 --- a/cmd/non-admin/backup/backup_test.go +++ b/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", }, }, { @@ -130,6 +132,7 @@ func TestNonAdminBackupCommands(t *testing.T) { expectContains: []string{ "Describe non-admin resources", "backup", + "--timeout", }, }, { diff --git a/cmd/non-admin/backup/describe.go b/cmd/non-admin/backup/describe.go index 1215cd7..0eaad47 100644 --- a/cmd/non-admin/backup/describe.go +++ b/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,13 +61,15 @@ 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) @@ -65,7 +77,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) { +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