Skip to content

(#21069) refresh the cache of launchd plists on a miss #1914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adrienthebo merged 1 commit into from
Sep 27, 2013
Merged

(#21069) refresh the cache of launchd plists on a miss #1914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 35 additions & 23 deletions lib/puppet/provider/service/launchd.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,39 +112,51 @@ def self.return_globbed_list_of_file_paths(path)
array_of_files.compact
end

# Get a hash of all launchd plists, keyed by label. This value is cached, but
# the cache will be refreshed if refresh is true.
#
# @api private
def self.make_label_to_path_map(refresh=false)
return @label_to_path_map if @label_to_path_map and not refresh
@label_to_path_map = {}
launchd_paths.each do |path|
return_globbed_list_of_file_paths(path).each do |filepath|
job = read_plist(filepath)
next if job.nil?
if job.has_key?("Label")
@label_to_path_map[job["Label"]] = filepath
else
Puppet.warning("The #{filepath} plist does not contain a 'label' key; " +
"Puppet is skipping it")
next
end
end
end
@label_to_path_map
end

# Sets a class instance variable with a hash of all launchd plist files that
# are found on the system. The key of the hash is the job id and the value
# is the path to the file. If a label is passed, we return the job id and
# path for that specific job.
def self.jobsearch(label=nil)
@label_to_path_map ||= {}
if @label_to_path_map.empty?
launchd_paths.each do |path|
return_globbed_list_of_file_paths(path).each do |filepath|
job = read_plist(filepath)
next if job.nil?
if job.has_key?("Label")
@label_to_path_map[job["Label"]] = filepath
if job["Label"] == label
return { label => filepath }
end
else
Puppet.warning("The #{filepath} plist does not contain a 'label' key; " +
"Puppet is skipping it")
next
end
end
end
end
by_label = make_label_to_path_map

if label
if @label_to_path_map.has_key? label
return { label => @label_to_path_map[label] }
if by_label.has_key? label
return { label => by_label[label] }
else
raise Puppet::Error.new("Unable to find launchd plist for job: #{label}")
# try refreshing the map, in case a plist has been added in the interim
by_label = make_label_to_path_map(true)
if by_label.has_key? label
return { label => by_label[label] }
else
raise Puppet::Error, "Unable to find launchd plist for job: #{label}"
end
end
else
@label_to_path_map
# caller wants the whole map
by_label
end
end

Expand Down
109 changes: 71 additions & 38 deletions spec/unit/provider/service/launchd_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,49 +209,82 @@
end
end

describe "when encountering malformed plists" do
let(:plist_without_label) do
{
'LimitLoadToSessionType' => 'Aqua'
}
end
let(:busted_plist_path) { '/Library/LaunchAgents/org.busted.plist' }
describe "make_label_to_path_map" do
before do
# clear out this class variable between runs
if provider.instance_variable_defined? :@label_to_path_map
provider.send(:remove_instance_variable, :@label_to_path_map)
end
end
describe "when encountering malformed plists" do
let(:plist_without_label) do
{
'LimitLoadToSessionType' => 'Aqua'
}
end
let(:busted_plist_path) { '/Library/LaunchAgents/org.busted.plist' }

it "[17624] should warn that the plist in question is being skipped" do
provider.expects(:launchd_paths).returns(['/Library/LaunchAgents'])
provider.expects(:return_globbed_list_of_file_paths).with('/Library/LaunchAgents').returns([busted_plist_path])
provider.expects(:read_plist).with(busted_plist_path).returns(plist_without_label)
Puppet.expects(:warning).with("The #{busted_plist_path} plist does not contain a 'label' key; Puppet is skipping it")
provider.jobsearch
end
it "[17624] should warn that the plist in question is being skipped" do
provider.expects(:launchd_paths).returns(['/Library/LaunchAgents'])
provider.expects(:return_globbed_list_of_file_paths).with('/Library/LaunchAgents').returns([busted_plist_path])
provider.expects(:read_plist).with(busted_plist_path).returns(plist_without_label)
Puppet.expects(:warning).with("The #{busted_plist_path} plist does not contain a 'label' key; Puppet is skipping it")
provider.make_label_to_path_map
end

it "[15929] should skip plists that plutil cannot read" do
provider.expects(:plutil).with('-convert', 'xml1', '-o', '/dev/stdout',
busted_plist_path).raises(Puppet::ExecutionFailure, 'boom')
Puppet.expects(:warning).with("Cannot read file #{busted_plist_path}; " +
"Puppet is skipping it. \n" +
"Details: boom")
provider.read_plist(busted_plist_path)
it "[15929] should skip plists that plutil cannot read" do
provider.expects(:plutil).with('-convert', 'xml1', '-o', '/dev/stdout',
busted_plist_path).raises(Puppet::ExecutionFailure, 'boom')
Puppet.expects(:warning).with("Cannot read file #{busted_plist_path}; " +
"Puppet is skipping it. \n" +
"Details: boom")
provider.read_plist(busted_plist_path)
end
end
it "should return the cached value when available" do
provider.instance_variable_set(:@label_to_path_map, {'xx'=>'yy'})
provider.make_label_to_path_map.should eq({'xx'=>'yy'})
end
describe "when successful" do
let(:launchd_dir) { '/Library/LaunchAgents' }
let(:plist) { launchd_dir + '/foo.bar.service.plist' }
let(:label) { 'foo.bar.service' }
before do
provider.instance_variable_set(:@label_to_path_map, nil)
provider.expects(:launchd_paths).returns([launchd_dir])
provider.expects(:return_globbed_list_of_file_paths).with(launchd_dir).returns([plist])
provider.expects(:read_plist).with(plist).returns({'Label'=>'foo.bar.service'})
end
it "should read the plists and return their contents" do
provider.make_label_to_path_map.should eq({label=>plist})
end
it "should re-read the plists and return their contents when refreshed" do
provider.instance_variable_set(:@label_to_path_map, {'xx'=>'yy'})
provider.make_label_to_path_map(true).should eq({label=>plist})
end
end
end

describe "when searching for jobs" do
it "should store the launchd job in the label_to_map" do
provider.instance_variable_set(:@label_to_path_map, nil)
provider.expects(:launchd_paths).returns(['/Library/LaunchAgents'])
provider.expects(:return_globbed_list_of_file_paths).with('/Library/LaunchAgents').returns(['/Library/LaunchAgents/foo.bar.service.plist'])
provider.expects(:read_plist).with('/Library/LaunchAgents/foo.bar.service.plist').returns({'Label'=>'foo.bar.service'})
provider.jobsearch()
provider.instance_variable_get(:@label_to_path_map).should eq({"foo.bar.service"=>"/Library/LaunchAgents/foo.bar.service.plist"})
end

it "should store the launchd job in the label_to_map even if the specified job is returned" do
provider.instance_variable_set(:@label_to_path_map, nil)
provider.expects(:launchd_paths).returns(['/Library/LaunchAgents'])
provider.expects(:return_globbed_list_of_file_paths).with('/Library/LaunchAgents').returns(['/Library/LaunchAgents/foo.bar.service.plist'])
provider.expects(:read_plist).with('/Library/LaunchAgents/foo.bar.service.plist').returns({'Label'=>'foo.bar.service'})
provider.jobsearch('foo.bar.service').should eq({"foo.bar.service"=>"/Library/LaunchAgents/foo.bar.service.plist"})
provider.instance_variable_get(:@label_to_path_map).should eq({"foo.bar.service"=>"/Library/LaunchAgents/foo.bar.service.plist"})
describe "jobsearch" do
let(:map) { {"org.mozilla.puppet" => "/path/to/puppet.plist",
"org.mozilla.python" => "/path/to/python.plist"} }
it "returns the entire map with no args" do
provider.expects(:make_label_to_path_map).returns(map)
provider.jobsearch.should == map
end
it "returns a singleton hash when given a label" do
provider.expects(:make_label_to_path_map).returns(map)
provider.jobsearch("org.mozilla.puppet").should == { "org.mozilla.puppet" => "/path/to/puppet.plist" }
end
it "refreshes the label_to_path_map when label is not found" do
provider.expects(:make_label_to_path_map).with().returns({})
provider.expects(:make_label_to_path_map).with(true).returns(map)
provider.jobsearch("org.mozilla.puppet").should == { "org.mozilla.puppet" => "/path/to/puppet.plist" }
end
it "raises Puppet::Error when the label is still not found" do
provider.expects(:make_label_to_path_map).with().returns(map)
provider.expects(:make_label_to_path_map).with(true).returns(map)
expect { provider.jobsearch("NOSUCH") }.to raise_error(Puppet::Error)
end
end
end