fix: warn when config pre-loading times out and make the timeout configurable#3287
Open
jtprogru wants to merge 1 commit into
Open
fix: warn when config pre-loading times out and make the timeout configurable#3287jtprogru wants to merge 1 commit into
jtprogru wants to merge 1 commit into
Conversation
…igurable Config pre-loading (used to register custom commands and dynamic pipeline flags) was cancelled after a hardcoded 10s timeout. When variable resolution took longer than that, the error was swallowed and commands later failed with unrelated errors like 'unknown flag: --apps'. - print a clear warning when pre-loading exceeds the timeout, explaining that custom commands, pipeline flags and variables from the config might be unavailable for this run - defer pre-load log messages until the log level flags are parsed so --silent suppresses them, shell completions stay silent, and on fatal errors such as 'unknown flag' the warning is flushed right before the error - keep the underlying load error (Unwrap) and log it at debug level so --debug reveals the real cause, e.g. which variable timed out - allow overriding the timeout via the DEVSPACE_PRELOAD_TIMEOUT environment variable (a duration such as 30s or plain seconds; 0 disables the timeout, and the plain-seconds path guards against int64 overflow) - skip the variable resolver in RawConfig.GetEnv when its context is already cancelled to avoid doomed command executions after a timeout - document the pre-loading timeout in the variables docs Fixes devspace-sh#3213
✅ Deploy Preview for devspace-docs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What issue type does this pull request address?
/kind bug
What does this pull request do? Which issues does it resolve?
Fixes #3213
Config pre-loading in
BuildRoot(used to register custom commands and dynamic pipeline flags) is cancelled after a hardcoded 10s timeout. When variable resolution takes longer (e.g. secrets fetched via an external CLI in thevarsblock), the failure was swallowed entirely:parseConfigreturned the partial config with anilerror, the dynamic pipeline flags were silently never registered, and the command later died with an unrelatedfatal unknown flag: --apps.This PR surfaces the timeout and makes it configurable:
unknown flag, the warning is printed right before the error so the failure is explained.DEVSPACE_PRELOAD_TIMEOUTenvironment variable: a duration such as30s/1m, plain seconds such as30, or0to disable the timeout entirely (the plain-seconds path guards againstint64overflow by treating overflowing values as "no timeout").--silentsuppresses the warning and shell completions (__complete) stay silent instead of emitting the warning on every TAB press.Unwrap) and logged at debug level, so--debugreveals the real cause, e.g.fill variable BLAH with command 'sleep 12 && …': context deadline exceeded— previously it was discarded.RawConfig.GetEnvnow skips the variable resolver when its context is already cancelled, avoiding doomed command executions (spawn + immediate kill) for every env lookup after a timeout.From Commandsection), including the note that the variable must be set in the process environment because it is read before.envfiles are loaded.Please provide a short message that should be published in the DevSpace release notes
Fixed an issue where slow variable resolution during config pre-loading caused confusing
unknown flagerrors; DevSpace now prints a clear warning and the timeout is configurable viaDEVSPACE_PRELOAD_TIMEOUT(0disables it).What else do we need to know?
Verified manually against the reproduction from #3213 (a
varscommand sleeping 12s plus a pipeline flag): the default run now prints the warning beforeunknown flag: --apps; withDEVSPACE_PRELOAD_TIMEOUT=30s,=13(plain seconds) or=0the flag is registered and parsed correctly;--silentsuppresses the warning;--debugshows the underlying cause; invalid values (abc, negative) warn and fall back to the 10s default; fast configs behave exactly as before.go test ./cmd,go vetandgolangci-lint(repo config) pass with no new issues.One known limitation kept out of scope: when the timeout does fire, pipeline commands still hard-fail on their dynamic flags — cobra rejects them before
RunE. A deeper fix would be adopting theDisableFlagParsing+ late-registration pattern thatdevspace runalready uses (which is whydevspace runsurvives a pre-load timeout), but that requires refactoring the sharedRunPipelineCmdplumbing and seems better suited for a follow-up. Happy to open an issue for it or take a stab in a separate PR if you agree with the direction.