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

Add an annotation that skips rollout verification #282

Closed
wants to merge 1 commit into from

Conversation

dturn
Copy link
Contributor

@dturn dturn commented Apr 17, 2018

Allow a resource to skip rollout verification via a new annotation 'kubernetes-deploy.shopify.io/no-rollout-verification'

This prevents the deploy from being a failure if the resource fails to deploy, by causing ResourceWatcher to ignore the resource though it does print a message saying its ignoring the resource.

Feature wanted from: #229

screen shot 2018-04-18 at 10 17 50 am

@dturn dturn requested review from KnVerey and klautcomputing and removed request for KnVerey April 17, 2018 16:43
Copy link
Contributor

@klautcomputing klautcomputing left a comment

Choose a reason for hiding this comment

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

Looks solid to me and there are tests!

We should try it out somewhere to that we know it works in production.

@@ -190,6 +190,32 @@ def test_restart_failure
in_order: true)
end

def test_restart_failure_can_be_ignored
success = deploy_fixtures("hello-cloud", subset: ["configmap-data.yml", "web.yml.erb"]) do |fixtures|
Copy link
Contributor

Choose a reason for hiding this comment

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

This is my ultimate nitpick: success is what we like, but so far we only know it's a result

@dturn
Copy link
Contributor Author

dturn commented May 9, 2018

@KnVerey No rush on this, but want to make sure its on your radar

@dturn
Copy link
Contributor Author

dturn commented Sep 13, 2018

We're closing this pr unmerged because it doesn't seem important enough to justify making the state tracking code more cluttered. We have an issue to improve this code, but its not high priority. If this feature becomes high priority we can either bring this back as is or fix up the state tracking and then build this the right way.

@dturn dturn closed this Sep 13, 2018
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.

2 participants