Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add timeout to apply/replace kubectl calls (#572) #573

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading