Add flag status-check-deadline instead of default 10 minutes

[tejal29]

Change the deadline for deployments to stabilize from 10 mins to read from command line.
The flag's default value is 10m

[codecov]

Codecov Report

Merging #2591 into master will decrease coverage by <.01%.
The diff coverage is 46.66%.

Impacted Files	Coverage Δ
pkg/skaffold/runner/new.go	70.07% <ø> (ø)	⬆️
pkg/skaffold/schema/latest/config.go	100% <ø> (ø)	⬆️
pkg/skaffold/schema/defaults/defaults.go	90.41% <ø> (ø)	⬆️
pkg/skaffold/schema/versions.go	74.35% <ø> (ø)	⬆️
cmd/skaffold/app/cmd/flags.go	100% <ø> (ø)	⬆️
pkg/skaffold/deploy/status_check.go	63.23% <46.15%> (-4.46%)	⬇️
pkg/skaffold/runner/deploy.go	70.58% <50%> (ø)	⬆️
pkg/skaffold/server/server.go	58.57% <0%> (ø)	⬆️
pkg/skaffold/schema/profiles.go	90.47% <0%> (+2.04%)	⬆️

[nkubala]

I'm starting to get hesitant about adding more CLI flags to skaffold, we already have a ton and I think it's getting a little overwhelming. would this be more suited toward a global config value instead? or a field in the deploy stanza of the skaffold.yaml?

apiVersion: skaffold/v1beta13
kind: Config
deploy:
  statusCheckDeadline: 5

[tejal29]

I'm starting to get hesitant about adding more CLI flags to skaffold, we already have a ton and I think it's getting a little overwhelming. would this be more suited toward a global config value instead? or a field in the deploy stanza of the skaffold.yaml?

apiVersion: skaffold/v1beta13
kind: Config
deploy:
  statusCheckDeadline: 5

@nkubala I thought about where this flag could be,

1. The reason why i dont want this to be embeded in the config is i am not sure if "time for resources to stabalize" vary as per cluster size or cluster type.
   E.g. Alice is deploying on minikube, VS Bob is deploying on a shared cluster in this his namespace VS CI deploys in non shared private cluster with privately hosted image repository?

My thinking was k8 deploy check could be sensitive to all these hence embedding the health check flag in config would not work best. (users will have to change in skaffold.yaml)

The reason, i did not want stausCheckDeadline to be a global flag was, users might run skaffold deploy on various projects of different sizes and hence i wanted to scope it a project.

I am more inclined to embed it as in config if the flag is deploy cluster independent

[balopat]

I like the idea of adding it to the skaffold.yaml - and if people have one-off overrides they can file an issue for a flag. Chances are you set this once per profile and then you're golden.

I wouldn't think this is a global config either, it is highly project specific.

[nkubala]

it feels strange for this value to be an actual parsable "duration" in the config. can we just make it an int and then document that it refers to seconds? or maybe change the field itself to statusCheckDeadlineSeconds? then we won't have to special case this one particular field in the validation of the config.

deploy:
  statusCheckDeadline: 1 # refers to seconds, as per the docs

or

deploy:
  statusCheckDeadlineSeconds: 1

[tejal29]

it feels strange for this value to be an actual parsable "duration" in the config. can we just make it an int and then document that it refers to seconds? or maybe change the field itself to statusCheckDeadlineSeconds? then we won't have to special case this one particular field in the validation of the config.

deploy:
  statusCheckDeadline: 1 # refers to seconds, as per the docs

or

deploy:
  statusCheckDeadlineSeconds: 1

Done, Add this as StatusCheckDeadlineInSeconds in sync with kubernetes deadlineProgressInSeconds field in deployment.Spec

addressed