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

Don't block waiting for individual deployments #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daniel-ac-martin
Copy link
Contributor

Waits until all resources have been created / updated before checking
the roll-out of a deployment. This way it is possible to deploy quicker
as deployments can roll-out parallel to one another.

Addresses #82.

Waits until all resources have been created / updated before checking
the roll-out of a deployment. This way it is possible to deploy quicker
as deployments can roll-out parallel to one another.
Copy link
Contributor

@marcinc marcinc left a comment

Choose a reason for hiding this comment

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

Would prefer that to be a bool flag option, say skip-watch or similar. By default watching should be enabled but could be skipped with that flag set.

@daniel-ac-martin
Copy link
Contributor Author

@marcinc: The PR doesn't actually skip the watching it just defers it until the rest of the Kubernetes resources in a multi-resource YAML file have been created / updated.

@gambol99
Copy link
Contributor

gambol99 commented Jun 19, 2018

hi @daniel-ac-martin, I like the feature but do kinda agree with @marcinc .. It fundamentally changes deployment behavior and should be gated with a flag .. You go even go the full hog and thread the watchResource .. or perhaps another PR

@daniel-ac-martin
Copy link
Contributor Author

Okay. Well I can add a flag if you like. But it's worth pointing out that supporting both behaviours will complicate the code a little.

In terms of changing the deployment behaviour it's worth bearing in mind that it wasn't too long ago that multi-resource YAML files didn't support watching. Are many people using multi-resource YAML files?

If we do add a flag it also raises the question of what the default behaviour should be?

@gambol99
Copy link
Contributor

gambol99 commented Jun 19, 2018

hey @daniel-ac-martin ... maybe i'm misreading the code but from a glance ...

throwing aside the yaml splitting .. if I have/had multiple deployments all of them are going to be deployed in one iteration and checked afterwards ... rather than before which was deploy and check, deploy and check .. etc which is a pretty big change

@daniel-ac-martin
Copy link
Contributor Author

Ah, I didn't think about multiple files being deployed at once. (Didn't know that was supported!)

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