Skip to content

Commit

Permalink
fix: add timeout to apply/replace kubectl calls (argoproj#572)
Browse files Browse the repository at this point in the history
Signed-off-by: Jonathan West <[email protected]>
  • Loading branch information
jgwest committed May 4, 2024
1 parent fbecbb8 commit 44817fb
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/utils/kube/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ const (
HorizontalPodAutoscalerKind = "HorizontalPodAutoscaler"
)

const (
// defaultKubectlRequestTimeout is the timeout value used when calling the 'apply' command of kubectl. The previous default was no timeout, which would allow apply operation to potentially run forever, thus leaking YAML files into /dev/shm until Pod restart.
defaultKubectlRequestTimeout = time.Hour * 1
)

type ResourceInfoProvider interface {
IsNamespaced(gk schema.GroupKind) (bool, error)
}
Expand Down
13 changes: 13 additions & 0 deletions pkg/utils/kube/resource_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ func (k *kubectlResourceOperations) ReplaceResource(ctx context.Context, obj *un
if err != nil {
return err
}

// When calling the kubectl 'replace' command, it will run _without_ a timeout (as of this writing, May 2024):
// - If users are finding that 'replace' operations are running forever (and thus leaking manifest files into '/dev/shm'), one can enable 'force' via sync options or annotation, which will enable a default timeout of 5 minutes within the 'replace' kubectl call.
// - However, this guidance apply to replace options only (i.e. not apply).

return replaceOptions.Run(f)
})
}
Expand Down Expand Up @@ -261,6 +266,14 @@ func (k *kubectlResourceOperations) ApplyResource(ctx context.Context, obj *unst
if err != nil {
return err
}

// If no timeout is specified (and thus an infinite wait), we will substitute a LONG default value.
// This allows enough time to complete for any valid, expected long running apply operations, while also preventing excessive leaks of resources into /dev/shm, due to operations that are likely never going to complete.
if applyOpts.DeleteOptions.Timeout == 0 {
// Yes, this is correct: Apply uses the 'DeleteOptions' struct to set the timeout val
applyOpts.DeleteOptions.Timeout = defaultKubectlRequestTimeout
}

return applyOpts.Run()
})
}
Expand Down

0 comments on commit 44817fb

Please sign in to comment.