Skip to content

Conversation

xinWeiWei24
Copy link
Contributor

@xinWeiWei24 xinWeiWei24 commented May 21, 2025

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.

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.
@xinWeiWei24 xinWeiWei24 requested a review from fuweid June 18, 2025 06:34
)

type DeploymentsTimeout struct {
deployTimeout time.Duration
Copy link
Collaborator

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)
           }
}

Copy link
Contributor Author

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.

Copy link
Collaborator

@fuweid fuweid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fuweid fuweid merged commit 76a0d12 into Azure:main Jul 18, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants