Only run cleanup tasks when they are defined on the provisioner
This helps with some confusion caused in GH-2538, since the output says: > Running cleanup tasks for 'shell' provisioner... But that's actually not true. It is running the cleanup tasks iff the provisioner defined a cleanup task. This commit changes the provisioner_cleanup middleware to only run cleanup tasks if the subclass defines a cleanup task. The reason we can't just check if the provisioner `respond_to?` the `cleanup` method is because the parent provisioner base class (which all provisioners inherit from) defines a blank cleanup method. This is important because it means we never risk calling an unimplemented cleanup function, and it also helps define the public API for a provisioner.
This commit is contained in:
parent
b63fdf756f
commit
64ff69c64b
|
@ -32,10 +32,20 @@ module Vagrant
|
||||||
|
|
||||||
# Ask the provisioners to modify the configuration if needed
|
# Ask the provisioners to modify the configuration if needed
|
||||||
provisioner_instances(env).each do |p, _|
|
provisioner_instances(env).each do |p, _|
|
||||||
env[:ui].info(I18n.t(
|
name = type_map[p].to_s
|
||||||
"vagrant.provisioner_cleanup",
|
|
||||||
name: type_map[p].to_s))
|
# Check if the subclass defined a cleanup method. The parent
|
||||||
|
# provisioning class defines a `cleanup` method, so we cannot use
|
||||||
|
# `respond_to?` here. Instead, we have to check if _this_ instance
|
||||||
|
# defines a cleanup task.
|
||||||
|
if p.public_methods(false).include?(:cleanup)
|
||||||
|
env[:ui].info(I18n.t("vagrant.provisioner_cleanup",
|
||||||
|
name: name,
|
||||||
|
))
|
||||||
p.cleanup
|
p.cleanup
|
||||||
|
else
|
||||||
|
@logger.debug("Skipping cleanup tasks for `#{name}' - not defined")
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -55,4 +55,30 @@ describe Vagrant::Action::Builtin::ProvisionerCleanup do
|
||||||
instance.call(env)
|
instance.call(env)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "only runs cleanup tasks if the subclass defines it" do
|
||||||
|
parent = Class.new do
|
||||||
|
class_variable_set(:@@cleanup, false)
|
||||||
|
|
||||||
|
def self.called?
|
||||||
|
class_variable_get(:@@cleanup)
|
||||||
|
end
|
||||||
|
|
||||||
|
def cleanup
|
||||||
|
self.class.class_variable_set(:@@cleanup)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
child = Class.new(parent)
|
||||||
|
|
||||||
|
allow_any_instance_of(described_class).to receive(:provisioner_type_map)
|
||||||
|
.and_return(child => :test_provisioner)
|
||||||
|
allow_any_instance_of(described_class).to receive(:provisioner_instances)
|
||||||
|
.and_return([child])
|
||||||
|
|
||||||
|
expect(parent.called?).to be(false)
|
||||||
|
instance = described_class.new(app, env)
|
||||||
|
instance.call(env)
|
||||||
|
expect(parent.called?).to be(false)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue