Merge pull request #1211 from justinstoller/CODEMGMT-1415-fixups

(CODEMGMT-1415) Sundry Improvements

CHANGELOG.mkd

@@ -4,6 +4,7 @@ CHANGELOG
 Unreleased
 ----------
 
+- (CODEMGMT-1415) Provide a `--incremental` flag to only sync those modules in a Puppetfile whose definitions have changed since last sync, or those whose versions could change. [#1200](https://github.com/puppetlabs/r10k/pull/1200)
 - (CODEMGMT-1454) Ensure missing repo caches are re-synced [#1210](https://github.com/puppetlabs/r10k/pull/1210)
 - (PF-2437) Allow token authentication to be used with the Forge. [#1192](https://github.com/puppetlabs/r10k/pull/1192)
 

doc/dynamic-environments/usage.mkd

@@ -84,7 +84,7 @@ explicit, static version. These are released Forge versions, or Git modules usin
 the `:tag`, or `:commit` keys. Git `:ref`s containing only the full 40 character
 commit SHA will also be treated as static versions. Then invoke a deploy with:
 
-    r10k deploy environment production --modules --assume-unchanged
+    r10k deploy environment production --modules --incremental
 
 - - -
 

integration/tests/basic_functionality/basic_deployment.rb

@@ -0,0 +1,176 @@
+require 'git_utils'
+require 'r10k_utils'
+require 'master_manipulator'
+
+
+test_name 'Basic Environment Deployment Workflows'
+
+# This isn't a block because we want to use the local variables throughout the file
+step 'init'
+  @env_path             = on(master, puppet('config print environmentpath')).stdout.rstrip
+  r10k_fqp              = get_r10k_fqp(master)
+
+  control_repo_gitdir   = '/git_repos/environments.git'
+  control_repo_worktree = '/root/environments'
+  last_commit           = git_last_commit(master, control_repo_worktree)
+  git_provider          = ENV['GIT_PROVIDER']
+
+  config_path           = get_r10k_config_file_path(master)
+  config_backup_path    = "#{config_path}.bak"
+
+  puppetfile1 =<<-EOS
+    mod 'puppetlabs/apache', '0.10.0'
+    mod 'puppetlabs/stdlib', '8.0.0'
+    EOS
+
+  r10k_conf = <<-CONF
+    cachedir: '/var/cache/r10k'
+    git:
+      provider: '#{git_provider}'
+    sources:
+      control:
+        basedir: "#{@env_path}"
+        remote: "#{control_repo_gitdir}"
+    deploy:
+      purge_levels: ['deployment','environment','puppetfile']
+
+    CONF
+
+
+def and_stdlib_is_correct
+  metadata_path = "#{@env_path}/production/modules/stdlib/metadata.json"
+  on(master, "test -f #{metadata_path}", accept_all_exit_codes: true) do |result|
+    assert(result.exit_code == 0, 'stdlib content has been inappropriately purged')
+  end
+  metadata_info = JSON.parse(on(master, "cat #{metadata_path}").stdout)
+  assert(metadata_info['version'] == '8.0.0', 'stdlib deployed to wrong version')
+end
+
+teardown do
+  on(master, "mv #{config_backup_path} #{config_path}")
+  clean_up_r10k(master, last_commit, control_repo_worktree)
+end
+
+step 'Set up r10k and control repo' do
+
+  # Backup and replace r10k config
+  on(master, "mv #{config_path} #{config_backup_path}")
+  create_remote_file(master, config_path, r10k_conf)
+
+  # Place our Puppetfile in the control repo's production branch
+  git_on(master, 'checkout production', control_repo_worktree)
+  create_remote_file(master, "#{control_repo_worktree}/Puppetfile", puppetfile1)
+  git_add_commit_push(master, 'production', 'add Puppetfile for Basic Deployment test', control_repo_worktree)
+
+  # Ensure the production environment will be deployed anew
+  on(master, "rm -rf #{@env_path}/production")
+end
+
+test_path = "#{@env_path}/production/modules/apache/metadata.json"
+step 'Test initial environment deploy works' do
+  on(master, "#{r10k_fqp} deploy environment production --verbose=info") do |result|
+    assert(result.output =~ /.*Deploying module to .*apache.*/, 'Did not log apache deployment')
+    assert(result.output =~ /.*Deploying module to .*stdlib.*/, 'Did not log stdlib deployment')
+  end
+  on(master, "test -f #{test_path}", accept_all_exit_codes: true) do |result|
+    assert(result.exit_code == 0, 'Expected module in Puppetfile was not installed')
+  end
+
+  and_stdlib_is_correct
+end
+
+original_apache_info = JSON.parse(on(master, "cat #{test_path}").stdout)
+
+step 'Test second run of deploy updates control repo, but leaves moduledir untouched' do
+  puppetfile2 =<<-EOS
+    # Current latest of apache is 6.5.1 as of writing this test
+    mod 'puppetlabs/apache', :latest
+    mod 'puppetlabs/stdlib', '8.0.0'
+    mod 'puppetlabs/concat', '7.0.0'
+    EOS
+
+  git_on(master, 'checkout production', control_repo_worktree)
+  create_remote_file(master, "#{control_repo_worktree}/Puppetfile", puppetfile2)
+  git_add_commit_push(master, 'production', 'add Puppetfile for Basic Deployment test', control_repo_worktree)
+
+  on(master, "#{r10k_fqp} deploy environment production --verbose=info") do |result|
+    refute(result.output =~ /.*Deploying module to .*apache.*/, 'Inappropriately updated apache')
+    refute(result.output =~ /.*Deploying module to .*stdlib.*/, 'Inappropriately updated stdlib')
+  end
+
+  on(master, "test -f #{test_path}", accept_all_exit_codes: true) do |result|
+    assert(result.exit_code == 0, 'Expected module content in Puppetfile was inappropriately purged')
+  end
+
+  new_apache_info = JSON.parse(on(master, "cat #{test_path}").stdout)
+  on(master, "cat #{@env_path}/production/Puppetfile | grep ':latest'", accept_all_exit_codes: true) do |result|
+    assert(result.exit_code == 0, 'Puppetfile not updated on subsequent r10k deploys')
+  end
+
+  assert(original_apache_info['version'] == new_apache_info['version'] &&
+         new_apache_info['version'] == '0.10.0',
+         'Module content updated on subsequent r10k invocations w/o providing --modules')
+
+  on(master, "test -f #{@env_path}/production/modules/concat/metadata.json", accept_all_exit_codes: true) do |result|
+    assert(result.exit_code == 1, 'Module content deployed on subsequent r10k invocation w/o providing --modules')
+  end
+
+  and_stdlib_is_correct
+end
+
+step 'Test --modules updates modules' do
+  on(master, "#{r10k_fqp} deploy environment production --modules --verbose=info") do |result|
+    assert(result.output =~ /.*Deploying module to .*apache.*/, 'Did not log apache deployment')
+    assert(result.output =~ /.*Deploying module to .*stdlib.*/, 'Did not log stdlib deployment')
+    assert(result.output =~ /.*Deploying module to .*concat.*/, 'Did not log concat deployment')
+  end
+
+  on(master, "test -f #{test_path}", accept_all_exit_codes: true) do |result|
+    assert(result.exit_code == 0, 'Expected module content in Puppetfile was inappropriately purged')
+  end
+
+  on(master, "test -f #{@env_path}/production/modules/concat/metadata.json", accept_all_exit_codes: true) do |result|
+    assert(result.exit_code == 0, 'New module content was not deployed when providing --modules')
+  end
+
+  new_apache_info = JSON.parse(on(master, "cat #{test_path}").stdout)
+  apache_major_version = new_apache_info['version'].split('.').first.to_i
+  assert(apache_major_version > 5, 'Module not updated correctly using --modules')
+
+  and_stdlib_is_correct
+end
+
+step 'Test --modules --incremental deploys changed & dynamic modules, but not unchanged, static modules' do
+  puppetfile3 =<<-EOS
+    # Current latest of apache is 6.5.1 as of writing this test
+    mod 'puppetlabs/apache', :latest
+    mod 'puppetlabs/stdlib', '8.0.0'
+    mod 'puppetlabs/concat', '7.1.0'
+    EOS
+
+  git_on(master, 'checkout production', control_repo_worktree)
+  create_remote_file(master, "#{control_repo_worktree}/Puppetfile", puppetfile3)
+  git_add_commit_push(master, 'production', 'add Puppetfile for Basic Deployment test', control_repo_worktree)
+
+  on(master, "#{r10k_fqp} deploy environment production --modules --incremental --verbose=debug1") do |result|
+    assert(result.output =~ /.*Deploying module to .*apache.*/, 'Did not log apache deployment')
+    assert(result.output =~ /.*Deploying module to .*concat.*/, 'Did not log concat deployment')
+    assert(result.output =~ /.*Not updating module stdlib, assuming content unchanged.*/, 'Did not log notice of skipping stdlib')
+  end
+
+  on(master, "test -f #{test_path}", accept_all_exit_codes: true) do |result|
+    assert(result.exit_code == 0, 'Expected module content in Puppetfile was inappropriately purged')
+  end
+
+  new_apache_info = JSON.parse(on(master, "cat #{test_path}").stdout)
+  apache_major_version = new_apache_info['version'].split('.').first.to_i
+  assert(apache_major_version > 5, 'Module not updated correctly using --modules & --incremental')
+
+  concat_info = JSON.parse(on(master, "cat #{@env_path}/production/modules/concat/metadata.json").stdout)
+  concat_minor_version = concat_info['version'].split('.')[1].to_i
+  assert(concat_minor_version == 1, 'Module not updated correctly using --modules & --incremental')
+
+  and_stdlib_is_correct
+end
+
+

lib/r10k/action/deploy/environment.rb

@@ -42,7 +42,7 @@ def initialize(opts, argv, settings = {})
                 default_branch_override: @default_branch_override,
                 generate_types: @generate_types || settings.dig(:deploy, :generate_types) || false,
                 preload_environments: true,
-                assume_unchanged: @assume_unchanged
+                incremental: @incremental
               },
               modules: {
                 exclude_spec: settings.dig(:deploy, :exclude_spec),
@@ -239,7 +239,7 @@ def allowed_initialize_opts
           super.merge(puppetfile: :modules,
                       modules: :self,
                       cachedir: :self,
-                      'assume-unchanged': :self,
+                      incremental: :self,
                       'no-force': :self,
                       'exclude-spec': :self,
                       'generate-types': :self,

lib/r10k/cli/deploy.rb

@@ -69,7 +69,7 @@ def self.command
 
           flag :p, :puppetfile, 'Deploy modules (deprecated, use -m)'
           flag :m, :modules, 'Deploy modules'
-          flag nil, :'assume-unchanged', 'Assume previously deployed modules are unchanged'
+          flag nil, :incremental, 'Used with the --modules flag, only update those modules whose definition has changed or whose definition allows the version to float'
           option nil, :'default-branch-override', 'Specify a branchname to override the default branch in the puppetfile',
                  argument: :required
 

lib/r10k/environment/base.rb

@@ -37,7 +37,7 @@ class R10K::Environment::Base
   #   @return [String] The puppetfile name (relative)
   attr_reader :puppetfile_name
 
-  attr_reader :managed_directories, :purge_exclusions, :desired_contents
+  attr_reader :managed_directories, :desired_contents
 
   attr_reader :loader
 
@@ -70,14 +70,14 @@ def initialize(name, basedir, dirname, options = {})
 
     @loader = R10K::ModuleLoader::Puppetfile.new(**loader_options)
 
-    if @overrides.dig(:environments, :assume_unchanged)
+    if @overrides.dig(:environments, :incremental)
       @loader.load_metadata
     end
 
     @base_modules = nil
+    @purge_exclusions = nil
     @managed_directories = [ @full_path ]
     @desired_contents = []
-    @purge_exclusions = []
   end
 
   # Synchronize the given environment.
@@ -204,6 +204,14 @@ def determine_purge_exclusions(pf_managed_dirs     = @puppetfile.managed_directo
     list.to_a
   end
 
+  def purge_exclusions
+    if @purge_exclusions.nil?
+      load_puppetfile_modules
+    end
+
+    @purge_exclusions
+  end
+
   def generate_types!
     argv = [R10K::Settings.puppet_path, 'generate', 'types', '--environment', dirname, '--environmentpath', basedir, '--config', R10K::Settings.puppet_conf]
     subproc = R10K::Util::Subprocess.new(argv)

lib/r10k/module_loader/puppetfile.rb

@@ -48,9 +48,6 @@ def initialize(basedir:,
         @managed_directories = []
         @desired_contents = []
         @purge_exclusions = []
-
-        logger.info _("Using Puppetfile '%{puppetfile}'") % {puppetfile: @puppetfile_path}
-        logger.debug _("Using moduledir '%{moduledir}'") % {moduledir: @moduledir}
       end
 
       def load
@@ -60,6 +57,9 @@ def load
       end
 
       def load!
+        logger.info _("Using Puppetfile '%{puppetfile}'") % {puppetfile: @puppetfile_path}
+        logger.debug _("Using moduledir '%{moduledir}'") % {moduledir: @moduledir}
+
         dsl = R10K::ModuleLoader::Puppetfile::DSL.new(self)
         dsl.instance_eval(puppetfile_content(@puppetfile_path), @puppetfile_path)
 

spec/unit/action/deploy/environment_spec.rb

@@ -63,8 +63,8 @@
       described_class.new({ :'exclude-spec' => true }, [], {})
     end
 
-    it 'can accept an assume-unchanged option' do
-      described_class.new({ :'assume-unchanged' => true }, [], {})
+    it 'can accept an incremental option' do
+      described_class.new({ :incremental => true }, [], {})
     end
 
     describe "initializing errors" do
@@ -127,7 +127,7 @@
       end
     end
 
-    describe "with assume-unchanged flag" do
+    describe "with incremental flag" do
       let(:loader) do
         instance_double("R10K::ModuleLoader::Puppetfile",
                         :load => {
@@ -147,11 +147,11 @@
           and_return(loader).at_least(:once)
       end
 
-      it "assume unchanged flag causes the module definitons to be preloaded by the loader" do
+      it "incremental flag causes the module definitons to be preloaded by the loader" do
         expect(loader).to receive(:load_metadata).exactly(4).times
         action = described_class.new({:config => "/some/nonexistent/path",
                                       :modules => true,
-                                      :'assume-unchanged' => true},
+                                      :incremental => true},
                                       [],
                                       {})
         action.call