This optimization reduces the runtime by more then half on SELinux-enabled
systems in some informal performance testing I have been doing.
It would be an even bigger win for anyone managing files on a filesystem where
stat()s are particularly expensive (such as GFS).
puppetd --test run before patch:
real 1m56.197s
user 1m8.780s
sys 0m31.342s
puppetd --test run after patch:
real 0m58.404s
user 0m23.165s
sys 0m15.457s
Same host, same configs, no changes occurring on either run.
---
lib/puppet/util/selinux.rb | 59 +++++++++++++++++++++++++------------------
spec/unit/util/selinux.rb | 46 +++++++++++++++++++++++++++++----
2 files changed, 74 insertions(+), 31 deletions(-)
diff --git a/lib/puppet/util/selinux.rb b/lib/puppet/util/selinux.rb
index 0a4af3c..06f8dcb 100644
--- a/lib/puppet/util/selinux.rb
+++ b/lib/puppet/util/selinux.rb
@@ -8,6 +8,9 @@
module Puppet::Util::SELinux
+ @@selinux_actual_context = {}
+ @@selinux_default_context = {}
+
def selinux_support?
FileTest.exists?("/selinux/enforce")
end
@@ -18,23 +21,26 @@ module Puppet::Util::SELinux
unless selinux_support?
return nil
end
- context = ""
- begin
- execpipe("/usr/bin/stat -c %C #{file}") do |out|
- out.each do |line|
- context << line
+ unless @@selinux_actual_context.has_key? file
+ context = ""
+ begin
+ execpipe("/usr/bin/stat -c %C #{file}") do |out|
+ out.each do |line|
+ context << line
+ end
end
+ rescue Puppet::ExecutionFailure
+ return nil
end
- rescue Puppet::ExecutionFailure
- return nil
- end
- context.chomp!
- # Handle the case that the system seems to have SELinux support but
- # stat finds unlabled files.
- if context == "(null)"
- return nil
+ context.chomp!
+ # Handle the case that the system seems to have SELinux support but
+ # stat finds unlabled files.
+ if context == "(null)"
+ return nil
+ end
+ @@selinux_actual_context[file] = context
end
- return context
+ return @@selinux_actual_context[file]
end
# Use the matchpathcon command, if present, to return the SELinux context
@@ -51,20 +57,23 @@ module Puppet::Util::SELinux
unless FileTest.executable?("/usr/sbin/matchpathcon")
return nil
end
- context = ""
- begin
- execpipe("/usr/sbin/matchpathcon #{file}") do |out|
- out.each do |line|
- context << line
+ unless @@selinux_default_context.has_key? file
+ context = ""
+ begin
+ execpipe("/usr/sbin/matchpathcon #{file}") do |out|
+ out.each do |line|
+ context << line
+ end
end
+ rescue Puppet::ExecutionFailure
+ return nil
end
- rescue Puppet::ExecutionFailure
- return nil
+ # For a successful match, matchpathcon returns two fields
separated by
+ # a variable amount of whitespace. The second field is the full
context.
+ context = context.split(/\s/)[1]
+ @@selinux_default_context[file] = context
end
- # For a successful match, matchpathcon returns two fields separated by
- # a variable amount of whitespace. The second field is the full
context.
- context = context.split(/\s/)[1]
- return context
+ return @@selinux_default_context[file]
end
# Take the full SELinux context returned from the tools and parse it
diff --git a/spec/unit/util/selinux.rb b/spec/unit/util/selinux.rb
index 515c3a2..7b8e948 100644
--- a/spec/unit/util/selinux.rb
+++ b/spec/unit/util/selinux.rb
@@ -33,14 +33,30 @@ describe Puppet::Util::SELinux do
it "should return nil if an exception is raised calling stat" do
self.expects(:selinux_support?).returns true
- self.expects(:execpipe).with("/usr/bin/stat -c %C
/foo").raises(Puppet::ExecutionFailure, 'error')
- get_selinux_current_context("/foo").should be_nil
+ self.expects(:execpipe).with("/usr/bin/stat -c %C
/foo1").raises(Puppet::ExecutionFailure, 'error')
+ get_selinux_current_context("/foo1").should be_nil
end
it "should return nil if stat finds an unlabeled file" do
self.expects(:selinux_support?).returns true
- self.expects(:execpipe).with("/usr/bin/stat -c %C /foo").yields
["(null)\n"]
- get_selinux_current_context("/foo").should be_nil
+ self.expects(:execpipe).with("/usr/bin/stat -c %C /foo2").yields
["(null)\n"]
+ get_selinux_current_context("/foo2").should be_nil
+ end
+
+ it "should not call stat more then once per file" do
+ self.expects(:selinux_support?).times(2).returns true
+ self.expects(:execpipe).with("/usr/bin/stat -c %C /foo3").yields
["user_u:role_r:type_t:s0\n"]
+ get_selinux_current_context("/foo3").should ==
"user_u:role_r:type_t:s0"
+ get_selinux_current_context("/foo3").should ==
"user_u:role_r:type_t:s0"
+ end
+
+ it "should not return the previous cached results for a different
file" do
+ self.expects(:selinux_support?).times(3).returns true
+ self.expects(:execpipe).with("/usr/bin/stat -c %C /fooa").yields
["user_u:role_r:type_t:s0\n"]
+ self.expects(:execpipe).with("/usr/bin/stat -c %C /foob").yields
["newuser_u:newrole_r:newtype_t:s0\n"]
+ get_selinux_current_context("/fooa").should ==
"user_u:role_r:type_t:s0"
+ get_selinux_current_context("/foob").should ==
"newuser_u:newrole_r:newtype_t:s0"
+ get_selinux_current_context("/fooa").should ==
"user_u:role_r:type_t:s0"
end
end
@@ -66,8 +82,26 @@ describe Puppet::Util::SELinux do
it "should return nil if an exception is raised calling matchpathcon"
do
self.expects(:selinux_support?).returns true
FileTest.expects(:executable?).with("/usr/sbin/matchpathcon").returns true
- self.expects(:execpipe).with("/usr/sbin/matchpathcon
/foo").raises(Puppet::ExecutionFailure, 'error')
- get_selinux_default_context("/foo").should be_nil
+ self.expects(:execpipe).with("/usr/sbin/matchpathcon
/foo1").raises(Puppet::ExecutionFailure, 'error')
+ get_selinux_default_context("/foo1").should be_nil
+ end
+
+ it "should only call matchpathcon once for a given file" do
+ self.expects(:selinux_support?).times(2).returns true
+
FileTest.expects(:executable?).with("/usr/sbin/matchpathcon").times(2).returns
true
+ self.expects(:execpipe).with("/usr/sbin/matchpathcon
/foo2").yields ["/foo2\tuser_u:role_r:type_t:s0\n"]
+ get_selinux_default_context("/foo2").should ==
"user_u:role_r:type_t:s0"
+ get_selinux_default_context("/foo2").should ==
"user_u:role_r:type_t:s0"
+ end
+
+ it "should not return the previous file's cached results for a new
file" do
+ self.expects(:selinux_support?).times(3).returns true
+
FileTest.expects(:executable?).with("/usr/sbin/matchpathcon").times(3).returns
true
+ self.expects(:execpipe).with("/usr/sbin/matchpathcon
/foo3").yields ["/foo3\tuser_u:role_r:type_t:s0\n"]
+ self.expects(:execpipe).with("/usr/sbin/matchpathcon
/foo4").yields ["/foo4\tnewuser_u:newrole_r:newtype_t:s0\n"]
+ get_selinux_default_context("/foo3").should ==
"user_u:role_r:type_t:s0"
+ get_selinux_default_context("/foo4").should ==
"newuser_u:newrole_r:newtype_t:s0"
+ get_selinux_default_context("/foo3").should ==
"user_u:role_r:type_t:s0"
end
end
--
1.5.5.1
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" 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-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
