diff --git a/lib/r10k/action/deploy/environment.rb b/lib/r10k/action/deploy/environment.rb index 2e68befbe..075774acc 100644 --- a/lib/r10k/action/deploy/environment.rb +++ b/lib/r10k/action/deploy/environment.rb @@ -135,6 +135,7 @@ def visit_deployment(deployment) envs.reject! { |e| !requested_envs.include?(e) } if requested_envs.any? postcmd = postcmd.map { |e| e.gsub('$modifiedenvs', envs.join(' ')) } end + logger.debug _("Executing postrun command.") subproc = R10K::Util::Subprocess.new(postcmd) subproc.logger = logger subproc.execute diff --git a/lib/r10k/action/deploy/module.rb b/lib/r10k/action/deploy/module.rb index 1c1ffd0cf..0631b9a18 100644 --- a/lib/r10k/action/deploy/module.rb +++ b/lib/r10k/action/deploy/module.rb @@ -78,9 +78,9 @@ def visit_deployment(deployment) ensure if (postcmd = @settings[:postrun]) if @modified_envs.any? + envs_to_run = @modified_envs.join(' ') + logger.debug "Running postrun command for environments #{envs_to_run}." if postcmd.grep('$modifiedenvs').any? - envs_to_run = @modified_envs.join(' ') - logger.debug "Running postrun command for environments #{envs_to_run}." postcmd = postcmd.map { |e| e.gsub('$modifiedenvs', envs_to_run) } end subproc = R10K::Util::Subprocess.new(postcmd) @@ -103,11 +103,10 @@ def visit_environment(environment) else logger.debug1(_("Updating modules %{modules} in environment %{env_path}") % {modules: @settings.dig(:overrides, :modules, :requested_modules).inspect, env_path: environment.path}) - environment.deploy + updated_modules = environment.deploy - requested_mods = @settings.dig(:overrides, :modules, :requested_modules) || [] # We actually synced a module in this env - if !((environment.modules.map(&:name) & requested_mods).empty?) + if !(updated_modules.nil? || updated_modules.empty?) # Record modified environment for postrun command @modified_envs << environment.dirname diff --git a/lib/r10k/content_synchronizer.rb b/lib/r10k/content_synchronizer.rb index cb156af44..74052d055 100644 --- a/lib/r10k/content_synchronizer.rb +++ b/lib/r10k/content_synchronizer.rb @@ -8,24 +8,31 @@ def self.serial_accept(modules, visitor, loader) end def self.serial_sync(modules) + updated_modules = [] modules.each do |mod| - mod.sync + updated = mod.sync + updated_modules << mod.name if updated end + updated_modules end + # Returns a Queue of the names of modules actually updated def self.concurrent_accept(modules, visitor, loader, pool_size, logger) mods_queue = modules_visit_queue(modules, visitor, loader) sync_queue(mods_queue, pool_size, logger) end + # Returns a Queue of the names of modules actually updated def self.concurrent_sync(modules, pool_size, logger) mods_queue = modules_sync_queue(modules) sync_queue(mods_queue, pool_size, logger) end + # Returns a Queue of the names of modules actually updated def self.sync_queue(mods_queue, pool_size, logger) logger.debug _("Updating modules with %{pool_size} threads") % {pool_size: pool_size} - thread_pool = pool_size.times.map { sync_thread(mods_queue, logger) } + updated_modules = Queue.new + thread_pool = pool_size.times.map { sync_thread(mods_queue, logger, updated_modules) } thread_exception = nil # If any threads raise an exception the deployment is considered a failure. @@ -33,6 +40,8 @@ def self.sync_queue(mods_queue, pool_size, logger) # current work, then re-raise the first exception caught. begin thread_pool.each(&:join) + # Return the list of all modules that were actually updated + updated_modules rescue => e logger.error _("Error during concurrent deploy of a module: %{message}") % {message: e.message} mods_queue.clear @@ -65,11 +74,14 @@ def self.enqueue_modules(queue, modules) modules_by_cachedir.values.each {|mods| queue << mods } end - def self.sync_thread(mods_queue, logger) + def self.sync_thread(mods_queue, logger, updated_modules) Thread.new do begin while mods = mods_queue.pop(true) do - mods.each { |mod| mod.sync } + mods.each do |mod| + udpated = mod.sync + updated_modules << mod.name if udpated + end end rescue ThreadError => e logger.debug _("Module thread %{id} exiting: %{message}") % {message: e.message, id: Thread.current.object_id} diff --git a/lib/r10k/environment/base.rb b/lib/r10k/environment/base.rb index 056dafcc3..a3df66ee8 100644 --- a/lib/r10k/environment/base.rb +++ b/lib/r10k/environment/base.rb @@ -147,6 +147,8 @@ def accept(visitor) end end + + # Returns a Queue of the names of modules actually updated def deploy if @base_modules.nil? load_puppetfile_modules @@ -154,13 +156,15 @@ def deploy if ! @base_modules.empty? pool_size = @overrides.dig(:modules, :pool_size) - R10K::ContentSynchronizer.concurrent_sync(@base_modules, pool_size, logger) + updated_modules = R10K::ContentSynchronizer.concurrent_sync(@base_modules, pool_size, logger) end if (@overrides.dig(:purging, :purge_levels) || []).include?(:puppetfile) logger.debug("Purging unmanaged Puppetfile content for environment '#{dirname}'...") @puppetfile_cleaner.purge! end + + updated_modules end def load_puppetfile_modules diff --git a/lib/r10k/git/stateful_repository.rb b/lib/r10k/git/stateful_repository.rb index 867f00239..fbbe7a70a 100644 --- a/lib/r10k/git/stateful_repository.rb +++ b/lib/r10k/git/stateful_repository.rb @@ -35,6 +35,7 @@ def resolve(ref) @cache.resolve(ref) end + # Returns true if the sync actually updated the repo, false otherwise def sync(ref, force=true) @cache.sync if sync_cache?(ref) @@ -46,6 +47,7 @@ def sync(ref, force=true) workdir_status = status(ref) + updated = true case workdir_status when :absent logger.debug(_("Cloning %{repo_path} and checking out %{ref}") % {repo_path: @repo.path, ref: ref }) @@ -67,7 +69,9 @@ def sync(ref, force=true) end else logger.debug(_("%{repo_path} is already at Git ref %{ref}") % {repo_path: @repo.path, ref: ref }) + updated = false end + updated end def status(ref) diff --git a/lib/r10k/module/base.rb b/lib/r10k/module/base.rb index 66fd4c1e3..d7a2cd8f6 100644 --- a/lib/r10k/module/base.rb +++ b/lib/r10k/module/base.rb @@ -106,6 +106,7 @@ def delete_spec_dir # Synchronize this module with the indicated state. # @param [Hash] opts Deprecated + # @return [Boolean] true if the module was updated, false otherwise def sync(opts={}) raise NotImplementedError end diff --git a/lib/r10k/module/definition.rb b/lib/r10k/module/definition.rb index f275fb7ba..b66870953 100644 --- a/lib/r10k/module/definition.rb +++ b/lib/r10k/module/definition.rb @@ -23,8 +23,10 @@ def to_implementation end # syncing is a noop for module definitions + # Returns false to inidicate the module was not updated def sync(args = {}) logger.debug1(_("Not updating module %{name}, assuming content unchanged") % {name: name}) + false end def status diff --git a/lib/r10k/module/forge.rb b/lib/r10k/module/forge.rb index 7d53e1e3c..6d169013f 100644 --- a/lib/r10k/module/forge.rb +++ b/lib/r10k/module/forge.rb @@ -60,18 +60,24 @@ def initialize(title, dirname, opts, environment=nil) end # @param [Hash] opts Deprecated + # @return [Boolean] true if the module was updated, false otherwise def sync(opts={}) + updated = false if should_sync? case status when :absent + updated = true install when :outdated + updated = true upgrade when :mismatched + updated = true reinstall end maybe_delete_spec_dir end + updated end def properties diff --git a/lib/r10k/module/git.rb b/lib/r10k/module/git.rb index 7de55c592..f6873a799 100644 --- a/lib/r10k/module/git.rb +++ b/lib/r10k/module/git.rb @@ -98,10 +98,16 @@ def properties end # @param [Hash] opts Deprecated + # @return [Boolean] true if the module was updated, false otherwise def sync(opts={}) force = opts[:force] || @force - @repo.sync(version, force) if should_sync? + if should_sync? + updated = @repo.sync(version, force) + else + updated = false + end maybe_delete_spec_dir + updated end def status diff --git a/lib/r10k/module/local.rb b/lib/r10k/module/local.rb index f34681180..3fb29843b 100644 --- a/lib/r10k/module/local.rb +++ b/lib/r10k/module/local.rb @@ -28,7 +28,9 @@ def status end # @param [Hash] opts Deprecated + # @return [Boolean] false, because local modules are always considered in-sync def sync(opts={}) logger.debug1 _("Module %{title} is a local module, always indicating synced.") % {title: title} + false end end diff --git a/lib/r10k/module/svn.rb b/lib/r10k/module/svn.rb index d69389ca2..b83bdd3a0 100644 --- a/lib/r10k/module/svn.rb +++ b/lib/r10k/module/svn.rb @@ -75,17 +75,22 @@ def status # @param [Hash] opts Deprecated def sync(opts={}) + updated = false if should_sync? case status when :absent + updated = true install when :mismatched + updated = true reinstall when :outdated + updated = true update end maybe_delete_spec_dir end + updated end def exist? diff --git a/spec/unit/action/deploy/module_spec.rb b/spec/unit/action/deploy/module_spec.rb index 56f96e876..7be2bda67 100644 --- a/spec/unit/action/deploy/module_spec.rb +++ b/spec/unit/action/deploy/module_spec.rb @@ -103,21 +103,14 @@ end before do - @modules = [] allow(subject).to receive(:visit_environment).and_wrap_original do |original, environment, &block| - mod = R10K::Module::Local.new(environment.name, '/fakedir', {}, environment) - if mod.name == 'first' + if environment.name == 'first' + expect(environment).to receive(:deploy).and_return(['first']) expect(environment).to receive(:generate_types!) else + expect(environment).to receive(:deploy).and_return([]) expect(environment).not_to receive(:generate_types!) end - @modules << mod - allow(environment.loader).to receive(:load).and_return({ - modules: [mod], - desired_contents: [], - managed_directories: [], - purge_exclusions: [] - }).once original.call(environment, &block) end end @@ -126,9 +119,8 @@ expect(subject.settings[:overrides][:environments][:generate_types]).to eq(true) end - it 'only calls puppet generate types on environments with specified module' do + it 'only calls puppet generate types on environments where the specified module was updated' do subject.call - expect(@modules.length).to be(2) end end @@ -422,16 +414,13 @@ allow(mock_subprocess).to receive(:logger=) expect(mock_subprocess).to receive(:execute) - mock_mod = double('mock_mod', name: 'mod1') - expect(R10K::Util::Subprocess).to receive(:new). with(["/generate/types/wrapper", "first"]). and_return(mock_subprocess) expect(subject).to receive(:visit_environment).and_wrap_original do |original, environment, &block| if environment.name == 'first' - expect(environment).to receive(:deploy).and_return(true) - expect(environment).to receive(:modules).and_return([mock_mod]) + expect(environment).to receive(:deploy).and_return(['first']) end original.call(environment, &block) end.exactly(3).times @@ -462,12 +451,9 @@ with(["/generate/types/wrapper", "first third"]). and_return(mock_subprocess) - mock_mod = double('mock_mod', name: 'mod1') - expect(subject).to receive(:visit_environment).and_wrap_original do |original, environment, &block| - expect(environment).to receive(:deploy).and_return(true) if ['first', 'third'].include?(environment.name) - expect(environment).to receive(:modules).and_return([mock_mod]) + expect(environment).to receive(:deploy).and_return(['mod1']) end original.call(environment, &block) end.exactly(3).times @@ -480,9 +466,7 @@ mock_mod2 = double('mock_mod', name: 'mod2') expect(subject).to receive(:visit_environment).and_wrap_original do |original, environment, &block| - expect(environment).to receive(:deploy).and_return(true) - # Envs have a different module than the one we asked to deploy - expect(environment).to receive(:modules).and_return([mock_mod2]) + expect(environment).to receive(:deploy).and_return([]) original.call(environment, &block) end.exactly(3).times diff --git a/spec/unit/module/forge_spec.rb b/spec/unit/module/forge_spec.rb index df338fa3d..1183e9fdc 100644 --- a/spec/unit/module/forge_spec.rb +++ b/spec/unit/module/forge_spec.rb @@ -212,25 +212,31 @@ expect(subject).to receive(:install).never expect(subject).to receive(:upgrade).never expect(subject).to receive(:reinstall).never - subject.sync + expect(subject.sync).to be false end it 'reinstalls the module when it is mismatched' do allow(subject).to receive(:status).and_return :mismatched expect(subject).to receive(:reinstall) - subject.sync + expect(subject.sync).to be true end it 'upgrades the module when it is outdated' do allow(subject).to receive(:status).and_return :outdated expect(subject).to receive(:upgrade) - subject.sync + expect(subject.sync).to be true end it 'installs the module when it is absent' do allow(subject).to receive(:status).and_return :absent expect(subject).to receive(:install) - subject.sync + expect(subject.sync).to be true + end + + it 'returns false if `should_sync?` is false' do + # modules do not sync if they are not requested + mod = described_class.new('my_org/my_mod', '/path/to/mod', { overrides: { modules: { requested_modules: ['other_mod'] } } }) + expect(mod.sync).to be false end end diff --git a/spec/unit/module/git_spec.rb b/spec/unit/module/git_spec.rb index de417fa84..c30fb1abf 100644 --- a/spec/unit/module/git_spec.rb +++ b/spec/unit/module/git_spec.rb @@ -128,6 +128,25 @@ subject.sync expect(Dir.exist?(spec_path)).to eq true end + + it 'returns true if repo was updated' do + allow(mock_repo).to receive(:resolve).with('master').and_return('abc123') + expect(mock_repo).to receive(:sync).and_return(true) + expect(subject.sync).to be true + end + + it 'returns false if repo was not updated (in-sync)' do + allow(mock_repo).to receive(:resolve).with('master').and_return('abc123') + expect(mock_repo).to receive(:sync).and_return(false) + expect(subject.sync).to be false + end + + it 'returns false if `should_sync?` is false' do + # modules do not sync if they are not requested + mod = described_class.new(title, dirname, { overrides: { modules: { requested_modules: ['other_mod'] } } }) + allow(mock_repo).to receive(:resolve).with('master').and_return('abc123') + expect(mod.sync).to be false + end end describe "determining the status" do diff --git a/spec/unit/module/svn_spec.rb b/spec/unit/module/svn_spec.rb index b2b9b2550..6263ab03e 100644 --- a/spec/unit/module/svn_spec.rb +++ b/spec/unit/module/svn_spec.rb @@ -135,7 +135,6 @@ subject { described_class.new(title, dirname, {}) } it 'is kept by default' do - FileUtils.mkdir_p(spec_path) expect(subject).to receive(:status).and_return(:absent) expect(subject).to receive(:install).and_return(nil) @@ -157,7 +156,7 @@ it "installs the SVN module" do expect(subject).to receive(:install) - subject.sync + expect(subject.sync).to be true end end @@ -167,14 +166,14 @@ it "reinstalls the module" do expect(subject).to receive(:reinstall) - subject.sync + expect(subject.sync).to be true end it "removes the existing directory" do expect(subject.path).to receive(:rmtree) allow(subject).to receive(:install) - subject.sync + expect(subject.sync).to be true end end @@ -184,7 +183,7 @@ it "upgrades the repository" do expect(subject).to receive(:update) - subject.sync + expect(subject.sync).to be true end end @@ -196,8 +195,14 @@ expect(subject).to receive(:reinstall).never expect(subject).to receive(:update).never - subject.sync + expect(subject.sync).to be false end end + + it 'and `should_sync?` is false' do + # modules do not sync if they are not requested + mod = described_class.new('my_mod', '/path/to/mod', { overrides: { modules: { requested_modules: ['other_mod'] } } }) + expect(mod.sync).to be false + end end end diff --git a/spec/unit/puppetfile_spec.rb b/spec/unit/puppetfile_spec.rb index ea952960f..02389cd49 100644 --- a/spec/unit/puppetfile_spec.rb +++ b/spec/unit/puppetfile_spec.rb @@ -273,7 +273,7 @@ expect(subject).to receive(:modules).and_return([mod1, mod2]) expect(Thread).to receive(:new).exactly(pool_size).and_call_original - expect(Queue).to receive(:new).and_call_original + expect(Queue).to receive(:new).and_call_original.twice subject.accept(visitor) end