Skip to content

Reduce default status check deadline to 2 mins#3687

Merged
dgageot merged 4 commits intoGoogleContainerTools:masterfrom
tejal29:reduce_deadline
Feb 20, 2020
Merged

Reduce default status check deadline to 2 mins#3687
dgageot merged 4 commits intoGoogleContainerTools:masterfrom
tejal29:reduce_deadline

Conversation

@tejal29
Copy link
Copy Markdown
Contributor

@tejal29 tejal29 commented Feb 14, 2020

Related to #3638, #176

In this PR, we decrease the default Status check deadline to 3 seconds.

In this changes

  • if statusCheckDeadlineSeconds is present in the skaffold deploy config, respect that.
  • if not, then set deadline as maximum deadline seen in the progressDeadlineSeconds in Deployment.Spec
  • If none of them specified, use defaultStatusCheckDeadline

@tejal29 tejal29 changed the title Reduce deadline Reduce default status check deadline to 2 mins Feb 14, 2020
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 14, 2020

Codecov Report

Merging #3687 into master will decrease coverage by 0.03%.
The diff coverage is 75%.

Impacted Files Coverage Δ
pkg/skaffold/deploy/status_check.go 69.06% <75%> (+1.37%) ⬆️
pkg/skaffold/initializer/init.go 50% <0%> (-14.82%) ⬇️
...affold/kubernetes/portforward/kubectl_forwarder.go 65.85% <0%> (-2.44%) ⬇️
pkg/skaffold/docker/image.go 73.36% <0%> (-2.32%) ⬇️
pkg/skaffold/util/util.go 86.01% <0%> (-0.48%) ⬇️
pkg/skaffold/deploy/kubectl.go 66.4% <0%> (ø) ⬆️
cmd/skaffold/app/cmd/init.go 100% <0%> (ø) ⬆️
pkg/skaffold/initializer/deploy_init.go
pkg/skaffold/initializer/builders.go
pkg/skaffold/initializer/analyze.go
... and 25 more

if d.Spec.ProgressDeadlineSeconds == nil {
deadline = deadlineDuration
} else {
deadline = time.Duration(*d.Spec.ProgressDeadlineSeconds) * time.Second
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i dont understand what you mean?
Do you meant *d.Spec.ProgressDeadline * time.Second?
i see an IDE error

cannot use type int as type Duration in an assignment

and also a make error

pkg/skaffold/deploy/status_check.go:119:47: invalid operation: *d.Spec.ProgressDeadlineSeconds * time.Second (mismatched types int32 and time.Duration)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My bad. Only works with constants

@dgageot dgageot self-assigned this Feb 17, 2020
@tejal29
Copy link
Copy Markdown
Contributor Author

tejal29 commented Feb 19, 2020

@dgageot Addressed your comments. Please take another look.

if d.Spec.ProgressDeadlineSeconds == nil {
deadline = deadlineDuration
} else {
deadline = time.Duration(*d.Spec.ProgressDeadlineSeconds) * time.Second
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My bad. Only works with constants

@dgageot dgageot merged commit b436574 into GoogleContainerTools:master Feb 20, 2020
@tejal29 tejal29 deleted the reduce_deadline branch April 15, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants