diff options
author | Pete Higgins <pete@peterhiggins.org> | 2020-07-21 13:07:03 -0700 |
---|---|---|
committer | Pete Higgins <pete@peterhiggins.org> | 2020-07-21 16:44:54 -0700 |
commit | 4c31f7cc35ed76a59c818eb66389d4454b6b20c9 (patch) | |
tree | 2aabe985e04c57c070115e11e10a23ca37e75a3f | |
parent | 6d1ce408e055a4904b9a875c47b0b2a0b21a977c (diff) | |
download | chef-fix-cookbook-uploader-global-state.tar.gz |
Remove global state from cookbook uploader.fix-cookbook-uploader-global-state
Signed-off-by: Pete Higgins <pete@peterhiggins.org>
-rw-r--r-- | lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb | 6 | ||||
-rw-r--r-- | lib/chef/cookbook_loader.rb | 44 | ||||
-rw-r--r-- | lib/chef/knife/cookbook_upload.rb | 7 | ||||
-rw-r--r-- | spec/unit/knife/cookbook_upload_spec.rb | 4 |
4 files changed, 18 insertions, 43 deletions
diff --git a/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb b/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb index 63be9c9f4b..e9f973ef6f 100644 --- a/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb +++ b/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb @@ -76,8 +76,7 @@ class Chef raise Chef::Exceptions::MetadataNotFound.new(cookbook.root_paths[0], cookbook.name) unless cookbook.has_metadata_file? if cookbook - begin - tmp_cl = Chef::CookbookLoader.copy_to_tmp_dir_from_array([cookbook]) + Chef::CookbookLoader.copy_to_tmp_dir_from_array([cookbook]) do |tmp_cl| tmp_cl.load_cookbooks tmp_cl.compile_metadata tmp_cl.freeze_versions if options[:freeze] @@ -91,9 +90,6 @@ class Chef with_actual_cookbooks_dir(other.parent.file_path) do uploader.upload_cookbooks end - - ensure - tmp_cl.unlink! end end end diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index e6e6a4805d..f7efb2646e 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -44,14 +44,11 @@ class Chef # @return [Array<String>] the array of repo paths containing cookbook dirs attr_reader :repo_paths - attr_accessor :tmp_working_dir_path - # XXX: this is highly questionable combined with the Hash-style each method include Enumerable # @param repo_paths [Array<String>] the array of repo paths containing cookbook dirs def initialize(*repo_paths) - @tmp_working_dir_path = nil @repo_paths = repo_paths.flatten.compact.map { |p| File.expand_path(p) } raise ArgumentError, "You must specify at least one cookbook repo path" if @repo_paths.empty? end @@ -140,27 +137,23 @@ class Chef # This method creates tmp directory and copies all cookbooks into it and creates cookbook loader object which points to tmp directory def self.copy_to_tmp_dir_from_array(cookbooks) - tmp_cookbook_file = Tempfile.new("tmp_working_dir_path") - tmp_cookbook_file.close - @tmp_working_dir_path = tmp_cookbook_file.path - File.unlink(@tmp_working_dir_path) - FileUtils.mkdir_p(@tmp_working_dir_path) - cookbooks.each do |cookbook| - checksums_to_on_disk_paths = cookbook.checksums - cookbook.each_file do |manifest_record| - path_in_cookbook = manifest_record[:path] - on_disk_path = checksums_to_on_disk_paths[manifest_record[:checksum]] - dest = File.join(@tmp_working_dir_path, cookbook.name.to_s, path_in_cookbook) - FileUtils.mkdir_p(File.dirname(dest)) - FileUtils.cp_r(on_disk_path, dest) + Dir.mktmpdir do |tmp_dir| + cookbooks.each do |cookbook| + checksums_to_on_disk_paths = cookbook.checksums + cookbook.each_file do |manifest_record| + path_in_cookbook = manifest_record[:path] + on_disk_path = checksums_to_on_disk_paths[manifest_record[:checksum]] + dest = File.join(tmp_dir, cookbook.name.to_s, path_in_cookbook) + FileUtils.mkdir_p(File.dirname(dest)) + FileUtils.cp_r(on_disk_path, dest) + end end + tmp_cookbook_loader ||= begin + Chef::Cookbook::FileVendor.fetch_from_disk(tmp_dir) + CookbookLoader.new(tmp_dir) + end + yield tmp_cookbook_loader end - tmp_cookbook_loader ||= begin - Chef::Cookbook::FileVendor.fetch_from_disk(@tmp_working_dir_path) - CookbookLoader.new(@tmp_working_dir_path) - end - tmp_cookbook_loader.tmp_working_dir_path = @tmp_working_dir_path - tmp_cookbook_loader end # generates metadata.json adds it in the manifest @@ -181,13 +174,6 @@ class Chef end end - # removes the tmp_dir_path - def unlink! - raise "Invalid directory path." if @tmp_working_dir_path.nil? - - FileUtils.rm_rf(@tmp_working_dir_path) - end - alias :cookbooks :values private diff --git a/lib/chef/knife/cookbook_upload.rb b/lib/chef/knife/cookbook_upload.rb index 5a46972b43..b87aea7e04 100644 --- a/lib/chef/knife/cookbook_upload.rb +++ b/lib/chef/knife/cookbook_upload.rb @@ -107,8 +107,7 @@ class Chef cookbook_path = config[:cookbook_path].respond_to?(:join) ? config[:cookbook_path].join(", ") : config[:cookbook_path] ui.warn("Could not find any cookbooks in your cookbook path: '#{File.expand_path(cookbook_path)}'. Use --cookbook-path to specify the desired path.") else - begin - tmp_cl = Chef::CookbookLoader.copy_to_tmp_dir_from_array(cookbooks) + Chef::CookbookLoader.copy_to_tmp_dir_from_array(cookbooks) do |tmp_cl| tmp_cl.load_cookbooks tmp_cl.compile_metadata tmp_cl.freeze_versions if config[:freeze] @@ -127,7 +126,6 @@ class Chef ui.error("Uploading of some of the cookbooks must be failed. Remove cookbook whose version is frozen from your cookbooks repo OR use --force option.") upload_failures += 1 rescue SystemExit => e - tmp_cl.unlink! raise exit e.status end ui.info("Uploaded all cookbooks.") if upload_failures == 0 @@ -146,7 +144,6 @@ class Chef ui.warn("Not updating version constraints for #{cookbook_name} in the environment as the cookbook is frozen.") upload_failures += 1 rescue SystemExit => e - tmp_cl.unlink! raise exit e.status end end @@ -164,8 +161,6 @@ class Chef unless version_constraints_to_update.empty? update_version_constraints(version_constraints_to_update) if config[:environment] end - ensure - tmp_cl.unlink! end end end diff --git a/spec/unit/knife/cookbook_upload_spec.rb b/spec/unit/knife/cookbook_upload_spec.rb index c667742187..dbed8b8a67 100644 --- a/spec/unit/knife/cookbook_upload_spec.rb +++ b/spec/unit/knife/cookbook_upload_spec.rb @@ -40,7 +40,6 @@ describe Chef::Knife::CookbookUpload do allow(cookbook_loader).to receive(:load_cookbooks).and_return(cookbook_loader) allow(cookbook_loader).to receive(:compile_metadata).and_return(nil) allow(cookbook_loader).to receive(:freeze_versions).and_return(nil) - allow(cookbook_loader).to receive(:unlink!).and_return(nil) cookbook_loader end @@ -60,7 +59,7 @@ describe Chef::Knife::CookbookUpload do before(:each) do allow(Chef::CookbookLoader).to receive(:new).and_return(cookbook_loader) - allow(Chef::CookbookLoader).to receive(:copy_to_tmp_dir_from_array).and_return(cookbook_loader) + allow(Chef::CookbookLoader).to receive(:copy_to_tmp_dir_from_array).and_yield(cookbook_loader) end describe "with --concurrency" do @@ -70,7 +69,6 @@ describe Chef::Knife::CookbookUpload do test_cookbook = Chef::CookbookVersion.new("test_cookbook", "/tmp/blah") allow(cookbook_loader).to receive(:each).and_yield("test_cookbook", test_cookbook) allow(cookbook_loader).to receive(:cookbook_names).and_return(["test_cookbook"]) - allow(cookbook_loader).to receive(:tmp_working_dir_path).and_return("/tmp/blah") expect(Chef::CookbookUploader).to receive(:new) .with( kind_of(Array), { force: nil, concurrency: 3 }) .and_return(double("Chef::CookbookUploader", upload_cookbooks: true)) |