Issue #17603 has been reported by Josh Cooper.
----------------------------------------
Bug #17603: Puppet execute has unexpected semantics
https://projects.puppetlabs.com/issues/17603
Author: Josh Cooper
Status: Accepted
Priority: Normal
Assignee: eric sorenson
Category:
Target version:
Affected Puppet version: 0.22.2
Keywords: execute
Branch:
In the beginning, `Puppet::Util.execute` had a `failonfail` argument whose
default value was true, i.e. the method would raise an exception if the command
returned a non-zero exit code. See commit
2fcbc7f5433fab36b60bedf5a97dd610ef231721.
In commit efe9a833c43358e23ae252456a07b37cc9904a0a, released in 0.22.4, the
`execute` method was refactored, which resulted in a subtle change in
semantics. The `failonfail` argument defaulted to true provided no other
arguments were specified. So `execute(command, {:squelch => true})` would make
`failonfail` be false.
In commit d32d7f30f672e59c1427d9dfe32e7b7be35a48ab, released in 0.24.7 for
#1752, the `combine` argument was added, and it had the same unexpected
semantics as `failonfail`.
Puppet itself is not consistent in how it calls the `execute` method. For
example, `lib/puppet/indirector/exec.rb` calls execute in a begin/rescue block,
clearly expecting `failonfail` to be true, but since it specifies `combine =>
true`, `failonfail` is actually false and an exception is never raised. Other
providers, like `macauthorization.rb`, pass in `combine => false`, so
`failonfail` is false, but they don't use a begin/rescue block and so are not
expecting an exception to be raised. Other providers with similar issues:
<pre>
lib/puppet/indirector/exec.rb
lib/puppet/provider.rb
lib/puppet/provider/macauthorization/macauthorization.rb
lib/puppet/provider/package/dpkg.rb
lib/puppet/provider/package/macports.rb
lib/puppet/provider/package/msi.rb
lib/puppet/provider/package/sun.rb
lib/puppet/provider/service/freebsd.rb
lib/puppet/provider/zpool/zpool.rb
lib/puppet/util.rb
lib/puppet/util/diff.rb
</pre>
In commit 2ea85ef38212ca9ebb79a6e1d57b54a759fbb76e, released in 3.0, the
semantics for `execute` changed unexpectedly. The new semantics are that
failonfail and combine always default to true. Although this is how people
expect the `execute` method to behave, it is a breaking change. For example,
many providers pass in options to the execute method (combine, squelch,
stdinfile) and expect to look at the exit code to determine if the command
succeeded. The change in 3.0.x breaks these providers since they are not
expecting an ExecutionException to be raised. This is what happened in #17007.
We need to fix the semantics around `execute` by creating a new method, so that
providers can opt-in to the new behavior. The new behavior should be the way
3.0 was trying to do, failonfail and combine are always true by default. The
new method should also provide a way for the caller to determine the child
process's exit status without using $CHILD_STATUS.
--
You have received this notification because you have either subscribed to it,
or are involved in it.
To change your notification preferences, please click here:
http://projects.puppetlabs.com/my/account
--
You received this message because you are subscribed to the Google Groups
"Puppet Bugs" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-bugs?hl=en.
