(PUP-9653) Refactor tests using uneeded master

acceptance/fixtures/catalog.yaml.erb

@@ -0,0 +1,35 @@
+--- !ruby/object:Puppet::Resource::Catalog
+tags:
+- settings
+name: <%= node_name %>
+version: 1556652267
+code_id:
+catalog_uuid: c552da2c-a0d5-47fe-b3f7-ea42c6e1f940
+catalog_format: 1
+environment: production
+resources:
+- type: Stage
+  title: main
+  tags:
+  - stage
+  exported: false
+- type: Class
+  title: Settings
+  tags:
+  - class
+  - settings
+  exported: false
+- type: Class
+  title: Main
+  tags:
+  - class
+  exported: false
+  parameters:
+    name: main
+edges:
+- source: Stage[main]
+  target: Class[Settings]
+- source: Stage[main]
+  target: Class[Main]
+classes:
+- settings

acceptance/fixtures/puppet_dir.pp.erb

@@ -0,0 +1,57 @@
+File {
+  ensure => directory,
+  mode => '0750',
+}
+
+file {
+  '<%= testdir %>':;
+  '<%= testdir %>/puppet':;
+  '<%= testdir %>/hieradata':;
+  '<%= testdir %>/code':;
+  '<%= testdir %>/code/environments':;
+  '<%= testdir %>/code/environments/production':;
+  '<%= testdir %>/code/environments/production/manifests':;
+  '<%= testdir %>/code/environments/production/modules':;
+}
+
+file { '<%= testdir %>/puppet/puppet.conf':
+  ensure  => file,
+  content => '
+[user]
+environment = production
+[main]
+codedir = <%= testdir %>/code
+environmentpath = <%= testdir %>/code/environments
+hiera_config = <%= testdir %>/hiera.yaml
+',
+}
+
+file { '<%= testdir %>/hiera.yaml':
+  ensure  => file,
+  content => '---
+    :backends:
+      - "yaml"
+    :logger: "console"
+    :hierarchy:
+      - "%{fqdn}"
+      - "%{environment}"
+      - "global"
+    :yaml:
+      :datadir: "<%= testdir %>/hieradata"
+  ',
+  mode    => '0640',
+}
+
+file { '<%= testdir %>/hieradata/global.yaml':
+  ensure  => file,
+  content => "---",
+  mode    => '0640',
+}
+
+file { '<%= testdir %>/code/environments/production/manifests/site.pp':
+  ensure  => file,
+  content => "
+    node default {
+    }",
+  mode    => '0640',
+}

acceptance/lib/puppet/acceptance/environment_utils.rb

@@ -371,39 +371,58 @@ def environmentpath
       end
       module_function :environmentpath
 
-      # create a tmpdir to hold a temporary environment bound by puppet environment naming restrictions
-      # symbolically link environment into environmentpath
-      # we can't use the temp_file utils in our own lib because host.tmpdir violates puppet's naming requirements
-      # in rare cases we want to do this on agents when testing things that use the default manifest
+      # Create a tmpdir to hold a temporary environment bound by puppet
+      # environment naming restrictions.
+      # Copy environment into environmentpath.
+      # We can't use the temp_file utils in our own lib because host.tmpdir
+      # violates puppet's naming requirements.
+      # In rare cases we want to do this on agents when testing things that use
+      # the default manifest.
+      #
+      # @param [Beaker::Host] host
+      # @param [String] environment Prefix for the environment to be created
+      # @returns [String] tmp_environment Name of the created environment
       def mk_tmp_environment_with_teardown(host, environment)
         # add the tmp_environment to a set to ensure no collisions
         @@tmp_environment_set ||= Set.new
-        deadman = 100; loop_num = 0
-        while @@tmp_environment_set.include?(tmp_environment = environment.downcase + '_' + random_string) do
-          break if (loop_num = loop_num + 1) > deadman
+        deadman = 100
+        loop_num = 0
+        while @@tmp_environment_set.include?(tmp_environment = environment.downcase + '_' + random_string)
+          break if (loop_num += 1) > deadman
         end
         @@tmp_environment_set << tmp_environment
-        tmpdir = File.join('','tmp',tmp_environment)
-        on host, "mkdir -p #{tmpdir}/manifests #{tmpdir}/modules; chmod -R 755 #{tmpdir}"
+        tmpdir = File.join('', 'tmp', tmp_environment)
+        env_path = File.join(host.puppet["environmentpath"], tmp_environment)
+        on host, "mkdir -p #{tmpdir}/manifests #{tmpdir}/modules"
 
-        # register teardown to remove the link below
+        # register teardown to remove the environement
         teardown do
-          on host, "rm -rf #{File.join(environmentpath,tmp_environment)}"
+          on host, "rm -rf #{env_path}"
         end
 
-        # WARNING: this won't work with filesync (symlinked environments are not supported)
-        on host, "mkdir -p #{environmentpath}; ln -sf #{tmpdir} #{File.join(environmentpath,tmp_environment)}"
+        on host, "mkdir -p #{host.puppet['environmentpath']}"
+        # The path is copied rather than linked in order to work on Windows
+        on host, "cp -r #{tmpdir} #{env_path}"
+        on host, "chmod -R 755 #{tmpdir} #{env_path}"
         return tmp_environment
       end
       module_function :mk_tmp_environment_with_teardown
 
-      # create sitepp in a tmp_environment as created by mk_tmp_environment_with_teardown
+      # Create site.pp in a tmp_environment as created by
+      # mk_tmp_environment_with_teardown.
+      #
+      # @param [Beaker::Host] host
+      # @param [String] tmp_environment Name of the environment
+      # @param [String] file_content Contents to be placed in site.pp
       def create_sitepp(host, tmp_environment, file_content)
-        file_path = File.join('','tmp',tmp_environment,'manifests','site.pp')
-        create_remote_file(host, file_path, file_content)
-        on host, "chmod -R 755 #{file_path}"
+        tmp_file_path = File.join("", "tmp", tmp_environment, "manifests", "site.pp")
+        env_file_path = File.join(host.puppet["environmentpath"],
+                                  tmp_environment, "manifests", "site.pp")
+        create_remote_file(host, tmp_file_path, file_content)
+        # The path is copied rather than linked in order to work on Windows
+        on host, "cp -r #{tmp_file_path} #{env_file_path}"
+        on host, "chmod -R 755 #{tmp_file_path} #{env_file_path}"
       end
-
     end
   end
 end

acceptance/tests/agent/agent_disable_lockfile.rb

@@ -1,100 +1,87 @@
-test_name "C4553 - agent --disable/--enable functionality should manage the agent lockfile properly"
-confine :except, :platform => 'cisco_nexus' #See BKR-749
+test_name "C4553 - agent --disable/--enable functionality should manage the agent lockfile properly" do
+  confine :except, platform: "cisco_nexus" # See BKR-749
 
-tag 'audit:integration', # lockfile uses the standard `vardir` location to store/query lockfile.
-                         # The validation of the `vardir` at the OS level
-                         # should be accomplished in another test.
-    'audit:medium',
-    'audit:refactor'     # This test should not require a master. Remove the use of `with_puppet_running_on`.
+  require "json"
 
-#
-# This test is intended to ensure that puppet agent --enable/--disable
-#  work properly, both in terms of complying with our public "API" around
-#  lockfile semantics ( http://links.puppet.com/agent_lockfiles ), and
-#  in terms of actually restricting or allowing new agent runs to begin.
-#
+  tag 'audit:integration', # lockfile uses the standard `vardir` location to store/query lockfile.
+                           # The validation of the `vardir` at the OS level
+                           # should be accomplished in another test.
+      'audit:medium'
 
-require 'puppet/acceptance/temp_file_utils'
+  #
+  #  This test is intended to ensure that puppet agent --enable/--disable
+  #  work properly.
+  #
 
-extend Puppet::Acceptance::TempFileUtils
+  require "puppet/acceptance/temp_file_utils"
 
-initialize_temp_dirs()
-@all_tests_passed = false
+  extend Puppet::Acceptance::TempFileUtils
 
+  initialize_temp_dirs
+  @all_tests_passed = false
 
-###############################################################################
-# BEGIN TEST LOGIC
-###############################################################################
+  ###############################################################################
+  # BEGIN TEST LOGIC
+  ###############################################################################
 
-teardown do
-  if @all_tests_passed then
-    remove_temp_dirs()
-  end
-  agents.each do |agent|
-    on(agent, puppet('agent', "--enable"))
+  teardown do
+    remove_temp_dirs if @all_tests_passed
+    agents.each do |agent|
+      on(agent, puppet("agent", "--enable"))
+    end
   end
-end
 
-tuples = [
+  fakemaster = ('a'..'z').to_a.shuffle[0,10].join
+
+  tuples = [
     ["reason not specified", false],
     ["I'm busy; go away.'", true]
-]
+  ]
 
-with_puppet_running_on(master, {}) do
   tuples.each do |expected_message, explicitly_specify_message|
-    step "disable the agent; specify message? '#{explicitly_specify_message}', message: '#{expected_message}'" do
-      agents.each do |agent|
-        if (explicitly_specify_message)
-          on(agent, puppet('agent', "--disable \"#{expected_message}\""))
+    agents.each do |agent|
+      step "disable the agent; specify message? '#{explicitly_specify_message}', message: '#{expected_message}'" do
+        if explicitly_specify_message
+          on(agent, puppet("agent", "--disable \"#{expected_message}\""))
         else
-          on(agent, puppet('agent', "--disable"))
+          on(agent, puppet("agent", "--disable"))
         end
 
         agent_disabled_lockfile = "#{agent.puppet['vardir']}/state/agent_disabled.lock"
-        unless file_exists?(agent, agent_disabled_lockfile) then
-          fail_test("Failed to create disabled lock file '#{agent_disabled_lockfile}' on agent '#{agent}'")
-        end
+        assert(agent.file_exist?(agent_disabled_lockfile),
+               "Failed to create disabled lock file '#{agent_disabled_lockfile}' on agent '#{agent}'")
+
         lock_file_content = file_contents(agent, agent_disabled_lockfile)
 
-        # This is a hack; we should parse the JSON into a hash, but I don't
-        # think I have a library available from the acceptance test framework
-        # that I can use to do that.  So I'm falling back to <gasp> regex.
-        lock_file_content_regex = /"disabled_message"\s*:\s*"#{expected_message}"/
-        unless lock_file_content =~ lock_file_content_regex
-          fail_test("Disabled lock file contents invalid; expected to match '#{lock_file_content_regex}', got '#{lock_file_content}' on agent '#{agent}'")
-        end
+        file_lock_json = JSON.parse(lock_file_content)
+        assert_equal(expected_message, file_lock_json["disabled_message"])
       end
-    end
 
-    step "attempt to run the agent (message: '#{expected_message}')" do
-      agents.each do |agent|
-        on(agent, puppet('agent', "--test --server #{master}"),
-                     :acceptable_exit_codes => [1]) do
+      step "attempt to run disabled agent (message: '#{expected_message}')" do
+        skip_test "unable to assert match in Japanese" if agent["locale"] == "ja"
+        on(agent, puppet("agent", "--test --server #{fakemaster}"),
+           acceptable_exit_codes: [1]) do |result|
           disabled_regex = /administratively disabled.*'#{expected_message}'/
-          unless result.stdout =~ disabled_regex
-            fail_test("Unexpected output from attempt to run agent disabled; expecting to match '#{disabled_regex}', got '#{result.stdout}' on agent '#{agent}'") unless agent['locale'] == 'ja'
-          end
+          assert_match(disabled_regex, result.stdout)
         end
       end
-    end
-
-    step "enable the agent (message: '#{expected_message}')" do
-      agents.each do |agent|
 
+      step "enable the agent (message: '#{expected_message}')" do
         agent_disabled_lockfile = "#{agent.puppet['vardir']}/state/agent_disabled.lock"
-        on(agent, puppet('agent', "--enable"))
-        if file_exists?(agent, agent_disabled_lockfile) then
-          fail_test("Failed to remove disabled lock file '#{agent_disabled_lockfile}' on agent '#{agent}'")
-        end
+        on(agent, puppet("agent", "--enable"))
+        assert_equal(agent.file_exist?(agent_disabled_lockfile), false,
+                     "Failed to remove disabled lock file '#{agent_disabled_lockfile}' on agent '#{agent}'")
       end
-    end
 
-    step "verify that we can run the agent (message: '#{expected_message}')" do
-      agents.each do |agent|
-        on(agent, puppet('agent', "--test --server #{master}"))
+      step "verify that agent is enabled (message: '#{expected_message}')" do
+        # Agent should attempt to run, but fail to connect to fakemaster
+        on(agent, puppet("agent", "--test --server #{fakemaster}"),
+           acceptable_exit_codes: [1]) do |result|
+          assert_match(/Failed to open TCP connection/, result.stdout)
+        end
       end
     end
   end # tuples block
-end # with_puppet_running_on block
 
-@all_tests_passed = true
+  @all_tests_passed = true
+end

acceptance/tests/agent/fallback_to_cached_catalog.rb

@@ -1,22 +1,32 @@
-test_name "fallback to the cached catalog"
+test_name "fallback to the cached catalog" do
+  tag 'audit:medium',
+      'audit:integration' # This test is not OS sensitive.
 
-tag 'audit:medium',
-    'audit:integration', # This test is not OS sensitive.
-    'audit:refactor'     # A catalog fixture can be used for this test. Remove the usage of `with_puppet_running_on`.
+  fixture_dir = File.expand_path(File.join(File.dirname(__FILE__),
+                                           "../../fixtures"))
 
-step "run agents once to cache the catalog" do
-  with_puppet_running_on master, {} do
-    on(agents, puppet("agent -t --server #{master}"))
-  end
-end
-
-step "run agents again, verify they use cached catalog" do
   agents.each do |agent|
-    # can't use --test, because that will set usecacheonfailure=false
-    # We use a server that the agent can't possibly talk to in order
-    # to guarantee that no communication can take place.
-    on(agent, puppet("agent --onetime --no-daemonize --server puppet.example.com --verbose")) do |result|
-      assert_match(/Using cached catalog/, result.stdout) unless agent['locale'] == 'ja'
+    skip_test("cannot assert regex match for Japanese") if agent["locale"] == "ja"
+    step "seed catalog cache with fixture" do
+      node_name = agent.node_name
+      catalogdir = File.join(agent.puppet["vardir"], "client_yaml/catalog")
+      catalogfile = File.join(catalogdir, "#{node_name}.yaml")
+      template = File.read("#{fixture_dir}/catalog.yaml.erb")
+      renderer = ERB.new(template)
+      catalog = renderer.result(binding)
+
+      agent.mkdir_p(catalogdir)
+      create_remote_file(agent, catalogfile, catalog, mkdirs: true)
+    end
+
+    step "run agents again, verify they use cached catalog" do
+      # can't use --test, because that will set usecacheonfailure=false
+      # We use a server that the agent can't possibly talk to in order
+      # to guarantee that no communication can take place.
+      on(agent, puppet("agent --onetime --no-daemonize --verbose",
+                       "--server puppet.example.com --verbose")) do |result|
+        assert_match(/Using cached catalog/, result.stdout)
+      end
     end
   end
 end