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

Reduce default status check deadline to 2 mins #3687

Merged
merged 4 commits into from
Feb 20, 2020

Conversation

tejal29
Copy link
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

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

pkg/skaffold/deploy/status_check.go Outdated Show resolved Hide resolved
@@ -110,7 +113,7 @@ func getDeployments(client kubernetes.Interface, ns string, l *DefaultLabeller,
deployments := make([]Resource, 0, len(deps.Items))
for _, d := range deps.Items {
var deadline time.Duration
if d.Spec.ProgressDeadlineSeconds == nil || *d.Spec.ProgressDeadlineSeconds > int32(deadlineDuration.Seconds()) {
if d.Spec.ProgressDeadlineSeconds == nil {
deadline = deadlineDuration
} else {
deadline = time.Duration(*d.Spec.ProgressDeadlineSeconds) * time.Second
Copy link
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
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
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

pkg/skaffold/deploy/status_check.go Show resolved Hide resolved
pkg/skaffold/deploy/status_check.go Show resolved Hide resolved
pkg/skaffold/deploy/status_check.go Show resolved Hide resolved
@dgageot dgageot self-assigned this Feb 17, 2020
@tejal29
Copy link
Contributor Author

tejal29 commented Feb 19, 2020

@dgageot Addressed your comments. Please take another look.

@@ -110,7 +113,7 @@ func getDeployments(client kubernetes.Interface, ns string, l *DefaultLabeller,
deployments := make([]Resource, 0, len(deps.Items))
for _, d := range deps.Items {
var deadline time.Duration
if d.Spec.ProgressDeadlineSeconds == nil || *d.Spec.ProgressDeadlineSeconds > int32(deadlineDuration.Seconds()) {
if d.Spec.ProgressDeadlineSeconds == nil {
deadline = deadlineDuration
} else {
deadline = time.Duration(*d.Spec.ProgressDeadlineSeconds) * time.Second
Copy link
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