Skip to content

Commit 1350684

Browse files
committed
fix: warn when config pre-loading times out and make the timeout configurable
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 #3213 Signed-off-by: Mikhail Savin <jtprogru@gmail.com>
1 parent 8ff6260 commit 1350684

2 files changed

Lines changed: 120 additions & 6 deletions

File tree

cmd/root.go

Lines changed: 116 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import (
55
"flag"
66
"fmt"
77
"io"
8+
"math"
89
"os"
10+
"strconv"
911
"strings"
1012
"sync"
1113
"time"
@@ -67,6 +69,12 @@ func NewRootCmd(f factory.Factory) *cobra.Command {
6769
log.SetLevel(logrus.DebugLevel)
6870
}
6971

72+
// print log messages deferred during config pre-loading, now that the
73+
// log level from the flags is known; skip them for shell completions
74+
if cobraCmd.Name() != cobra.ShellCompRequestCmd && cobraCmd.Name() != cobra.ShellCompNoDescRequestCmd && cobraCmd.Name() != "completion" {
75+
flushPreloadLogs(log)
76+
}
77+
7078
ansi.DisableColors(globalFlags.NoColors)
7179

7280
if globalFlags.KubeConfig != "" {
@@ -159,6 +167,10 @@ func Execute() {
159167
os.Exit(retCode.ExitCode)
160168
}
161169

170+
// print any log messages deferred during config pre-loading, so e.g. an
171+
// "unknown flag" error caused by a pre-load timeout is explained
172+
flushPreloadLogs(f.GetLog())
173+
162174
// error hooks
163175
pluginErr := hook.ExecuteHooks(nil, map[string]interface{}{"error": err}, "root.errorExecution", "command:error")
164176
if pluginErr != nil {
@@ -200,8 +212,16 @@ func BuildRoot(f factory.Factory, excludePlugins bool) *cobra.Command {
200212
if os.Getenv(expression.DevSpaceSkipPreloadEnv) == "" {
201213
rawConfig, err = parseConfig(f)
202214
if err != nil {
203-
f.GetLog().Debugf("error parsing raw config: %v", err)
204-
} else {
215+
// defer the messages until the log level from the flags is known
216+
var timeoutErr *preloadTimeoutError
217+
if errors.As(err, &timeoutErr) {
218+
deferPreloadLog(logrus.WarnLevel, timeoutErr.Error())
219+
deferPreloadLog(logrus.DebugLevel, fmt.Sprintf("error pre-loading config: %v", timeoutErr.Unwrap()))
220+
} else {
221+
deferPreloadLog(logrus.DebugLevel, fmt.Sprintf("error parsing raw config: %v", err))
222+
}
223+
}
224+
if rawConfig != nil {
205225
env.GlobalGetEnv = rawConfig.GetEnv
206226
}
207227
}
@@ -318,6 +338,84 @@ func disableKlog() {
318338
klogv2.SetOutput(io.Discard)
319339
}
320340

341+
// DevSpacePreloadTimeoutEnv can be used to change the maximum time DevSpace waits
342+
// for pre-loading the config (e.g. resolving variables from commands) before
343+
// executing a command. Accepts a duration such as 30s or 1m as well as plain
344+
// seconds; 0 disables the timeout. Must be set in the process environment, as it
345+
// is read before .env files and config variables are loaded.
346+
const DevSpacePreloadTimeoutEnv = "DEVSPACE_PRELOAD_TIMEOUT"
347+
348+
// defaultPreloadTimeout is the default timeout for pre-loading the config
349+
const defaultPreloadTimeout = time.Second * 10
350+
351+
// preloadLogs collects log messages produced while pre-loading the config in
352+
// BuildRoot, which runs before cobra has parsed the log-level flags; they are
353+
// flushed once the final log level is known (or before a fatal error)
354+
type preloadLog struct {
355+
level logrus.Level
356+
message string
357+
}
358+
359+
var preloadLogs []preloadLog
360+
361+
func deferPreloadLog(level logrus.Level, message string) {
362+
preloadLogs = append(preloadLogs, preloadLog{level: level, message: message})
363+
}
364+
365+
func flushPreloadLogs(logger log.Logger) {
366+
for _, l := range preloadLogs {
367+
if l.level == logrus.WarnLevel {
368+
logger.Warnf("%s", l.message)
369+
} else {
370+
logger.Debugf("%s", l.message)
371+
}
372+
}
373+
preloadLogs = nil
374+
}
375+
376+
// preloadTimeoutError is returned by parseConfig if pre-loading the config was
377+
// cancelled because it took longer than the configured timeout
378+
type preloadTimeoutError struct {
379+
timeout time.Duration
380+
err error
381+
}
382+
383+
func (e *preloadTimeoutError) Error() string {
384+
return fmt.Sprintf("pre-loading the config took longer than %s and was cancelled, which usually means resolving a variable or expression took too long. Custom commands, pipeline flags and variables defined in the config might be unavailable for this run. You can increase this timeout via the %s environment variable (0 disables it), e.g. %s=30s", e.timeout, DevSpacePreloadTimeoutEnv, DevSpacePreloadTimeoutEnv)
385+
}
386+
387+
func (e *preloadTimeoutError) Unwrap() error {
388+
return e.err
389+
}
390+
391+
// preloadTimeout returns the timeout for pre-loading the config, 0 meaning no timeout
392+
func preloadTimeout() time.Duration {
393+
value := os.Getenv(DevSpacePreloadTimeoutEnv)
394+
if value == "" {
395+
return defaultPreloadTimeout
396+
}
397+
398+
timeout, err := time.ParseDuration(value)
399+
if err != nil {
400+
// also allow specifying plain seconds, e.g. DEVSPACE_PRELOAD_TIMEOUT=30
401+
seconds, convErr := strconv.Atoi(value)
402+
if convErr != nil {
403+
deferPreloadLog(logrus.WarnLevel, fmt.Sprintf("Invalid value %q for %s, falling back to %s: %v", value, DevSpacePreloadTimeoutEnv, defaultPreloadTimeout, err))
404+
return defaultPreloadTimeout
405+
}
406+
if int64(seconds) > math.MaxInt64/int64(time.Second) {
407+
// converting to a duration would overflow; treat as no timeout
408+
return 0
409+
}
410+
timeout = time.Duration(seconds) * time.Second
411+
}
412+
if timeout < 0 {
413+
deferPreloadLog(logrus.WarnLevel, fmt.Sprintf("Invalid value %q for %s, falling back to %s: timeout must not be negative", value, DevSpacePreloadTimeoutEnv, defaultPreloadTimeout))
414+
return defaultPreloadTimeout
415+
}
416+
return timeout
417+
}
418+
321419
func parseConfig(f factory.Factory) (*RawConfig, error) {
322420
// get current working dir
323421
cwd, err := os.Getwd()
@@ -341,15 +439,26 @@ func parseConfig(f factory.Factory) (*RawConfig, error) {
341439
}
342440

343441
// Parse commands
344-
timeoutCtx, cancel := context.WithTimeout(context.Background(), time.Second*10)
345-
defer cancel()
442+
timeoutCtx := context.Background()
443+
timeout := preloadTimeout()
444+
if timeout > 0 {
445+
var cancel context.CancelFunc
446+
timeoutCtx, cancel = context.WithTimeout(timeoutCtx, timeout)
447+
defer cancel()
448+
}
346449

347450
r := &RawConfig{
348451
resolved: map[string]string{},
349452
}
350453
_, err = configLoader.LoadWithParser(timeoutCtx, nil, nil, r, &loader.ConfigOptions{
351454
Dry: true,
352455
}, log.Discard)
456+
if err != nil && timeoutCtx.Err() != nil {
457+
// pre-loading was cancelled by the timeout, so custom commands and pipeline
458+
// flags from the config might be missing; return a dedicated error to avoid
459+
// confusing follow-up errors such as "unknown flag"
460+
return r, &preloadTimeoutError{timeout: timeout, err: err}
461+
}
353462
if r.Resolver != nil {
354463
return r, nil
355464
}
@@ -394,8 +503,9 @@ func (r *RawConfig) GetEnv(name string) string {
394503
return value
395504
}
396505

397-
// try to find devspace variable
398-
if r.Resolver != nil {
506+
// try to find devspace variable; skip the resolver if its context is already
507+
// cancelled (e.g. after a pre-load timeout), as resolving could never succeed
508+
if r.Resolver != nil && r.Ctx != nil && r.Ctx.Err() == nil {
399509
r.resolvedMutex.Lock()
400510
defer r.resolvedMutex.Unlock()
401511

docs/pages/configuration/variables.mdx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ vars:
7373
args: ["rev-parse", "HEAD"]
7474
```
7575

76+
:::warning Pre-Loading Timeout
77+
Before executing a command, DevSpace pre-loads the config (including resolving variables from commands) to make custom commands and pipeline flags available. This pre-loading is limited to 10 seconds by default. If resolving all variables takes longer than that, DevSpace prints a warning and custom commands and pipeline flags defined in the config will be unavailable for this run. You can change this timeout via the `DEVSPACE_PRELOAD_TIMEOUT` environment variable, e.g. `DEVSPACE_PRELOAD_TIMEOUT=30s` (plain seconds such as `30` also work, and `0` disables the timeout entirely). Note that `DEVSPACE_PRELOAD_TIMEOUT` must be set in the process environment: it is read before `.env` files (loaded via `DEVSPACE_ENV_FILE`) and config variables are processed, so setting it there has no effect.
78+
:::
79+
7680

7781
### From User Input (Question)
7882
DevSpace can also ask the user to provide a value for a variable and you can provide a custom question and configure other input attributes for the question:

0 commit comments

Comments
 (0)