[PATCH/puppet 1/1] Fixing #1107 - support for tag exclusions
Jesse Hallett
of tags on a resource. Tags prefixed with '!' will be treated as
negated tags.
Also extended the same method to check multiple tags at once via an
Array argument or multiple arguments.
Includes a small fix in Puppet::Transaction for cases where no required
tags are supplied to the transaction.
Signed-off-by: Jesse Hallett <hall...@gmail.com>
---
lib/puppet/transaction.rb | 2 +-
lib/puppet/util/tagging.rb | 16 +++++++-
spec/integration/transaction.rb | 44 +++++++++++++++++++++
spec/unit/transaction.rb | 25 ++++++++++--
spec/unit/util/tagging.rb | 81 +++++++++++++++++++++++++++++++++++++++
5 files changed, 160 insertions(+), 8 deletions(-)
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index c0b4c0e..b928bdf 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -618,7 +618,7 @@ class Transaction
# Is this resource tagged appropriately?
def missing_tags?(resource)
return false if self.ignore_tags? or tags.empty?
- return true unless resource.tagged?(tags)
+ return !resource.tagged?(tags)
end
# Are there any edges that target this resource?
diff --git a/lib/puppet/util/tagging.rb b/lib/puppet/util/tagging.rb
index f421d18..4d9a9a2 100644
--- a/lib/puppet/util/tagging.rb
+++ b/lib/puppet/util/tagging.rb
@@ -21,8 +21,10 @@ module Puppet::Util::Tagging
end
# Are we tagged with the provided tag?
- def tagged?(tag)
- defined?(@tags) and @tags.include?(tag.to_s)
+ def tagged?(*tags)
+ tags.flatten.reduce(true) do |decision, tag|
+ decision && tagged_with_single?(tag)
+ end
end
# Return a copy of the tag list, so someone can't ask for our tags
@@ -37,4 +39,14 @@ module Puppet::Util::Tagging
def valid_tag?(tag)
tag =~ /^\w[-\w:.]*$/
end
+
+ def tagged_with_single?(tag)
+ tag = tag.to_s
+ neg = !!(tag =~ /^!/)
+ real_tag = tag.sub(/^!/, '')
+
+ return neg unless defined?(@tags)
+ return !neg if @tags.include?(real_tag)
+ return neg
+ end
end
diff --git a/spec/integration/transaction.rb b/spec/integration/transaction.rb
index 2b8a5d9..0760663 100755
--- a/spec/integration/transaction.rb
+++ b/spec/integration/transaction.rb
@@ -36,3 +36,47 @@ describe Puppet::Transaction do
transaction.evaluate
end
end
+
+describe Puppet::Transaction, "when skipping a resource" do
+
+ before :each do
+ Puppet[:name] = "puppet"
+
+ @catalog = Puppet::Resource::Catalog.new
+ @resource = Puppet::Type.type(:file).new :path => "/foo/bar", :backup => false
+ @transaction = Puppet::Transaction.new(@catalog)
+ end
+
+ it "should process resources tagged with the given tags" do
+ Puppet[:tags] = "foo, bar"
+ @resource.tag "foo"
+ @resource.tag "bar"
+ @transaction.should_not be_a_skip(@resource)
+ end
+
+ it "should not process resources that are not tagged with the given tags" do
+ Puppet[:tags] = "foo, bar"
+ @resource.tag "bar"
+ @transaction.should be_a_skip(@resource)
+ end
+
+ it "should not process resources tagged with the given negated tags" do
+ Puppet[:tags] = "!foo, !bar"
+ @resource.tag "bar"
+ @transaction.should be_a_skip(@resource)
+ end
+
+ it "should process resources not tagged with the given negated tags" do
+ Puppet[:tags] = "!foo, !bar"
+ @transaction.should_not be_a_skip(@resource)
+ end
+
+ it "should give negated tags precedence when given both tags and negated tags" do
+ Puppet[:tags] = "foo, !bar, baz"
+ @resource.tag "foo"
+ @resource.tag "bar"
+ @resource.tag "baz"
+ @transaction.should be_a_skip(@resource)
+ end
+
+end
diff --git a/spec/unit/transaction.rb b/spec/unit/transaction.rb
index 0e36747..2b4099a 100755
--- a/spec/unit/transaction.rb
+++ b/spec/unit/transaction.rb
@@ -55,14 +55,29 @@ describe Puppet::Transaction do
describe "when skipping a resource" do
before :each do
- @resource = stub_everything 'res'
+ @resource = Puppet::Type.type(:file).new(:path => '/foo')
@catalog = Puppet::Resource::Catalog.new
@transaction = Puppet::Transaction.new(@catalog)
+
+ @transaction.stubs(:ignore_tags?).returns(false)
+ @resource.stubs(:scheduled?).returns(true)
end
- it "should skip resource with missing tags" do
- @transaction.stubs(:missing_tags?).returns(true)
- @transaction.skip?(@resource).should be_true
+ it "should not skip resource with all required tags" do
+ @transaction.stubs(:tags).returns(["foo", "bar"])
+ @resource.expects(:tagged?).with(["foo", "bar"]).returns(true)
+ @transaction.should_not be_a_skip(@resource)
+ end
+
+ it "should skip resources that are not tagged with the required tags" do
+ @transaction.stubs(:tags).returns(["foo", "bar"])
+ @resource.expects(:tagged?).with(["foo", "bar"]).returns(false)
+ @transaction.should be_a_skip(@resource)
+ end
+
+ it "should not filter resources by tag if no tags are given" do
+ @transaction.stubs(:tags).returns([])
+ @transaction.should_not be_a_skip(@resource)
end
it "should skip not scheduled resources" do
@@ -71,7 +86,7 @@ describe Puppet::Transaction do
end
it "should skip resources with failed dependencies" do
- @transaction.stubs(:failed_dependencies?).returns(false)
+ @transaction.stubs(:failed_dependencies?).returns(true)
@transaction.skip?(@resource).should be_true
end
diff --git a/spec/unit/util/tagging.rb b/spec/unit/util/tagging.rb
index d61ee8c..bf4f58b 100755
--- a/spec/unit/util/tagging.rb
+++ b/spec/unit/util/tagging.rb
@@ -80,6 +80,13 @@ describe Puppet::Util::Tagging, "when adding tags" do
@tagger.tags.should be_include("two")
@tagger.tags.should be_include("three")
end
+end
+
+describe Puppet::Util::Tagging, "when querying tags" do
+ before do
+ @tagger = Object.new
+ @tagger.extend(Puppet::Util::Tagging)
+ end
it "should indicate when the object is tagged with a provided tag" do
@tagger.tag("one")
@@ -89,4 +96,78 @@ describe Puppet::Util::Tagging, "when adding tags" do
it "should indicate when the object is not tagged with a provided tag" do
@tagger.should_not be_tagged("one")
end
+
+ it "should accept an array of tags or multiple tags as separate parameters" do
+ @tagger.tag("one::two")
+ @tagger.tagged?(%w/one two/).should == @tagger.tagged?('one', 'two')
+ end
+
+ describe "when some tags are present" do
+
+ before :each do
+ @tagger.tag("one")
+ end
+
+ it "should indicate when the object is not tagged with the negation of a tag" do
+ @tagger.should_not be_tagged("!one")
+ end
+
+ it "should consider an object that is not tagged with a given tag to be tagged with that tag's negation" do
+ @tagger.should be_tagged("!foo")
+ end
+
+ it "should be true when tagged with all of the provided tags" do
+ @tagger.tag("two")
+ @tagger.should be_tagged(%w/one two/)
+ end
+
+ it "should be false when not tagged with all of the provided tags" do
+ @tagger.should_not be_tagged(%w/one two/)
+ end
+
+ it "should be true when not tagged with any of the given negated tags" do
+ @tagger.should be_tagged(%w/!two !three/)
+ end
+
+ it "should be false when tagged with some of the given negated tags" do
+ @tagger.should_not be_tagged(%w/!one !two/)
+ end
+
+ it "should be false when tagged with a given tag and a given negated tag" do
+ @tagger.tag("two")
+ @tagger.should_not be_tagged(%w/one !two/)
+ @tagger.should_not be_tagged(%w/!two one/)
+ end
+
+ it "should be false when given a tag and its negation" do
+ @tagger.should_not be_tagged(%w/one !one/)
+ @tagger.should_not be_tagged(%w/!one one/)
+ end
+
+ end
+
+ describe "when no tags are defined" do
+
+ it "should be false when not tagged with the provided tag" do
+ @tagger.should_not be_tagged("foo")
+ end
+
+ it "should consider an object that is not tagged with a given tag to be tagged with that tag's negation" do
+ @tagger.should be_tagged("!foo")
+ end
+
+ it "should be false when not tagged with all of the provided tags" do
+ @tagger.should_not be_tagged(%w/one two/)
+ end
+
+ it "should be true when not tagged with any of the given negated tags" do
+ @tagger.should be_tagged(%w/!two !three/)
+ end
+
+ it "should be false when given a tag and its negation" do
+ @tagger.should_not be_tagged(%w/one !one/)
+ @tagger.should_not be_tagged(%w/!one one/)
+ end
+
+ end
end
--
1.6.3.2
Luke Kanies
a simple commit, so it's fine to leave it as one.
I've got one comment below.
resource, or even to make sure File.open was never called on the
actual file, since that would make it a bit more of an integration test.
As we saw when doing this coding, the 'skip?' method is relatively
complicated, and it's essentially an internal method, so I'd feel
better testing the result -- we either do or don't process that
resource -- rather than the result of this method, at least in our
integration test.
I could be convinced otherwise, of course.
To get back my youth I would do anything in the world, except take
exercise, get up early, or be respectable. -- Oscar Wilde
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com