Skip to content

Commit

Permalink
(maint) Allow modules to decide if they should sync
Browse files Browse the repository at this point in the history 
Previously, the deploy module action would determine in the visit_module
method if the visited module should be synchronized.

This patch moves that logic into the Module::Base class and makes it
available to subclasses via the `will_sync?` method. It also updates
each module implementation's sync method to only act if `will_sync?`
returns true.
  • Loading branch information
@justinstoller
justinstoller committed May 7, 2021
1 parent dd7f8ff commit 1901284
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 60 deletions.
17 changes: 7 additions & 10 deletions lib/r10k/action/deploy/module.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def initialize(opts, argv, settings)
generate_types: @generate_types
},
modules: {
requested_modules: @argv,
requested_modules: @argv.map.to_a,
# force here is used to make it easier to reason about
force: !@no_force
},
Expand Down Expand Up @@ -77,15 +77,12 @@ def visit_puppetfile(puppetfile)
end

def visit_module(mod)
requested_mods = @settings.dig(:overrides, :modules, :requested_modules)
if requested_mods.include?(mod.name)
mod.sync
if mod.environment && @settings.dig(:overrides, :environments, :generate_types)
logger.debug("Generating puppet types for environment '#{mod.environment.dirname}'...")
mod.environment.generate_types!
end
else
logger.debug1(_("Only updating modules %{modules}, skipping module %{mod_name}") % {modules: requested_mods.inspect, mod_name: mod.name})
mod.sync

requested_mods = @settings.dig(:overrides, :modules, :requested_modules) || []
if requested_mods.include?(mod.name) && mod.environment && @settings.dig(:overrides, :environments, :generate_types)
logger.debug("Generating puppet types for environment '#{mod.environment.dirname}'...")
mod.environment.generate_types!
end
end

Expand Down
6 changes: 3 additions & 3 deletions lib/r10k/environment/base.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ def initialize(name, basedir, dirname, options = {})
@dirname = dirname
@options = options
@puppetfile_name = options.delete(:puppetfile_name)
@overrides = options.delete(:overrides)
@overrides = options.delete(:overrides) || {}

@full_path = File.join(@basedir, @dirname)
@path = Pathname.new(File.join(@basedir, @dirname))

@puppetfile = R10K::Puppetfile.new(@full_path,
{overrides: @options[:overrides],
force: @options.dig(:overrides, :modules, :force),
{overrides: @overrides,
force: @overrides.dig(:modules, :force),
puppetfile_name: @puppetfile_name})
@puppetfile.environment = self
end
Expand Down
17 changes: 16 additions & 1 deletion lib/r10k/module/base.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ def initialize(title, dirname, args, environment=nil)
@owner, @name = parse_title(@title)
@path = Pathname.new(File.join(@dirname, @name))
@environment = environment
@overrides = args.delete(:overrides) || {}
@origin = 'external' # Expect Puppetfile or R10k::Environment to set this to a specific value

@requested_modules = @overrides.dig(:modules, :requested_modules) || []
@to_be_synced = (@requested_modules.empty? || @requested_modules.include?(@name))
end

# @deprecated
Expand All @@ -63,9 +67,20 @@ def full_path
# Synchronize this module with the indicated state.
# @param [Hash] opts Deprecated
def sync(opts={})
logger.info _("Checking module %{path}") % {path: path}
raise NotImplementedError
end

def will_sync?
if @to_be_synced
logger.info _("Checking module %{path}") % {path: path}
true
else
logger.debug1(_("Only updating modules %{modules}, skipping module %{name}") % {modules: @requested_modules.inspect, name: name})
false
end
end


# Return the desired version of this module
# @abstract
def version
Expand Down
25 changes: 12 additions & 13 deletions lib/r10k/module/forge.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,9 @@ def initialize(title, dirname, opts, environment=nil)

setopts(opts, {
# Standard option interface
:version => :expected_version,
:source => ::R10K::Util::Setopts::Ignore,
:type => ::R10K::Util::Setopts::Ignore,
:overrides => :self,
:version => :expected_version,
:source => ::R10K::Util::Setopts::Ignore,
:type => ::R10K::Util::Setopts::Ignore,
})

@expected_version ||= current_version || :latest
Expand All @@ -60,15 +59,15 @@ def initialize(title, dirname, opts, environment=nil)

# @param [Hash] opts Deprecated
def sync(opts={})
super

case status
when :absent
install
when :outdated
upgrade
when :mismatched
reinstall
if will_sync?
case status
when :absent
install
when :outdated
upgrade
when :mismatched
reinstall
end
end
end

Expand Down
24 changes: 11 additions & 13 deletions lib/r10k/module/git.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,25 @@ def self.implement?(name, args)
include R10K::Util::Setopts

def initialize(title, dirname, opts, environment=nil)

super
setopts(opts, {
# Standard option interface
:version => :desired_ref,
:source => :remote,
:type => ::R10K::Util::Setopts::Ignore,
:overrides => :self,
:version => :desired_ref,
:source => :remote,
:type => ::R10K::Util::Setopts::Ignore,

# Type-specific options
:branch => :desired_ref,
:tag => :desired_ref,
:commit => :desired_ref,
:ref => :desired_ref,
:git => :remote,
:branch => :desired_ref,
:tag => :desired_ref,
:commit => :desired_ref,
:ref => :desired_ref,
:git => :remote,
:default_branch => :default_ref,
:default_branch_override => :default_override_ref,
})

force = @overrides && @overrides.dig(:modules, :force)
force = @overrides.dig(:modules, :force)
@force = force == false ? false : true

@desired_ref ||= 'master'
Expand Down Expand Up @@ -84,10 +84,8 @@ def properties

# @param [Hash] opts Deprecated
def sync(opts={})
super

force = opts[:force] || @force
@repo.sync(version, force)
@repo.sync(version, force) if will_sync?
end

def status
Expand Down
35 changes: 17 additions & 18 deletions lib/r10k/module/svn.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,16 @@ def initialize(name, dirname, opts, environment=nil)
super
setopts(opts, {
# Standard option interface
:source => :url,
:version => :expected_revision,
:type => ::R10K::Util::Setopts::Ignore,
:overrides => :self,
:source => :url,
:version => :expected_revision,
:type => ::R10K::Util::Setopts::Ignore,

# Type-specific options
:svn => :url,
:rev => :expected_revision,
:revision => :expected_revision,
:username => :self,
:password => :self
:svn => :url,
:rev => :expected_revision,
:revision => :expected_revision,
:username => :self,
:password => :self
})

@working_dir = R10K::SVN::WorkingDir.new(@path, :username => @username, :password => @password)
Expand All @@ -72,15 +71,15 @@ def status

# @param [Hash] opts Deprecated
def sync(opts={})
super

case status
when :absent
install
when :mismatched
reinstall
when :outdated
update
if will_sync?
case status
when :absent
install
when :mismatched
reinstall
when :outdated
update
end
end
end

Expand Down
55 changes: 54 additions & 1 deletion spec/unit/action/deploy/module_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
before do
allow(subject).to receive(:visit_environment).and_wrap_original do |original, environment, &block|
expect(environment.puppetfile).to receive(:modules).and_return(
[R10K::Module::Local.new(environment.name, '/fakedir', [], environment)]
[R10K::Module::Local.new(environment.name, '/fakedir', {}, environment)]
)
original.call(environment, &block)
end
Expand Down Expand Up @@ -177,4 +177,57 @@
expect(subject.instance_variable_get(:@oauth_token)).to eq('/nonexistent')
end
end

describe 'with modules' do

subject { described_class.new({ config: '/some/nonexistent/path' }, ['mod1', 'mod2'], {}) }

let(:cache) { instance_double("R10K::Git::Cache", 'sanitized_dirname' => 'foo', 'cached?' => true, 'sync' => true) }
let(:repo) { instance_double("R10K::Git::StatefulRepository", cache: cache, resolve: 'main') }

it 'does not sync modules not given' do
allow(R10K::Deployment).to receive(:new).and_wrap_original do |original, settings, &block|
original.call(settings.merge({
sources: {
main: {
remote: 'git://not/a/remote',
basedir: '/not/a/basedir',
type: 'git'
}
}
}))
end

allow(R10K::Git::StatefulRepository).to receive(:new).and_return(repo)
allow(R10K::Git).to receive_message_chain(:cache, :generate).and_return(cache)
allow_any_instance_of(R10K::Source::Git).to receive(:branch_names).and_return([R10K::Environment::Name.new('first', {})])

expect(subject).to receive(:visit_puppetfile).and_wrap_original do |original, puppetfile, &block|
expect(puppetfile).to receive(:load) do
puppetfile.add_module('mod1', { git: 'git://remote' })
puppetfile.add_module('mod2', { git: 'git://remote' })
puppetfile.add_module('mod3', { git: 'git://remote' })
end

original.call(puppetfile, &block)
end

expect(subject).to receive(:visit_module).and_wrap_original do |original, mod, &block|
if ['mod1', 'mod2'].include?(mod.name)
expect(mod.will_sync?).to be(true)
else
expect(mod.will_sync?).to be(false)
end

expect(mod).to receive(:sync).and_call_original

original.call(mod)
end.exactly(3).times

expect(repo).to receive(:sync).twice

subject.call
end
end
end

2 changes: 1 addition & 1 deletion spec/unit/action/puppetfile/install_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def installer(opts = {}, argv = [], settings = {})
describe "installing modules" do
let(:modules) do
(1..4).map do |idx|
R10K::Module::Base.new("author/modname#{idx}", "/some/nonexistent/path/modname#{idx}", nil)
R10K::Module::Base.new("author/modname#{idx}", "/some/nonexistent/path/modname#{idx}", {})
end
end

Expand Down

0 comments on commit 1901284

Please sign in to comment.