From 729d62d1eaba17e4e6c4950de8d2aee623ccc70b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 7 Jul 2011 21:15:41 -0700 Subject: [PATCH] Chef solo no longer overwrites share folders when multiple provisioners exist --- lib/vagrant/provisioners/chef_solo.rb | 135 +++++--------- lib/vagrant/util.rb | 1 + lib/vagrant/util/counter.rb | 20 ++ test/vagrant/provisioners/chef_solo_test.rb | 191 ++------------------ test/vagrant/util/counter_test.rb | 29 +++ 5 files changed, 113 insertions(+), 263 deletions(-) create mode 100644 lib/vagrant/util/counter.rb create mode 100644 test/vagrant/util/counter_test.rb diff --git a/lib/vagrant/provisioners/chef_solo.rb b/lib/vagrant/provisioners/chef_solo.rb index 74a3be9aa..7e7d0c4bb 100644 --- a/lib/vagrant/provisioners/chef_solo.rb +++ b/lib/vagrant/provisioners/chef_solo.rb @@ -4,6 +4,8 @@ module Vagrant class ChefSolo < Chef register :chef_solo + extend Util::Counter + class Config < Chef::Config attr_accessor :cookbooks_path attr_accessor :roles_path @@ -28,10 +30,18 @@ module Vagrant end end + attr_reader :cookbook_folders + attr_reader :role_folders + attr_reader :data_bags_folders + def prepare - share_cookbook_folders - share_role_folders - share_data_bags_folders + @cookbook_folders = expanded_folders(config.cookbooks_path) + @role_folders = expanded_folders(config.roles_path) + @data_bags_folders = expanded_folders(config.data_bags_path) + + share_folders("csc", @cookbook_folders) + share_folders("csr", @role_folders) + share_folders("csdb", @data_bags_folders) end def provision! @@ -42,25 +52,43 @@ module Vagrant run_chef_solo end - def share_cookbook_folders - host_cookbook_paths.each_with_index do |cookbook, i| - env.config.vm.share_folder("v-csc-#{i}", cookbook_path(i), cookbook, :nfs => config.nfs) + # Converts paths to a list of properly expanded paths with types. + def expanded_folders(paths) + # Convert the path to an array if it is a string or just a single + # path element which contains the folder location (:host or :vm) + paths = [paths] if paths.is_a?(String) || paths.first.is_a?(Symbol) + + paths.map do |path| + path = [:host, path] if !path.is_a?(Array) + type, path = path + + # Create the local/remote path based on whether this is a host + # or VM path. + local_path = nil + local_path = File.expand_path(path, env.root_path) if type == :host + remote_path = type == :host ? "#{config.provisioning_path}/chef-solo-#{self.class.get_and_update_counter}" : path + + # Return the result + [type, local_path, remote_path] end end - def share_role_folders - host_role_paths.each_with_index do |role, i| - env.config.vm.share_folder("v-csr-#{i}", role_path(i), role, :nfs => config.nfs) - end - end - - def share_data_bags_folders - host_data_bag_paths.each_with_index do |data_bag, i| - env.config.vm.share_folder("v-csdb-#{i}", data_bag_path(i), data_bag, :nfs => config.nfs) + # Shares the given folders with the given prefix. The folders should + # be of the structure resulting from the `expanded_folders` function. + def share_folders(prefix, folders) + folders.each do |type, local_path, remote_path| + if type == :host + env.config.vm.share_folder("v-#{prefix}-#{self.class.get_and_update_counter}", + remote_path, local_path, :nfs => config.nfs) + end end end def setup_solo_config + cookbooks_path = guest_paths(@cookbook_folders) + roles_path = guest_paths(@role_folders) + data_bags_path = guest_paths(@data_bags_folders) + setup_config("chef_solo_solo", "solo.rb", { :node_name => config.node_name, :provisioning_path => config.provisioning_path, @@ -87,80 +115,9 @@ module Vagrant end end - def host_folder_paths(paths) - # Convert single cookbook paths such as "cookbooks" or [:vm, "cookbooks"] - # into a proper array representation. - paths = [paths] if paths.is_a?(String) || paths.first.is_a?(Symbol) - - paths.inject([]) do |acc, path| - path = [:host, path] if !path.is_a?(Array) - type, path = path - - acc << File.expand_path(path, env.root_path) if type == :host - acc - end - end - - def folder_path(*args) - File.join(config.provisioning_path, args.join("-")) - end - - def folders_path(folders, folder) - # Convert single cookbook paths such as "cookbooks" or [:vm, "cookbooks"] - # into a proper array representation. - folders = [folders] if folders.is_a?(String) || folders.first.is_a?(Symbol) - - # Convert each path to the proper absolute path depending on if the path - # is a host path or a VM path - result = [] - folders.each_with_index do |path, i| - path = [:host, path] if !path.is_a?(Array) - type, path = path - - result << folder_path(folder, i) if type == :host - result << folder_path(path) if type == :vm - end - - # We're lucky that ruby's string and array syntax for strings is the - # same as JSON, so we can just convert to JSON here and use that - result = result[0].to_s if result.length == 1 - result - end - - def host_cookbook_paths - host_folder_paths(config.cookbooks_path) - end - - def host_role_paths - host_folder_paths(config.roles_path) - end - - def host_data_bag_paths - host_folder_paths(config.data_bags_path) - end - - def cookbook_path(i) - folder_path("cookbooks", i) - end - - def role_path(i) - folder_path("roles", i) - end - - def data_bag_path(i) - folder_path("data_bags", i) - end - - def cookbooks_path - folders_path(config.cookbooks_path, "cookbooks").to_json - end - - def roles_path - folders_path(config.roles_path, "roles").to_json - end - - def data_bags_path - folders_path(config.data_bags_path, "data_bags").to_json + # Extracts only the remote paths from a list of folders + def guest_paths(folders) + folders.map { |parts| parts[2] } end end end diff --git a/lib/vagrant/util.rb b/lib/vagrant/util.rb index bcfb27841..5583759fe 100644 --- a/lib/vagrant/util.rb +++ b/lib/vagrant/util.rb @@ -1,6 +1,7 @@ module Vagrant module Util autoload :Busy, 'vagrant/util/busy' + autoload :Counter, 'vagrant/util/counter' autoload :GlobLoader, 'vagrant/util/glob_loader' autoload :HashWithIndifferentAccess, 'vagrant/util/hash_with_indifferent_access' autoload :PlainLogger, 'vagrant/util/plain_logger' diff --git a/lib/vagrant/util/counter.rb b/lib/vagrant/util/counter.rb new file mode 100644 index 000000000..75d1fde84 --- /dev/null +++ b/lib/vagrant/util/counter.rb @@ -0,0 +1,20 @@ +module Vagrant + module Util + # Atomic counter implementation. This is useful for incrementing + # a counter which is guaranteed to only be used once in its class. + module Counter + def get_and_update_counter + mutex.synchronize do + @__counter ||= 1 + result = @__counter + @__counter += 1 + result + end + end + + def mutex + @__counter_mutex ||= Mutex.new + end + end + end +end diff --git a/test/vagrant/provisioners/chef_solo_test.rb b/test/vagrant/provisioners/chef_solo_test.rb index e07e8764c..66fa85980 100644 --- a/test/vagrant/provisioners/chef_solo_test.rb +++ b/test/vagrant/provisioners/chef_solo_test.rb @@ -32,23 +32,6 @@ class ChefSoloProvisionerTest < Test::Unit::TestCase end end - context "preparing" do - should "share cookbook folders" do - @action.expects(:share_cookbook_folders).once - @action.prepare - end - - should "share role folders" do - @action.expects(:share_role_folders).once - @action.prepare - end - - should "share data bag folders" do - @action.expects(:share_data_bags_folders).once - @action.prepare - end - end - context "provisioning" do should "run the proper sequence of methods in order" do prov_seq = sequence("prov_seq") @@ -60,165 +43,24 @@ class ChefSoloProvisionerTest < Test::Unit::TestCase @action.provision! end end - context "sharing cookbook folders" do - setup do - @host_cookbook_paths = ["foo", "bar"] - @action.stubs(:host_cookbook_paths).returns(@host_cookbook_paths) + + context "creating expanded folder sets" do + should "expand VM folders properly" do + assert_equal [[:vm, nil, "/foo"]], @action.expanded_folders([:vm, "/foo"]) end - should "share each cookbook folder" do - share_seq = sequence("share_seq") - @host_cookbook_paths.each_with_index do |cookbook, i| - @env.config.vm.expects(:share_folder).with("v-csc-#{i}", @action.cookbook_path(i), cookbook, :nfs => false).in_sequence(share_seq) - end - - @action.share_cookbook_folders + should "expand host folders properly" do + path = "foo" + local_path = File.expand_path(path, @env.root_path) + remote_path = "#{@action.config.provisioning_path}/chef-solo-1" + assert_equal [[:host, local_path, remote_path]], @action.expanded_folders([:host, path]) end end - context "sharing role folders" do - setup do - @host_role_paths = ["foo", "bar"] - @action.stubs(:host_role_paths).returns(@host_role_paths) - end - - should "share each role folder" do - share_seq = sequence("share_seq") - @host_role_paths.each_with_index do |role, i| - @env.config.vm.expects(:share_folder).with("v-csr-#{i}", @action.role_path(i), role, :nfs => false).in_sequence(share_seq) - end - - @action.share_role_folders - end - end - - context "sharing data bag folders" do - setup do - @host_data_bag_paths = ["foo", "bar"] - @action.stubs(:host_data_bag_paths).returns(@host_data_bag_paths) - end - - should "share each data bag folder" do - share_seq = sequence("share_seq") - @host_data_bag_paths.each_with_index do |data_bag, i| - @env.config.vm.expects(:share_folder).with("v-csdb-#{i}", @action.data_bag_path(i), data_bag, :nfs => false).in_sequence(share_seq) - end - - @action.share_data_bags_folders - end - end - - context "host folder paths" do - should "ignore VM paths" do - assert @action.host_folder_paths([:vm, "foo"]).empty? - end - - should "return as an array if was originally a string" do - folder = "foo" - File.stubs(:expand_path).returns("bar") - assert_equal ["bar"], @action.host_folder_paths(folder) - end - - should "return the array of folders if its an array" do - folders = ["foo", "bar"] - expand_seq = sequence('expand_seq') - folders.collect! { |folder| File.expand_path(folder, @env.root_path) } - - assert_equal folders, @action.host_folder_paths(folders) - end - end - - context "host cookbooks paths" do - should "get folders path for configured cookbooks path" do - result = mock("result") - @config.stubs(:cookbooks_path).returns("foo") - @action.expects(:host_folder_paths).with(@config.cookbooks_path).returns(result) - assert_equal result, @action.host_cookbook_paths - end - end - - context "host roles paths" do - should "get folders path for configured roles path" do - result = mock("result") - @config.stubs(:roles_path).returns("foo") - @action.expects(:host_folder_paths).with(@config.roles_path).returns(result) - assert_equal result, @action.host_role_paths - end - end - - context "host data bags paths" do - should "get folders path for configured data bag path" do - result = mock("result") - @config.stubs(:data_bags_path).returns("foo") - @action.expects(:host_folder_paths).with(@config.data_bags_path).returns(result) - assert_equal result, @action.host_data_bag_paths - end - end - - context "folder path" do - should "return a proper path to a single folder" do - expected = File.join(@config.provisioning_path, "cookbooks-5") - assert_equal expected, @action.folder_path("cookbooks", 5) - end - - should "return array-representation of folder paths if multiple" do - @folders = (0..5).to_a - @cookbooks = @folders.inject([]) do |acc, i| - acc << @action.cookbook_path(i) - end - - assert_equal @cookbooks, @action.folders_path(@folders, "cookbooks") - end - - should "return a single string representation if folder paths is single" do - @folder = "cookbooks" - @cookbooks = @action.folder_path(@folder, 0) - - assert_equal @cookbooks, @action.folders_path([0], @folder) - end - - should "properly format VM folder paths" do - @config.provisioning_path = "/foo" - assert_equal "/foo/bar", @action.folders_path([:vm, "bar"], nil) - end - end - - context "cookbooks path" do - should "return a proper path to a single cookbook" do - expected = File.join(@config.provisioning_path, "cookbooks-5") - assert_equal expected, @action.cookbook_path(5) - end - - should "properly call folders path and return result" do - result = [:a, :b, :c] - @action.expects(:folders_path).with(@config.cookbooks_path, "cookbooks").once.returns(result) - assert_equal result.to_json, @action.cookbooks_path - end - end - - context "roles path" do - should "return a proper path to a single role" do - expected = File.join(@config.provisioning_path, "roles-5") - assert_equal expected, @action.role_path(5) - end - - should "properly call folders path and return result" do - result = [:a, :b, :c] - @action.expects(:folders_path).with(@config.roles_path, "roles").once.returns(result) - assert_equal result.to_json, @action.roles_path - end - end - - context "data bags path" do - should "return a proper path to a single data bag" do - expected = File.join(@config.provisioning_path, "data_bags-5") - assert_equal expected, @action.data_bag_path(5) - end - - should "properly call folders path and return result" do - result = [:a, :b, :c] - @action.expects(:folders_path).with(@config.data_bags_path, "data_bags").once.returns(result) - assert_equal result.to_json, @action.data_bags_path + context "guest paths" do + should "extract the parts properly" do + structure = [[1,2,3],[1,2,3]] + assert_equal [3,3], @action.guest_paths(structure) end end @@ -227,16 +69,17 @@ class ChefSoloProvisionerTest < Test::Unit::TestCase @vm.ssh.stubs(:upload!) @config.recipe_url = "foo/bar/baz" + @action.prepare end should "call setup_config with proper variables" do @action.expects(:setup_config).with("chef_solo_solo", "solo.rb", { :node_name => @config.node_name, :provisioning_path => @config.provisioning_path, - :cookbooks_path => @action.cookbooks_path, + :cookbooks_path => @action.guest_paths(@action.cookbook_folders), :recipe_url => @config.recipe_url, - :roles_path => @action.roles_path, - :data_bags_path => @action.data_bags_path + :roles_path => @action.guest_paths(@action.role_folders), + :data_bags_path => @action.guest_paths(@action.data_bags_folders) }) @action.setup_solo_config diff --git a/test/vagrant/util/counter_test.rb b/test/vagrant/util/counter_test.rb new file mode 100644 index 000000000..10f9f55c8 --- /dev/null +++ b/test/vagrant/util/counter_test.rb @@ -0,0 +1,29 @@ +require "test_helper" + +class CounterUtilTest < Test::Unit::TestCase + setup do + @klass = Class.new do + extend Vagrant::Util::Counter + end + end + + context "basic counter" do + should "get and update the counter" do + assert_equal 1, @klass.get_and_update_counter + assert_equal 2, @klass.get_and_update_counter + end + end + + context "multiple classes with a counter" do + setup do + @klass2 = Class.new do + extend Vagrant::Util::Counter + end + end + + should "not affect other classes" do + assert_equal 1, @klass.get_and_update_counter + assert_equal 1, @klass2.get_and_update_counter + end + end +end