Add timeout option and env var to the OADP CLI tool #100
+1,133
−35
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prevents OADP CLI operation hanging indefinitely. Introduced default timeout of 10min which can be controlled via OADP_CLI_HTTP_TIMEOUT variable. Co-Authored-By: Claude <[email protected]> Signed-off-by: Michal Pryc <[email protected]>
This commit adds a --request-timeout flag (following kubectl conventions) to all OADP CLI commands, with full support for cluster unreachability: - Add --request-timeout flag to nonadmin backup logs and describe commands (takes precedence over OADP_CLI_REQUEST_TIMEOUT env var) - Add timeoutFactory wrapper that applies dial timeout to all Velero commands by overriding all client-creating methods (KubeClient, DynamicClient, DiscoveryClient, KubebuilderClient, KubebuilderWatchClient) - Add renameTimeoutFlag to rename Velero's --timeout to --request-timeout for consistent kubectl-style CLI experience - Set custom net.Dialer with timeout to handle TCP dial timeouts - Add context-based timeout handling with proper cancellation detection - Add FormatDownloadRequestTimeoutError for helpful timeout diagnostics - Add tests for timeout functionality (global timeout, config application, dialer timeout behavior) The timeout now applies to both HTTP requests and TCP connection attempts across all commands, ensuring the CLI times out quickly when the cluster is unreachable instead of waiting for the default ~30s TCP timeout. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Michal Pryc <[email protected]>
kaovilai
reviewed
Jan 15, 2026
Comment on lines
+40
to
+42
| // TimeoutEnvVar is the environment variable name that can be used to override the default timeout. | ||
| // Example: OADP_CLI_REQUEST_TIMEOUT=30m kubectl oadp nonadmin backup logs my-backup | ||
| const TimeoutEnvVar = "OADP_CLI_REQUEST_TIMEOUT" |
Member
There was a problem hiding this comment.
Ideally should document valid formats, ie. days month year valid? only hour/min/second?
Member
There was a problem hiding this comment.
could mention/link the libraries used to parse which has doc.
kaovilai
reviewed
Jan 15, 2026
| } | ||
| statusInfo = fmt.Sprintf(" Current status: %s.", strings.Join(conditions, ", ")) | ||
| } | ||
| return fmt.Errorf("timed out after %v waiting for NonAdminDownloadRequest %q to be processed.%s", timeout, req.Name, statusInfo) |
Member
There was a problem hiding this comment.
Suggested change
| 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. statusInfo: %s", timeout, req.Name, statusInfo) |
kaovilai
approved these changes
Jan 15, 2026
Member
kaovilai
left a comment
kaovilai
left a comment
There was a problem hiding this comment.
few nits. could be follow up
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prevents OADP CLI operation hanging indefinitely.
This PR adds a --request-timeout flag (following kubectl conventions)
to all OADP CLI commands, with full support for cluster unreachability:
(takes precedence over OADP_CLI_REQUEST_TIMEOUT env var)
commands by overriding all client-creating methods (KubeClient,
DynamicClient, DiscoveryClient, KubebuilderClient, KubebuilderWatchClient)
for consistent kubectl-style CLI experience
dialer timeout behavior)
The timeout now applies to both HTTP requests and TCP connection
attempts across all commands, ensuring the CLI times out quickly when
the cluster is unreachable instead of waiting for the default ~30s TCP
timeout.
This PR includes similar changes to #99