[UNTESTED] immediately abort chef-client if maintenance mode is set #13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| if maintenance_mode? | ||
| raise \ | ||
| "Pacemaker maintenance mode was already set on " + | ||
| "#{node.hostname}; aborting! Please diagnose why this was the " + |
There was a problem hiding this comment.
Style/MultilineOperationIndentation: Use 2 (not 0) spaces for indenting an expression spanning multiple lines.
aspiers
force-pushed
the
abort-on-maintenance-mode
branch
from
2cc0c35 to
4352717
Compare
| "not by Chef::Provider::PacemakerService; leaving as is") | ||
| # This shouldn't happen, and suggests that something is | ||
| # interfering in a way it shouldn't. | ||
| raise "Something put node into maintenance mode during run!" |
There was a problem hiding this comment.
Not sure I would raise something here; if the admin did put the node in maintenance while chef-client was running, then, well, that's the way it is and why should chef fail because of that?
There was a problem hiding this comment.
Yeah OK, I guess a Chef::Log.warn is more appropriate here.
aspiers
force-pushed
the
abort-on-maintenance-mode
branch
from
4352717 to
d4eb66f
Compare
|
+1 |
added 2 commits
September 11, 2015 00:58
The reasoning is explained in the comments. I'm not sure why I ever thought it was a good idea to allow a chef-client run to proceed if maintenance mode is set. It's not even as if that approach could ever restore an ill node to full health, because we were deliberately checking whether the maintenance mode was set prior to the chef-client run, and if so, leaving it set. This way, if a node is left in maintenance mode, it will be discovered sooner, resulting in the cloud operator being alerted to a degraded cluster sooner.
aspiers
force-pushed
the
abort-on-maintenance-mode
branch
from
d4eb66f to
8661acd
Compare
| end | ||
|
|
||
| Chef::Log.debug("Cluster is up") | ||
| return true |
There was a problem hiding this comment.
Style/RedundantReturn: Redundant return detected.
|
Crap, raising an exception in the start handler doesn't abort the run :-( Need to find a way to do this. |
Make #maintenance_mode? return a sensible value even when Pacemaker is down or uninstalled. This is especially helpful when it is invoked by Chef's start_handler. Cherry-picked from crowbar/barclamp-pacemaker@7dff6c1.
aspiers
force-pushed
the
abort-on-maintenance-mode
branch
from
8661acd to
43590c4
Compare
|
@aspiers still relevant? if so, can you rebase? |
|
Changes Unknown when pulling 43590c4 on aspiers:abort-on-maintenance-mode into ** on crowbar:master**. |
|
@aspiers is this still alive or should we close it? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The reasoning is explained in the comments. I'm not sure why I ever thought it was a good idea to allow a chef-client run to proceed if maintenance mode is set. It's not even as if that approach could ever restore an ill node to full health, because we were deliberately checking whether the maintenance mode was set prior to the chef-client run, and if so, leaving it set. This way, if a node is left in maintenance mode, it will be discovered sooner, resulting in the cloud operator being alerted to a degraded cluster sooner.
Also ported crowbar/barclamp-pacemaker#149 on top of this.