Hi,
As an FYI to anyone that uses the varnish cookbook >= 2.0.0, an update was
just released (2.2.1) that fixes a bug in the service resource and is
probably worth updating to sooner rather than later.
One of the libraries in the varnish cookbook used a prepend statement to
altered the behaviour of the Provider::Service::Init and
Provider::Service::Systemd classes, resulting in service resource
declarations attempting starts/restarts 5 times and failing silently.
Because it was in a loaded library simply including the cookbook in a
dependency chain would cause this.
This was a bug (it was only supposed to affect services of a certain name)
but I think it’s a pretty clear demonstration of the dangers of using
prepend on core Chef libraries.
Perhaps some sort of a warning should be issue if a prepend is used, as the
user may be signing up for something they aren’t aware of.
–
Yoshi Spendiff
Ops Engineer
Indochino
Mobile: +1 778 952 2025
Email: yoshi.spendiff@indochino.com
Yeah you are monkey patching core chef resources. That is alway bad
practice, you aren't adhering to the public APIs of those classes and we
can make no guarantees that we won't break you in minor or patch
releases of core chef.
A slightly better approach would be subclassing, but in ruby there's no
private methods which are inaccessible to the subclass, so its still
dependent upon you to only use the public APIs (and we don't necessarily
do the best job, or sometimes any job, of documenting the public APIs
correctly and expect you to be able to read the code and infer what the
public APIs are). A better solution is to use composition and use the
resources as they were intended. There's retry logic around the core
resource class which you could be using instead of monkeypatching the
action in the resource. If that doesn't get you what you need, you
should really submit a PR to add a feature so that you have a public API
to use.
In general cookbook code should never, ever under any circumstances open
up core chef classes and hack them (although a few use cases like
including modules into Chef::Recipe are supported idioms, but if its not
one of those idioms it is probably bad). It is also likely that
subclassing core chef providers is the wrong approach as well (that can
be used, but I'd say it requires expert level understanding of the
inheritance-vs-composition tradeoff).
On 08/25/2015 11:11 AM, Yoshi Spendiff wrote:
Hi,
As an FYI to anyone that uses the varnish cookbook >= 2.0.0, an update
was just released (2.2.1) that fixes a bug in the service resource and
is probably worth updating to sooner rather than later.
One of the libraries in the varnish cookbook used a prepend statement
to altered the behaviour of the Provider::Service::Init and
Provider::Service::Systemd classes, resulting in service resource
declarations attempting starts/restarts 5 times and failing silently.
Because it was in a loaded library simply including the cookbook in a
dependency chain would cause this.
This was a bug (it was only supposed to affect services of a certain
name) but I think it's a pretty clear demonstration of the dangers of
using prepend on core Chef libraries.
Perhaps some sort of a warning should be issue if a prepend is used,
as the user may be signing up for something they aren't aware of.
--
Yoshi Spendiff
Ops Engineer
Indochino
Mobile: +1 778 952 2025
Email: yoshi.spendiff@indochino.com mailto:yoshi.spendiff@indochino.com