-
Notifications
You must be signed in to change notification settings - Fork 57
set maintenance mode or stop corosync/pacemaker on update #563
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,5 +22,6 @@ | |
| version "0.0.1" | ||
|
|
||
| depends "utils" | ||
| depends "crowbar-pacemaker" | ||
|
|
||
| recipe "updater", "System Package Updater" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,9 @@ | |
| # limitations under the License. | ||
| # | ||
|
|
||
| ::Chef::Recipe.include CrowbarPacemaker::MaintenanceModeHelpers | ||
| ::Chef::Resource.include CrowbarPacemaker::MaintenanceModeHelpers | ||
|
|
||
| if !node[:updater].key?(:one_shot_run) || !node[:updater][:one_shot_run] | ||
|
|
||
| node[:updater][:one_shot_run] = true | ||
|
|
@@ -45,6 +48,65 @@ | |
| ignore_failure true | ||
| end | ||
|
|
||
| ruby_block "check for updates" do | ||
| block do | ||
| case node[:updater][:zypper][:method] | ||
| when "patch" | ||
| command = "list-patches" | ||
| when "update" | ||
| command = "list-updates" | ||
| when "dist-upgrade" | ||
| command = "list-updates" | ||
| end | ||
|
|
||
| node.run_state["needs_update"] = `zypper -q #{command}|wc -l`.chomp.to_i > 0 | ||
|
|
||
| command += '|egrep -q "corosync|pacemaker"' | ||
| system("zypper #{command}") | ||
| # exit 0: found, 1 not found | ||
| node.run_state["found_ha_packages"] = $?.exitstatus ? true : false | ||
| end | ||
| end | ||
|
|
||
| ["corosync", "pacemaker"].each do |s| | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The HA docs only say to stop pacemaker, why are both here? Could you add a comment? |
||
| service s do | ||
| action :stop | ||
| only_if { node.run_state["found_ha_packages"] } | ||
| not_if { node[:pacemaker] && node[:pacemaker][:is_remote] } | ||
| end | ||
| end | ||
|
|
||
| # set cluster to maintenance if | ||
| # HA packages are NOT gonna be updated | ||
| # And Node is part of a cluster | ||
| # And there is packages to update | ||
| execute "crm --wait node maintenance" do | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could add the remote node name following what crowbar/crowbar-ha#187 did |
||
| action :nothing | ||
| notifies :create, "ruby_block[set maintenance mode via this chef run]", :immediately | ||
| end | ||
|
|
||
| ruby_block "set maintenance mode via this chef run" do | ||
| action :nothing | ||
| block do | ||
| set_maintenance_mode_via_this_chef_run | ||
| end | ||
| end | ||
|
|
||
| ruby_block "set cluster maintenance" do | ||
| block do | ||
| Chef::Log.info("Triggering maintenance mode for this node") | ||
| true | ||
| end | ||
| only_if do | ||
| is_cluster = node.role? "pacemaker-cluster-member" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With crowbar/crowbar-ha#187 this could probably include pacemaker-remote roles too, yes? |
||
| !node.run_state["found_ha_packages"] && is_cluster && node.run_state["needs_update"] | ||
| end | ||
| not_if do | ||
| maintenance_mode_set_via_this_chef_run? && maintenance_mode? | ||
| end | ||
| notifies :run, "execute[crm --wait node maintenance]", :immediately | ||
| end | ||
|
|
||
| # Butt-ugly, enhance Chef::Provider::Package::Zypper later on... | ||
| ruby_block "running \"#{zypper_command}\"" do | ||
| block do | ||
|
|
@@ -94,8 +156,14 @@ | |
| end # case | ||
| end # while | ||
| end # block | ||
| only_if { node.run_state["needs_update"] } | ||
| end # ruby_block | ||
|
|
||
| service "pacemaker" do | ||
| action :start | ||
| not_if { node[:pacemaker] && node[:pacemaker][:is_remote] } | ||
| end | ||
|
|
||
| end # platform_family suse block | ||
|
|
||
| # handle case where there is a reboot needed from a previous run | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/SpecialGlobalVars: Prefer$CHILD_STATUS from the stdlib 'English' module (don't forget to require it) over $ ?. (https://github.com/bbatsov/ruby-style-guide#no-cryptic-perlisms)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree with hound here, I don't think $? is a cryptic perlism.
But I think the logic is wrong - $?.exitstatus produces the numeric exit code, and grep will return 1 if no matches were found, so
1 ? true : falseresults in true, so the pacemaker services get stopped even when it's not necessary.Also, if needs_update ends up being false you could probably skip the extra zypper call.