-
Notifications
You must be signed in to change notification settings - Fork 7
contrib/utils: Add defaut timeout for deployments and jobs #167
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
Conversation
Fixed timeouts could cause failures in benchmarking for large-scale deployments due to longer operation times. This change ensures smoother benchmarking by adapting timeouts to cluster size.
contrib/utils/utils.go
Outdated
) | ||
|
||
type DeploymentsTimeout struct { | ||
deployTimeout time.Duration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this field is unvisiable to other package. I think we can handle it like this.
type deploymentTimeoutOption struct {
deployTimeout time.Duration
restartTimeout time.Duration
rolloutTimeout time.Duration
rolloutInterval time.Duration
}
type DeploymentTimeoutOpt func(*deploymentTimeoutOption)
func WithDeploymentDeployTimeoutOpt(to time.Duration) DeploymentTimeoutOpt {
return func(dto *deploymentTimeoutOption) {
dto.deployTimeout = to
}
}
...
func DeployAndRepeatRollingUpdateDeployments(
ctx context.Context,
kubeCfgPath string,
releaseName string,
total, replica, paddingBytes int,
timeoutOps DeploymentTimeoutOpt...,
) (rollingUpdateFn, cleanupFn func(), retErr error) {
var to = &deploymentTimeout{
deployTimeout: 10 * time.Minute,
restartTimeout: 2 * time.Minute,
rolloutTimeout: 10 * time.Minute,
rolloutInterval: 5 * time.Second
}
for _, opt := range timeoutOpts {
opt(to)
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change it as suggested and include a new file containing the functions.
All conflicts have been successfully resolved.
This reverts commit 0d79a24.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixed timeouts could cause failures in benchmarking for large-scale deployments due to longer operation times. This change introduces configurable timeout settings for deploying both deployments and jobs, allowing greater flexibility and control during rollout.