diff --git a/lib/r10k/action/base.rb b/lib/r10k/action/base.rb index 056215b23..13742a76b 100644 --- a/lib/r10k/action/base.rb +++ b/lib/r10k/action/base.rb @@ -10,6 +10,13 @@ class Base attr_accessor :settings + # @param opts [Hash] A hash of options defined in #allowed_initialized_opts + # and managed by the SetOps mixin within the Action::Base class. + # Corresponds to the CLI flags and options. + # @param argv [CRI::ArgumentList] A list-like collection of the remaining + # arguments to the CLI invocation (after removing flags and options). + # @param settings [Hash] A hash of configuration loaded from the relevant + # config (r10k.yaml). def initialize(opts, argv, settings) @opts = opts @argv = argv diff --git a/lib/r10k/action/deploy/display.rb b/lib/r10k/action/deploy/display.rb index 0991bbdcb..2a627abcc 100644 --- a/lib/r10k/action/deploy/display.rb +++ b/lib/r10k/action/deploy/display.rb @@ -9,6 +9,13 @@ class Display < R10K::Action::Base include R10K::Action::Deploy::DeployHelpers + # @param opts [Hash] A hash of options defined in #allowed_initialized_opts + # and managed by the SetOps mixin within the Action::Base class. + # Corresponds to the CLI flags and options. + # @param argv [CRI::ArgumentList] A list-like collection of the remaining + # arguments to the CLI invocation (after removing flags and options). + # @param settings [Hash] A hash of configuration loaded from the relevant + # config (r10k.yaml). def initialize(opts, argv, settings) super diff --git a/lib/r10k/action/deploy/environment.rb b/lib/r10k/action/deploy/environment.rb index e81a2c667..c91c7621a 100644 --- a/lib/r10k/action/deploy/environment.rb +++ b/lib/r10k/action/deploy/environment.rb @@ -19,6 +19,13 @@ class Environment < R10K::Action::Base attr_reader :settings + # @param opts [Hash] A hash of options defined in #allowed_initialized_opts + # and managed by the SetOps mixin within the Action::Base class. + # Corresponds to the CLI flags and options. + # @param argv [CRI::ArgumentList] A list-like collection of the remaining + # arguments to the CLI invocation (after removing flags and options). + # @param settings [Hash] A hash of configuration loaded from the relevant + # config (r10k.yaml). def initialize(opts, argv, settings) super @@ -51,11 +58,17 @@ def initialize(opts, argv, settings) def call @visit_ok = true - expect_config! - deployment = R10K::Deployment.new(@settings) - check_write_lock!(@settings) + begin + expect_config! + deployment = R10K::Deployment.new(@settings) + check_write_lock!(@settings) + + deployment.accept(self) + rescue => e + @visit_ok = false + logger.error R10K::Errors::Formatting.format_exception(e, @trace) + end - deployment.accept(self) @visit_ok end @@ -182,11 +195,6 @@ def visit_puppetfile(puppetfile) end end - def visit_module(mod) - logger.info _("Deploying %{origin} content %{path}") % {origin: mod.origin, path: mod.path} - mod.sync(force: @settings.dig(:overrides, :modules, :force)) - end - def write_environment_info!(environment, started_at, success) module_deploys = begin diff --git a/lib/r10k/action/deploy/module.rb b/lib/r10k/action/deploy/module.rb index ff3ad80a5..41db0d7d2 100644 --- a/lib/r10k/action/deploy/module.rb +++ b/lib/r10k/action/deploy/module.rb @@ -15,8 +15,14 @@ class Module < R10K::Action::Base attr_reader :settings + # @param opts [Hash] A hash of options defined in #allowed_initialized_opts + # and managed by the SetOps mixin within the Action::Base class. + # Corresponds to the CLI flags and options. + # @param argv [CRI::ArgumentList] A list-like collection of the remaining + # arguments to the CLI invocation (after removing flags and options). + # @param settings [Hash] A hash of configuration loaded from the relevant + # config (r10k.yaml). def initialize(opts, argv, settings) - super requested_env = @opts[:environment] ? [@opts[:environment].gsub(/\W/, '_')] : [] @@ -28,7 +34,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 }, @@ -40,12 +46,17 @@ def initialize(opts, argv, settings) def call @visit_ok = true + begin + expect_config! + deployment = R10K::Deployment.new(@settings) + check_write_lock!(@settings) + + deployment.accept(self) + rescue => e + @visit_ok = false + logger.error R10K::Errors::Formatting.format_exception(e, @trace) + end - expect_config! - deployment = R10K::Deployment.new(@settings) - check_write_lock!(@settings) - - deployment.accept(self) @visit_ok end @@ -67,7 +78,15 @@ def visit_environment(environment) logger.debug1(_("Only updating modules in environment(s) %{opt_env} skipping environment %{env_path}") % {opt_env: requested_envs.inspect, env_path: environment.path}) else logger.debug1(_("Updating modules %{modules} in environment %{env_path}") % {modules: @settings.dig(:overrides, :modules, :requested_modules).inspect, env_path: environment.path}) + yield + + requested_mods = @settings.dig(:overrides, :modules, :requested_modules) || [] + generate_types = @settings.dig(:overrides, :environments, :generate_types) + if generate_types && !((environment.modules.map(&:name) & requested_mods).empty?) + logger.debug("Generating puppet types for environment '#{environment.dirname}'...") + environment.generate_types! + end end end @@ -76,20 +95,6 @@ def visit_puppetfile(puppetfile) yield end - def visit_module(mod) - requested_mods = @settings.dig(:overrides, :modules, :requested_modules) - if requested_mods.include?(mod.name) - logger.info _("Deploying module %{mod_path}") % {mod_path: mod.path} - mod.sync(force: @settings.dig(:overrides, :modules, :force)) - 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}) - end - end - def allowed_initialize_opts super.merge(environment: true, cachedir: :self, diff --git a/lib/r10k/action/puppetfile/install.rb b/lib/r10k/action/puppetfile/install.rb index 8c75ac335..159764cf4 100644 --- a/lib/r10k/action/puppetfile/install.rb +++ b/lib/r10k/action/puppetfile/install.rb @@ -11,11 +11,17 @@ class Install < R10K::Action::Base def call @visit_ok = true - pf = R10K::Puppetfile.new(@root, - {moduledir: @moduledir, - puppetfile_path: @puppetfile, - force: @force}) - pf.accept(self) + begin + pf = R10K::Puppetfile.new(@root, + {moduledir: @moduledir, + puppetfile_path: @puppetfile, + force: @force || false}) + pf.accept(self) + rescue => e + @visit_ok = false + logger.error R10K::Errors::Formatting.format_exception(e, @trace) + end + @visit_ok end @@ -33,17 +39,6 @@ def visit_puppetfile(pf) pf.purge_exclusions).purge! end - def visit_module(mod) - @force ||= false - logger.info _("Updating module %{mod_path}") % {mod_path: mod.path} - - if mod.respond_to?(:desired_ref) && mod.desired_ref == :control_branch - logger.warn _("Cannot track control repo branch for content '%{name}' when not part of a 'deploy' action, will use default if available." % {name: mod.name}) - end - - mod.sync(force: @force) - end - def allowed_initialize_opts super.merge(root: :self, puppetfile: :self, moduledir: :self, force: :self ) end diff --git a/lib/r10k/content_synchronizer.rb b/lib/r10k/content_synchronizer.rb index 40942df93..d313f5fee 100644 --- a/lib/r10k/content_synchronizer.rb +++ b/lib/r10k/content_synchronizer.rb @@ -4,7 +4,7 @@ module ContentSynchronizer def self.serial_accept(modules, visitor, loader) visitor.visit(:puppetfile, loader) do modules.each do |mod| - mod.accept(visitor) + mod.sync end end end @@ -12,7 +12,7 @@ def self.serial_accept(modules, visitor, loader) def self.concurrent_accept(modules, visitor, loader, pool_size, logger) logger.debug _("Updating modules with %{pool_size} threads") % {pool_size: pool_size} mods_queue = modules_queue(modules, visitor, loader) - thread_pool = pool_size.times.map { visitor_thread(visitor, mods_queue, logger) } + thread_pool = pool_size.times.map { sync_thread(mods_queue, logger) } thread_exception = nil # If any threads raise an exception the deployment is considered a failure. @@ -42,11 +42,11 @@ def self.modules_queue(modules, visitor, loader) end end - def self.visitor_thread(visitor, mods_queue, logger) + def self.sync_thread(mods_queue, logger) Thread.new do begin while mods = mods_queue.pop(true) do - mods.each {|mod| mod.accept(visitor) } + mods.each { |mod| mod.sync } 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 d02a70eaa..ac972f885 100644 --- a/lib/r10k/environment/base.rb +++ b/lib/r10k/environment/base.rb @@ -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 diff --git a/lib/r10k/environment/with_modules.rb b/lib/r10k/environment/with_modules.rb index 419dfcd8e..ad7a972d2 100644 --- a/lib/r10k/environment/with_modules.rb +++ b/lib/r10k/environment/with_modules.rb @@ -78,7 +78,7 @@ def module_conflicts?(mod_b) def accept(visitor) visitor.visit(:environment, self) do @modules.each do |mod| - mod.accept(visitor) + mod.sync end puppetfile.accept(visitor) @@ -87,25 +87,27 @@ def accept(visitor) def load_modules(module_hash) module_hash.each do |name, args| + if !args.is_a?(Hash) + args = { version: args } + end + add_module(name, args) end end # @param [String] name - # @param [*Object] args + # @param [Hash] args def add_module(name, args) - if args.is_a?(Hash) - # symbolize keys in the args hash - args = args.inject({}) { |memo,(k,v)| memo[k.to_sym] = v; memo } - args[:overrides] = @overrides - - if install_path = args.delete(:install_path) - install_path = resolve_install_path(install_path) - validate_install_path(install_path, name) - end - end + # symbolize keys in the args hash + args = args.inject({}) { |memo,(k,v)| memo[k.to_sym] = v; memo } + args[:overrides] = @overrides - install_path ||= @moduledir + if install_path = args.delete(:install_path) + install_path = resolve_install_path(install_path) + validate_install_path(install_path, name) + else + install_path = @moduledir + end # Keep track of all the content this environment is managing to enable purging. @managed_content[install_path] = Array.new unless @managed_content.has_key?(install_path) diff --git a/lib/r10k/module.rb b/lib/r10k/module.rb index e28a5ef74..79932af28 100644 --- a/lib/r10k/module.rb +++ b/lib/r10k/module.rb @@ -17,7 +17,7 @@ def self.register(klass) # # @param [String] name The unique name of the module # @param [String] basedir The root to install the module in - # @param [Object] args An arbitary value or set of values that specifies the implementation + # @param [Hash] args An arbitary Hash that specifies the implementation # @param [R10K::Environment] environment Optional environment that this module is a part of # # @return [Object < R10K::Module] A member of the implementing subclass diff --git a/lib/r10k/module/base.rb b/lib/r10k/module/base.rb index 1b0a3ed2d..61e99e859 100644 --- a/lib/r10k/module/base.rb +++ b/lib/r10k/module/base.rb @@ -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) || [] + @should_sync = (@requested_modules.empty? || @requested_modules.include?(@name)) end # @deprecated @@ -61,11 +65,22 @@ def full_path end # Synchronize this module with the indicated state. - # @abstract + # @param [Hash] opts Deprecated def sync(opts={}) raise NotImplementedError end + def should_sync? + if @should_sync + logger.info _("Deploying module to %{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 @@ -87,6 +102,7 @@ def status raise NotImplementedError end + # Deprecated def accept(visitor) visitor.visit(:module, self) end diff --git a/lib/r10k/module/forge.rb b/lib/r10k/module/forge.rb index 3c1790701..19b2faa5c 100644 --- a/lib/r10k/module/forge.rb +++ b/lib/r10k/module/forge.rb @@ -13,7 +13,12 @@ class R10K::Module::Forge < R10K::Module::Base R10K::Module.register(self) def self.implement?(name, args) - (args.is_a?(Hash) && args[:type].to_s == 'forge') || (!!(name.match %r[\w+[/-]\w+]) && valid_version?(args)) + args[:type].to_s == 'forge' || + (!! + ((args.keys & %i{git svn type}).empty? && + args.has_key?(:version) && + name.match(%r[\w+[/-]\w+]) && + valid_version?(args[:version]))) end def self.valid_version?(expected_version) @@ -40,29 +45,29 @@ def initialize(title, dirname, opts, environment=nil) @metadata_file = R10K::Module::MetadataFile.new(path + 'metadata.json') @metadata = @metadata_file.read - if opts.is_a?(Hash) - setopts(opts, { - # Standard option interface - :version => :expected_version, - :source => ::R10K::Util::Setopts::Ignore, - :type => ::R10K::Util::Setopts::Ignore, - :overrides => :self, - }) - else - @expected_version = opts || current_version || :latest - end + setopts(opts, { + # Standard option interface + :version => :expected_version, + :source => ::R10K::Util::Setopts::Ignore, + :type => ::R10K::Util::Setopts::Ignore, + }) + + @expected_version ||= current_version || :latest @v3_module = PuppetForge::V3::Module.new(:slug => @title) end + # @param [Hash] opts Deprecated def sync(opts={}) - case status - when :absent - install - when :outdated - upgrade - when :mismatched - reinstall + if should_sync? + case status + when :absent + install + when :outdated + upgrade + when :mismatched + reinstall + end end end diff --git a/lib/r10k/module/git.rb b/lib/r10k/module/git.rb index 957439b52..d580aef3c 100644 --- a/lib/r10k/module/git.rb +++ b/lib/r10k/module/git.rb @@ -8,7 +8,7 @@ class R10K::Module::Git < R10K::Module::Base R10K::Module.register(self) def self.implement?(name, args) - args.is_a?(Hash) && (args.has_key?(:git) || args[:type].to_s == 'git') + args.has_key?(:git) || args[:type].to_s == 'git' rescue false end @@ -36,28 +36,35 @@ 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.dig(:modules, :force) + @force = force == false ? false : true + @desired_ref ||= 'master' - if @desired_ref == :control_branch && @environment && @environment.respond_to?(:ref) - @desired_ref = @environment.ref + if @desired_ref == :control_branch + if @environment && @environment.respond_to?(:ref) + @desired_ref = @environment.ref + else + logger.warn _("Cannot track control repo branch for content '%{name}' when not part of a git-backed environment, will use default if available." % {name: name}) + end end @repo = R10K::Git::StatefulRepository.new(@remote, @dirname, @name) @@ -75,9 +82,10 @@ def properties } end + # @param [Hash] opts Deprecated def sync(opts={}) - force = opts && opts.fetch(:force, true) - @repo.sync(version, force) + force = opts[:force] || @force + @repo.sync(version, force) if should_sync? end def status diff --git a/lib/r10k/module/local.rb b/lib/r10k/module/local.rb index 61add3e2d..3135a96aa 100644 --- a/lib/r10k/module/local.rb +++ b/lib/r10k/module/local.rb @@ -30,6 +30,7 @@ def status :insync end + # @param [Hash] opts Deprecated def sync(opts={}) logger.debug1 _("Module %{title} is a local module, always indicating synced.") % {title: title} end diff --git a/lib/r10k/module/svn.rb b/lib/r10k/module/svn.rb index b20b2dcb4..16ab316ba 100644 --- a/lib/r10k/module/svn.rb +++ b/lib/r10k/module/svn.rb @@ -7,7 +7,7 @@ class R10K::Module::SVN < R10K::Module::Base R10K::Module.register(self) def self.implement?(name, args) - args.is_a?(Hash) && (args.has_key?(:svn) || args[:type].to_s == 'svn') + args.has_key?(:svn) || args[:type].to_s == 'svn' end # @!attribute [r] expected_revision @@ -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) @@ -70,14 +69,17 @@ def status end end + # @param [Hash] opts Deprecated def sync(opts={}) - case status - when :absent - install - when :mismatched - reinstall - when :outdated - update + if should_sync? + case status + when :absent + install + when :mismatched + reinstall + when :outdated + update + end end end diff --git a/lib/r10k/puppetfile.rb b/lib/r10k/puppetfile.rb index 3d0758bb5..56968790d 100644 --- a/lib/r10k/puppetfile.rb +++ b/lib/r10k/puppetfile.rb @@ -133,22 +133,29 @@ def set_moduledir(moduledir) end # @param [String] name - # @param [*Object] args + # @param [Hash, String, Symbol] args Calling with anything but a Hash is + # deprecated. The DSL will now convert String and Symbol versions to + # Hashes of the shape + # { version: } + # def add_module(name, args) - if args.is_a?(Hash) - args[:overrides] = @overrides + if !args.is_a?(Hash) + args = { version: args } + end - if install_path = args.delete(:install_path) - install_path = resolve_install_path(install_path) - validate_install_path(install_path, name) - end + args[:overrides] = @overrides - if @default_branch_override != nil - args[:default_branch_override] = @default_branch_override - end + if install_path = args.delete(:install_path) + install_path = resolve_install_path(install_path) + validate_install_path(install_path, name) + else + install_path = @moduledir + end + + if @default_branch_override != nil + args[:default_branch_override] = @default_branch_override end - install_path ||= @moduledir mod = R10K::Module.new(name, install_path, args, @environment) mod.origin = :puppetfile @@ -248,7 +255,13 @@ def initialize(librarian) end def mod(name, args = nil) - @librarian.add_module(name, args) + if args.is_a?(Hash) + opts = args + else + opts = { version: args } + end + + @librarian.add_module(name, opts) end def forge(location) diff --git a/spec/unit/action/deploy/module_spec.rb b/spec/unit/action/deploy/module_spec.rb index 2c7b722e6..1bc58075e 100644 --- a/spec/unit/action/deploy/module_spec.rb +++ b/spec/unit/action/deploy/module_spec.rb @@ -82,10 +82,16 @@ end before do + @modules = [] 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)] - ) + mod = R10K::Module::Local.new(environment.name, '/fakedir', {}, environment) + if mod.name == 'first' + expect(environment).to receive(:generate_types!) + else + expect(environment).not_to receive(:generate_types!) + end + @modules << mod + expect(environment.puppetfile).to receive(:modules).and_return([mod]).twice original.call(environment, &block) end end @@ -95,15 +101,8 @@ end it 'only calls puppet generate types on environments with specified module' do - expect(subject).to receive(:visit_module).and_wrap_original do |original, mod, &block| - if mod.name == 'first' - expect(mod.environment).to receive(:generate_types!) - else - expect(mod.environment).not_to receive(:generate_types!) - end - original.call(mod, &block) - end.twice subject.call + expect(@modules.length).to be(2) end end @@ -177,4 +176,54 @@ 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 + puppetfile.modules.each do |mod| + if ['mod1', 'mod2'].include?(mod.name) + expect(mod.should_sync?).to be(true) + else + expect(mod.should_sync?).to be(false) + end + + expect(mod).to receive(:sync).and_call_original + end + + original.call(puppetfile, &block) + end + + expect(repo).to receive(:sync).twice + + subject.call + end + end end + diff --git a/spec/unit/action/puppetfile/install_spec.rb b/spec/unit/action/puppetfile/install_spec.rb index 6ba5148e1..081751c0b 100644 --- a/spec/unit/action/puppetfile/install_spec.rb +++ b/spec/unit/action/puppetfile/install_spec.rb @@ -5,7 +5,7 @@ let(:default_opts) { { root: "/some/nonexistent/path" } } let(:puppetfile) { R10K::Puppetfile.new('/some/nonexistent/path', - {:moduledir => nil, :puppetfile_path => nil, :force => nil}) + {:moduledir => nil, :puppetfile_path => nil, :force => false}) } def installer(opts = {}, argv = [], settings = {}) @@ -17,7 +17,7 @@ def installer(opts = {}, argv = [], settings = {}) allow(puppetfile).to receive(:load!).and_return(nil) allow(R10K::Puppetfile).to receive(:new). with("/some/nonexistent/path", - {:moduledir => nil, :puppetfile_path => nil, :force => nil}). + {:moduledir => nil, :puppetfile_path => nil, :force => false}). and_return(puppetfile) end @@ -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 @@ -73,7 +73,7 @@ def installer(opts = {}, argv = [], settings = {}) it "can use a custom puppetfile path" do expect(R10K::Puppetfile).to receive(:new). with("/some/nonexistent/path", - {:moduledir => nil, :force => nil, puppetfile_path: "/some/other/path/Puppetfile"}). + {:moduledir => nil, :force => false, puppetfile_path: "/some/other/path/Puppetfile"}). and_return(puppetfile) installer({puppetfile: "/some/other/path/Puppetfile"}).call @@ -82,7 +82,7 @@ def installer(opts = {}, argv = [], settings = {}) it "can use a custom moduledir path" do expect(R10K::Puppetfile).to receive(:new). with("/some/nonexistent/path", - {:puppetfile_path => nil, :force => nil, moduledir: "/some/other/path/site-modules"}). + {:puppetfile_path => nil, :force => false, moduledir: "/some/other/path/site-modules"}). and_return(puppetfile) installer({moduledir: "/some/other/path/site-modules"}).call diff --git a/spec/unit/module/forge_spec.rb b/spec/unit/module/forge_spec.rb index cc9fb1253..b82964beb 100644 --- a/spec/unit/module/forge_spec.rb +++ b/spec/unit/module/forge_spec.rb @@ -11,15 +11,15 @@ describe "implementing the Puppetfile spec" do it "should implement 'branan/eight_hundred', '8.0.0'" do - expect(described_class).to be_implement('branan/eight_hundred', '8.0.0') + expect(described_class).to be_implement('branan/eight_hundred', { version: '8.0.0' }) end it "should implement 'branan-eight_hundred', '8.0.0'" do - expect(described_class).to be_implement('branan-eight_hundred', '8.0.0') + expect(described_class).to be_implement('branan-eight_hundred', { version: '8.0.0' }) end it "should fail with an invalid title" do - expect(described_class).to_not be_implement('branan!eight_hundred', '8.0.0') + expect(described_class).to_not be_implement('branan!eight_hundred', { version: '8.0.0' }) end end @@ -30,7 +30,7 @@ end describe "setting attributes" do - subject { described_class.new('branan/eight_hundred', '/moduledir', '8.0.0') } + subject { described_class.new('branan/eight_hundred', '/moduledir', { version: '8.0.0' }) } it "sets the name" do expect(subject.name).to eq 'eight_hundred' @@ -50,7 +50,7 @@ end describe "properties" do - subject { described_class.new('branan/eight_hundred', fixture_modulepath, '8.0.0') } + subject { described_class.new('branan/eight_hundred', fixture_modulepath, { version: '8.0.0' }) } it "sets the module type to :forge" do expect(subject.properties).to include(:type => :forge) @@ -67,7 +67,7 @@ end context "when a module is deprecated" do - subject { described_class.new('puppetlabs/corosync', fixture_modulepath, :latest) } + subject { described_class.new('puppetlabs/corosync', fixture_modulepath, { version: :latest }) } it "warns on sync if module is not already insync" do allow(subject).to receive(:status).and_return(:absent) @@ -77,6 +77,7 @@ logger_dbl = double(Log4r::Logger) allow_any_instance_of(described_class).to receive(:logger).and_return(logger_dbl) + allow(logger_dbl).to receive(:info).with(/Deploying module to.*/) expect(logger_dbl).to receive(:warn).with(/puppet forge module.*puppetlabs-corosync.*has been deprecated/i) subject.sync @@ -88,6 +89,7 @@ logger_dbl = double(Log4r::Logger) allow_any_instance_of(described_class).to receive(:logger).and_return(logger_dbl) + allow(logger_dbl).to receive(:info).with(/Deploying module to.*/) expect(logger_dbl).to_not receive(:warn).with(/puppet forge module.*puppetlabs-corosync.*has been deprecated/i) subject.sync @@ -96,12 +98,12 @@ describe '#expected_version' do it "returns an explicitly given expected version" do - subject = described_class.new('branan/eight_hundred', fixture_modulepath, '8.0.0') + subject = described_class.new('branan/eight_hundred', fixture_modulepath, { version: '8.0.0' }) expect(subject.expected_version).to eq '8.0.0' end it "uses the latest version from the forge when the version is :latest" do - subject = described_class.new('branan/eight_hundred', fixture_modulepath, :latest) + subject = described_class.new('branan/eight_hundred', fixture_modulepath, { version: :latest }) expect(subject.v3_module).to receive_message_chain(:current_release, :version).and_return('8.8.8') expect(subject.expected_version).to eq '8.8.8' end @@ -109,7 +111,7 @@ describe "determining the status" do - subject { described_class.new('branan/eight_hundred', fixture_modulepath, '8.0.0') } + subject { described_class.new('branan/eight_hundred', fixture_modulepath, { version: '8.0.0' }) } it "is :absent if the module directory is absent" do allow(subject).to receive(:exist?).and_return false @@ -154,7 +156,7 @@ end describe "#sync" do - subject { described_class.new('branan/eight_hundred', fixture_modulepath, '8.0.0') } + subject { described_class.new('branan/eight_hundred', fixture_modulepath, { version: '8.0.0' }) } it 'does nothing when the module is in sync' do allow(subject).to receive(:status).and_return :insync @@ -186,7 +188,7 @@ describe '#install' do it 'installs the module from the forge' do - subject = described_class.new('branan/eight_hundred', fixture_modulepath, '8.0.0') + subject = described_class.new('branan/eight_hundred', fixture_modulepath, { version: '8.0.0' }) release = instance_double('R10K::Forge::ModuleRelease') expect(R10K::Forge::ModuleRelease).to receive(:new).with('branan-eight_hundred', '8.0.0').and_return(release) expect(release).to receive(:install).with(subject.path) @@ -196,7 +198,7 @@ describe '#uninstall' do it 'removes the module path' do - subject = described_class.new('branan/eight_hundred', fixture_modulepath, '8.0.0') + subject = described_class.new('branan/eight_hundred', fixture_modulepath, { version: '8.0.0' }) expect(FileUtils).to receive(:rm_rf).with(subject.path.to_s) subject.uninstall end @@ -204,7 +206,7 @@ describe '#reinstall' do it 'uninstalls and then installs the module' do - subject = described_class.new('branan/eight_hundred', fixture_modulepath, '8.0.0') + subject = described_class.new('branan/eight_hundred', fixture_modulepath, { version: '8.0.0' }) expect(subject).to receive(:uninstall) expect(subject).to receive(:install) subject.reinstall diff --git a/spec/unit/module/git_spec.rb b/spec/unit/module/git_spec.rb index 231d3738b..3b1666658 100644 --- a/spec/unit/module/git_spec.rb +++ b/spec/unit/module/git_spec.rb @@ -210,6 +210,14 @@ def test_module(extra_opts, env=nil) expect(mod.desired_ref).to eq(:control_branch) end + it "warns control branch may be unresolvable" do + logger = double("logger") + allow_any_instance_of(described_class).to receive(:logger).and_return(logger) + expect(logger).to receive(:warn).with(/Cannot track control repo branch.*boolean.*/) + + test_module(branch: :control_branch) + end + context "when default ref is provided and resolvable" do it "uses default ref" do expect(mock_repo).to receive(:resolve).with('default').and_return('abc123') diff --git a/spec/unit/module_spec.rb b/spec/unit/module_spec.rb index 6f9ccad37..ec5022ed3 100644 --- a/spec/unit/module_spec.rb +++ b/spec/unit/module_spec.rb @@ -30,12 +30,12 @@ [ 'bar/quux', 'bar-quux', ].each do |scenario| - it "accepts a name matching #{scenario} and args nil" do - obj = R10K::Module.new(scenario, '/modulepath', nil) + it "accepts a name matching #{scenario} and version nil" do + obj = R10K::Module.new(scenario, '/modulepath', { version: nil }) expect(obj).to be_a_kind_of(R10K::Module::Forge) end end - [ '8.0.0', + [ {version: '8.0.0'}, {type: 'forge', version: '8.0.0'}, ].each do |scenario| it "accepts a name matching bar-quux and args #{scenario.inspect}" do @@ -65,7 +65,7 @@ end it 'sets the expected version to what is found in the metadata' do - obj = R10K::Module.new(@title, @dirname, nil) + obj = R10K::Module.new(@title, @dirname, {version: nil}) expect(obj.send(:instance_variable_get, :'@expected_version')).to eq('1.2.0') end end @@ -73,7 +73,7 @@ it "raises an error if delegation fails" do expect { - R10K::Module.new('bar!quux', '/modulepath', ["NOPE NOPE NOPE NOPE!"]) + R10K::Module.new('bar!quux', '/modulepath', {version: ["NOPE NOPE NOPE NOPE!"]}) }.to raise_error RuntimeError, /doesn't have an implementation/ end end diff --git a/spec/unit/puppetfile_spec.rb b/spec/unit/puppetfile_spec.rb index 653a0b4c4..ede28077e 100644 --- a/spec/unit/puppetfile_spec.rb +++ b/spec/unit/puppetfile_spec.rb @@ -68,15 +68,15 @@ end describe "adding modules" do - it "should accept Forge modules with a string arg" do - allow(R10K::Module).to receive(:new).with('puppet/test_module', subject.moduledir, '1.2.3', anything).and_call_original + it "should transform Forge modules with a string arg to have a version key" do + allow(R10K::Module).to receive(:new).with('puppet/test_module', subject.moduledir, hash_including(version: '1.2.3'), anything).and_call_original expect { subject.add_module('puppet/test_module', '1.2.3') }.to change { subject.modules } expect(subject.modules.collect(&:name)).to include('test_module') end it "should not accept Forge modules with a version comparison" do - allow(R10K::Module).to receive(:new).with('puppet/test_module', subject.moduledir, '< 1.2.0', anything).and_call_original + allow(R10K::Module).to receive(:new).with('puppet/test_module', subject.moduledir, hash_including(version: '< 1.2.0'), anything).and_call_original expect { subject.add_module('puppet/test_module', '< 1.2.0') @@ -181,7 +181,7 @@ describe '#managed_directories' do it 'returns an array of paths that can be purged' do - allow(R10K::Module).to receive(:new).with('puppet/test_module', subject.moduledir, '1.2.3', anything).and_call_original + allow(R10K::Module).to receive(:new).with('puppet/test_module', subject.moduledir, hash_including(version: '1.2.3'), anything).and_call_original subject.add_module('puppet/test_module', '1.2.3') expect(subject.managed_directories).to match_array(["/some/nonexistent/basedir/modules"]) @@ -301,7 +301,7 @@ def expect_wrapped_error(orig, pf_path, wrapped_error) subject.accept(visitor) end - it "passes the visitor to each module if the visitor yields" do + it "synchronizes each module if the visitor yields" do visitor = spy('visitor') expect(visitor).to receive(:visit) do |type, other, &block| expect(type).to eq :puppetfile @@ -311,8 +311,8 @@ def expect_wrapped_error(orig, pf_path, wrapped_error) mod1 = instance_double('R10K::Module::Base', :cachedir => :none) mod2 = instance_double('R10K::Module::Base', :cachedir => :none) - expect(mod1).to receive(:accept).with(visitor) - expect(mod2).to receive(:accept).with(visitor) + expect(mod1).to receive(:sync) + expect(mod2).to receive(:sync) expect(subject).to receive(:modules).and_return([mod1, mod2]) subject.accept(visitor) @@ -332,8 +332,8 @@ def expect_wrapped_error(orig, pf_path, wrapped_error) mod1 = instance_double('R10K::Module::Base', :cachedir => :none) mod2 = instance_double('R10K::Module::Base', :cachedir => :none) - expect(mod1).to receive(:accept).with(visitor) - expect(mod2).to receive(:accept).with(visitor) + expect(mod1).to receive(:sync) + expect(mod2).to receive(:sync) expect(subject).to receive(:modules).and_return([mod1, mod2]) expect(Thread).to receive(:new).exactly(pool_size).and_call_original