From 91d7f115ee9582f8cab35b34868bf8c23abb3f46 Mon Sep 17 00:00:00 2001 From: "R.I.Pienaar" Date: Mon, 26 Dec 2016 14:00:52 +0100 Subject: [PATCH 1/2] (#102) support arguments to scripts used in shell nodes Now uses the M::Shell system to run the commands and additionally checks for exit code 0, only accepts STDOUT specifically --- CHANGELOG.md | 1 + lib/mcollective/util/playbook/nodes/shell_nodes.rb | 12 ++++++++++-- .../util/playbook/nodes/shell_nodes_spec.rb | 10 +++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6187757e..cfea6f0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ |Date |Issue |Description | |----------|------|---------------------------------------------------------------------------------------------------------| +|2016/12/26|102 |Support arguments to commands ran by the shell node set in playbooks | |2016/12/26| |Release 0.0.12 | |2016/12/26|104 |Support posting messages to slack as a task step | |2016/12/25|95 |Support running shell commands as task steps | diff --git a/lib/mcollective/util/playbook/nodes/shell_nodes.rb b/lib/mcollective/util/playbook/nodes/shell_nodes.rb index f9cfcb2b..f3ebfe6d 100644 --- a/lib/mcollective/util/playbook/nodes/shell_nodes.rb +++ b/lib/mcollective/util/playbook/nodes/shell_nodes.rb @@ -17,6 +17,7 @@ def validate_configuration! def from_hash(data) @script = data["script"] + @_data = nil self end @@ -28,10 +29,17 @@ def valid_hostname?(host) def data return @_data if @_data - @_data = `#{@script}`.lines.map do |line| + shell = Shell.new(@script, "timeout" => 10) + shell.runcommand + + exitcode = shell.status.exitstatus + + raise("Could not disocver nodes via shell method, command exited with code %d" % [exitcode]) unless exitcode == 0 + + @_data = shell.stdout.lines.map do |line| line.chomp! - raise("%s is not a valid certname" % line) unless valid_hostname?(line) + raise("%s is not a valid hostname" % line) unless valid_hostname?(line) line end diff --git a/spec/unit/mcollective/util/playbook/nodes/shell_nodes_spec.rb b/spec/unit/mcollective/util/playbook/nodes/shell_nodes_spec.rb index 1456ead4..27a129a1 100644 --- a/spec/unit/mcollective/util/playbook/nodes/shell_nodes_spec.rb +++ b/spec/unit/mcollective/util/playbook/nodes/shell_nodes_spec.rb @@ -50,7 +50,15 @@ class Nodes describe "#data" do it "should detect corrupt data" do nodes.from_hash("script" => corrupt_fixture) - expect { nodes.data }.to raise_error("node1 example.net is not a valid certname") + expect { nodes.data }.to raise_error("node1 example.net is not a valid hostname") + end + + it "should only accept outputs that exit with status 0" do + nodes.from_hash("script" => "echo 'example.net';exit 0") + expect(nodes.data).to eq(["example.net"]) + + nodes.from_hash("script" => "echo 'example.net';exit 1") + expect { nodes.data }.to raise_error("Could not disocver nodes via shell method, command exited with code 1") end end From 078a8f883abcd317480e9712e2134c69f64f6c56 Mon Sep 17 00:00:00 2001 From: "R.I.Pienaar" Date: Mon, 26 Dec 2016 14:05:58 +0100 Subject: [PATCH 2/2] (#102) drop duplicate discovered names --- lib/mcollective/util/playbook/nodes.rb | 2 +- spec/unit/mcollective/util/playbook/nodes_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mcollective/util/playbook/nodes.rb b/lib/mcollective/util/playbook/nodes.rb index f28fb75e..640ed473 100644 --- a/lib/mcollective/util/playbook/nodes.rb +++ b/lib/mcollective/util/playbook/nodes.rb @@ -79,7 +79,7 @@ def prepare def resolve_nodes(node_set) node_props = @nodes[node_set] node_props[:resolver].prepare - node_props[:discovered] = node_props[:resolver].discover + node_props[:discovered] = node_props[:resolver].discover.uniq end # Checks if the agents on the nodes matches the desired versions diff --git a/spec/unit/mcollective/util/playbook/nodes_spec.rb b/spec/unit/mcollective/util/playbook/nodes_spec.rb index eea754fe..52f6146c 100644 --- a/spec/unit/mcollective/util/playbook/nodes_spec.rb +++ b/spec/unit/mcollective/util/playbook/nodes_spec.rb @@ -133,7 +133,7 @@ class Playbook seq = sequence(:nodes) resolver = nodes.nodes["load_balancers"][:resolver] resolver.expects(:prepare).in_sequence(seq) - resolver.expects(:discover).in_sequence(seq).returns(["rspec1", "rspec2"]) + resolver.expects(:discover).in_sequence(seq).returns(["rspec1", "rspec2", "rspec1"]) nodes.resolve_nodes("load_balancers")