(MODULES-8348) Acceptance scaffold with beaker-puppet #371

caseywilliams · 2019-01-23T14:29:58Z

Adds an acceptance directory containing a set of scaffold test files
and helpers to allow for testing this module with beaker-puppet instead
of beaker-puppet_install_helper (which is no longer supported). The
README in the acceptance directory describes how to run the new tests.

This leaves the existing tests in spec/acceptance intact for now; in the
future these old tests will either be migrated to the acceptance
directory, or (if they test puppet 3 functionality) removed.

Take note that this process will also eventually remove beaker-rspec,
since it is incompatible with the use of subcommands in beaker 4.

Do not merge until puppetlabs/beaker-puppet#97 has been merged and released. I will need to update the beaker version in the Gemfile once that happens to require a new enough beaker release.

puppetcla · 2019-01-23T17:00:19Z

CLA signed by all contributors.

speedofdark · 2019-01-23T21:17:27Z

acceptance/Rakefile

+
+  beaker("init -h #{hosts_file} -o options.rb")
+  beaker("provision")
+  beaker("exec ./pre_suite")


I found during my work adding only-fails to beaker(-puppet) that if one of the beaker subcommands fails, it will kill the rake run right then and there, so no subsequent beaker subcommands (or indeed anything else in the Rakefile in question) will be executed. This means that provisioned machines will wait for their timeout before they are reaped.

I addressed this by wrapping things with a begin/rescue/ensure block, as illustrated here: https://github.com/puppetlabs/beaker-puppet/blob/1d269589adbb9c47888e47797e63c749133a58a7/tasks/ci.rake#L275-L297

I'm not sure I would want that to happen as a developer using these tests -- I would want to look at the SUTs and see what happened and then manually destroy them -- but I'll add that to the CI task now.

I see your point. Would we want to add the complexity of making that conditional, along the line of the --preserve-hosts flag?

I agree that an option to destroy the hosts would be useful.

This is one of the many places where the beaker-with-subcommands vs. beaker-without-subcommands workflows diverge. --preserve-hosts does not make a lot of sense with subcommands unless you're running the entire test suite with one invocation, since beaker destroy is already available and intended to be called separately. I'm hoping this is something we can refine in subsequent PRs.

Why not call the option destroy-hosts-on-failure? Then it'd be read as

rake prepare --destroy-hosts-on-failure

ekinanp

LGTM for the most part. Should there be a test for puppet4 => puppet6 upgrades?

acceptance/helpers.rb

ekinanp · 2019-01-23T21:26:41Z

acceptance/helpers.rb

+            else
+              agent_install_options = {
+                  puppet_agent_version: initial_package_version_or_collection,
+                  puppet_collection: puppet_collection_for_puppet_agent_version(initial_package_version_or_collection)


Can this be changed to puppet_collection_of? Then things will read as puppet_collection_of(package_version).

these are options passed into beaker-puppet's install_puppet_agent_on directly -- you'd have to change them there.

oh, you meant puppet_collection_for_puppet_agent_version -- same problem, though

ekinanp · 2019-01-23T21:41:04Z

acceptance/helpers.rb

+      # Wraps {Beaker::DSL::PuppetHelpers.with_puppet_running_on} to apply a default
+      # manifest to all nodes and execute a block. Behaves as follows:
+      #
+      # 1. Set up a `site.pp` file in the production environment on the master


How about

apply_node_manifest_on(agents, manifest, master_opts)

That way, this can be something that could be extracted to beaker or beaker-puppet. You should be able to reference the agents' fqdns and construct something like what's specified in https://docs.oracle.com/cd/E53394_01/html/E77676/gqqtf.html#scrolltoc for the site.pp contents

I don't know if a block makes sense here. with_puppet_running_on takes in a block b/c you want to clean-up the state after doing stuff with the running node, but what else can you do here with a site_pp aside from applying it?

Also, it would be better to restore the site.pp after each method call instead of each test so that it can have the flexibility of being used multiple times in the same test.

acceptance/helpers.rb

ekinanp · 2019-01-23T22:15:58Z

acceptance/helpers.rb

+      #
+      # @param [String] initial_package_version_or_collection Either a version
+      #   of puppet-agent or the name of a puppet collection to install the agent from.
+      def prepare_upgrade_with(initial_package_version_or_collection)


prepare_upgrade_with is too specific and should be something left to the tests instead of handled at the helper level. How about

setup_puppet_agent_on(hosts, package_version_or_collection, options = {})

and you can specify the master as an option so that the SSL cert-sign stuff can be conditionally triggered. This way, you ca re-use this helper in the master set-up stage without losing the clarity (since you're basically doing the same thing there).

ekinanp · 2019-01-23T22:29:49Z

acceptance/helpers.rb

+      # @param [String] initial_package_version_or_collection Either a version
+      #   of puppet-agent or the name of a puppet collection to install on agent hosts
+      # @param [String] upgrade_manifest A manifest to apply to all agent nodes
+      def run_foss_upgrade_with_manifest(initial_package_version_or_collection, upgrade_manifest)


I think the code here's short enough and that there are enough useful helpers already that it would be good enough to leave this code to the individual test files, especially when we'll start adding more complicated tests where the preparation's likely going to be more involved (like Windows service stuff).

ekinanp · 2019-01-23T23:10:14Z

acceptance/tests/test_upgrade_puppet5_to_puppet6.rb

+  end
+
+  upgrade_to = '6.0.0'
+  manifest = "class { puppet_agent: package_version => '#{upgrade_to}' }"


It might be useful to add a puppet_agent_manifest helper? Would be nice for tests where we're declaring a lot of parameters.

caseywilliams · 2019-01-24T01:36:03Z

(I've updated with a bugfix from sean and a few of the simpler renaming comments, but the design feedback will take me longer to address)

ekinanp

I'll address my comments in a follow-up PR since this is too foundational to wait on any longer.

caseywilliams · 2019-01-24T19:06:04Z

Ok, I believe this is ready to merge, but first:

We need to release beaker-puppet
I need to update the minimum beaker-puppet version here to match that new version

I'll remove the do not merge label once that's done.

caseywilliams · 2019-01-24T19:26:46Z

Actually, we need to wait until puppetlabs/beaker-puppet#100 is merged, too, since it rewrites a few helpers

caseywilliams · 2019-01-24T19:40:45Z

(ok, updated this code to use the new puppet_collection_for helper)

caseywilliams · 2019-01-25T00:23:55Z

I've updated this PR with a bugfix discussed out-of-band with @ekinanp and manually tested the results with the linked beaker-puppet PR. Once that PR is merged and released, I believe this is ready.

ekinanp · 2019-01-25T01:01:00Z

beaker-puppet PR was merged, now just need to wait on a beaker-puppet release before this can be merged.

Adds an acceptance directory containing a set of scaffold test files and helpers to allow for testing this module with beaker-puppet instead of beaker-puppet_install_helper (which is no longer supported). The README in the acceptance directory describes how to run the new tests. This leaves the existing tests in spec/acceptance intact for now; in the future these old tests will either be migrated to the acceptance directory, or (if they test puppet 3 functionality) removed. Take note that this process will also eventually remove beaker-rspec, since it is incompatible with the use of subcommands in beaker 4.

caseywilliams · 2019-01-25T20:06:04Z

beaker-puppet 1.15.0 has been released with the changes needed for these tests -- I've updated the Gemfile version here and removed the "don't merge" label.

caseywilliams added the don't merge label Jan 23, 2019

caseywilliams force-pushed the MODULES-8348/beaker-puppet-acceptance branch from 04d2b93 to eb450c3 Compare January 23, 2019 18:15

caseywilliams requested a review from mcdonaldseanp January 23, 2019 19:07

mcdonaldseanp approved these changes Jan 23, 2019

View reviewed changes

caseywilliams requested review from ekinanp and speedofdark January 23, 2019 20:53

speedofdark reviewed Jan 23, 2019

View reviewed changes

caseywilliams force-pushed the MODULES-8348/beaker-puppet-acceptance branch from eb450c3 to da23f9d Compare January 23, 2019 21:33

ekinanp suggested changes Jan 23, 2019

View reviewed changes

ekinanp reviewed Jan 23, 2019

View reviewed changes

geoffnichols requested a review from a team January 23, 2019 23:14

geoffnichols approved these changes Jan 23, 2019

View reviewed changes

caseywilliams force-pushed the MODULES-8348/beaker-puppet-acceptance branch from da23f9d to cac0723 Compare January 24, 2019 01:35

caseywilliams force-pushed the MODULES-8348/beaker-puppet-acceptance branch from cac0723 to d12a36e Compare January 24, 2019 01:36

ekinanp approved these changes Jan 24, 2019

View reviewed changes

caseywilliams force-pushed the MODULES-8348/beaker-puppet-acceptance branch 2 times, most recently from 945b8c7 to e800c07 Compare January 24, 2019 18:44

caseywilliams force-pushed the MODULES-8348/beaker-puppet-acceptance branch from e800c07 to d1fb83e Compare January 24, 2019 19:40

caseywilliams force-pushed the MODULES-8348/beaker-puppet-acceptance branch from d1fb83e to 6d365f1 Compare January 25, 2019 00:21

caseywilliams force-pushed the MODULES-8348/beaker-puppet-acceptance branch from 6d365f1 to 10f6331 Compare January 25, 2019 20:04

caseywilliams removed the don't merge label Jan 25, 2019

speedofdark merged commit 13ae763 into puppetlabs:master Jan 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(MODULES-8348) Acceptance scaffold with beaker-puppet #371

(MODULES-8348) Acceptance scaffold with beaker-puppet #371

caseywilliams commented Jan 23, 2019

puppetcla commented Jan 23, 2019

speedofdark Jan 23, 2019 •

edited

Loading

caseywilliams Jan 23, 2019

speedofdark Jan 23, 2019 •

edited

Loading

ekinanp Jan 23, 2019

caseywilliams Jan 23, 2019

ekinanp Jan 23, 2019

ekinanp left a comment

ekinanp Jan 23, 2019

caseywilliams Jan 23, 2019

caseywilliams Jan 23, 2019

ekinanp Jan 23, 2019

ekinanp Jan 23, 2019

ekinanp Jan 23, 2019

ekinanp Jan 23, 2019 •

edited

Loading

caseywilliams commented Jan 24, 2019

ekinanp left a comment •

edited

Loading

caseywilliams commented Jan 24, 2019 •

edited

Loading

caseywilliams commented Jan 24, 2019

caseywilliams commented Jan 24, 2019

caseywilliams commented Jan 25, 2019

ekinanp commented Jan 25, 2019

caseywilliams commented Jan 25, 2019 •

edited

Loading

(MODULES-8348) Acceptance scaffold with beaker-puppet #371

(MODULES-8348) Acceptance scaffold with beaker-puppet #371

Conversation

caseywilliams commented Jan 23, 2019

puppetcla commented Jan 23, 2019

speedofdark Jan 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

speedofdark Jan 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekinanp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekinanp Jan 23, 2019 • edited Loading

Choose a reason for hiding this comment

caseywilliams commented Jan 24, 2019

ekinanp left a comment • edited Loading

Choose a reason for hiding this comment

caseywilliams commented Jan 24, 2019 • edited Loading

caseywilliams commented Jan 24, 2019

caseywilliams commented Jan 24, 2019

caseywilliams commented Jan 25, 2019

ekinanp commented Jan 25, 2019

caseywilliams commented Jan 25, 2019 • edited Loading

speedofdark Jan 23, 2019 •

edited

Loading

speedofdark Jan 23, 2019 •

edited

Loading

ekinanp Jan 23, 2019 •

edited

Loading

ekinanp left a comment •

edited

Loading

caseywilliams commented Jan 24, 2019 •

edited

Loading

caseywilliams commented Jan 25, 2019 •

edited

Loading