(PUP-8305) Git testing should consume the runtime package by melissa · Pull Request #52 · puppetlabs/beaker-puppet

melissa · 2018-06-26T22:43:48Z

No description provided.

melissa · 2018-06-26T22:47:52Z

@joshcooper @jhelwig this is the current rough first draft of what I've been working on. Currently this works for debian8, fedora 27, solaris 11, solaris 10, windows 2012r2, osx 10.12, osx 10.13. and sles 12. I definitely want to iron out some of the logic here, since it's redundant and quite ugly.

melissa · 2018-06-26T23:02:51Z

windows2012r2_ja failed though, I haven't looked into it yet

/Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:447:in `count': invalid byte sequence in UTF-8 (ArgumentError)
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:447:in `binary?'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:316:in `visit_String'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:163:in `accept'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:593:in `block in dump_ivars'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:591:in `each'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:591:in `dump_ivars'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:190:in `visit_Object'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:163:in `accept'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:593:in `block in dump_ivars'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:591:in `each'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:591:in `dump_ivars'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:190:in `visit_Object'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:163:in `accept'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:513:in `block in visit_hash_subclass'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:511:in `each'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:511:in `visit_hash_subclass'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:389:in `visit_Hash'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:163:in `accept'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych/visitors/yaml_tree.rb:127:in `push'
        from /Users/melissa/.rbenv/versions/2.4.0/lib/ruby/2.4.0/psych.rb:416:in `dump'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/beaker-3.36.0/lib/beaker/cli.rb:245:in `block in preserve_hosts_file'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/beaker-3.36.0/lib/beaker/cli.rb:244:in `open'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/beaker-3.36.0/lib/beaker/cli.rb:244:in `preserve_hosts_file'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/beaker-3.36.0/lib/beaker/cli.rb:185:in `execute!'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/beaker-3.36.0/lib/beaker/subcommand.rb:202:in `exec'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/thor-0.20.0/lib/thor/command.rb:27:in `run'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in `invoke_command'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/thor-0.20.0/lib/thor.rb:387:in `dispatch'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/thor-0.20.0/lib/thor/base.rb:466:in `start'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/gems/beaker-3.36.0/bin/beaker:7:in `<top (required)>'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/bin/beaker:22:in `load'
        from /Users/Melissa/puppet/acceptance/.bundle/ruby/2.4.0/bin/beaker:22:in `<main>'
rake aborted!

melissa · 2018-06-26T23:31:20Z

+
+runtime_url = "#{dev_builds_url}/puppet-runtime/#{runtime_tag}/artifacts/"
+runtime_prefix = "agent-runtime-#{branch}-#{runtime_tag}."
+runtime_suffix = ".tar.gz"


I want to use artifactory to give us this information rather than constructing it here, but I haven't figured out the best way to do that yet

$ jfrog rt s generic__local/*agent-runtime*master*201806220*windows*.tar.gz [Info] Searching artifacts... [Info] Found 2 artifacts. [ { "path": "generic__local/development/puppet-runtime/201806220/windows-x64/agent-runtime-master-201806220.windows-2012r2-x64.tar.gz", "props": { "": "" } }, { "path": "generic__local/development/puppet-runtime/201806220/windows-x86/agent-runtime-master-201806220.windows-2012r2-x86.tar.gz", "props": { "": "" } } ]

melissa · 2018-07-06T17:42:49Z

            :solaris_11    => /solaris-11/,
            :windows       => /windows/,
            :eos           => /^eos-/,
+            :sles          => /sles/,


not sure if we still need this since I fixed up how defaults are loaded with voxpupuli/beaker#1532

Can confirm, we need this

Yes: https://github.com/puppetlabs/beaker-puppet/blame/24cf1cdac0a1bee2f9ab7b7c2cb1fdb7c11b5c8d/lib/beaker-puppet/install_utils/foss_utils.rb#L1417

melissa · 2018-07-06T23:04:59Z

        # @param [Host] host    A single host to construct pathing for
        def construct_puppet_path(host)
-          path = (%w(puppetbindir facterbindir hierabindir)).compact.reject(&:empty?)
+          path = (%w(puppetbindir facterbindir hierabindir privatebindir)).compact.reject(&:empty?)


we might not need this, but I have to double check

I think we do need this

melissa · 2018-07-06T23:05:52Z

+      on host, 'pkgadd -d http://get.opencsw.org/now -a /root/shutupsolaris -n all'
+      on host, '/opt/csw/bin/pkgutil -U all'
+      on host, '/opt/csw/bin/pkgutil -y -i pkgutil'
+      on host, '/opt/csw/bin/pkgutil -y -i coreutils'


we may not need pkgutil or coreutils for solaris

do not need these

melissa · 2018-07-06T23:06:17Z

-  when /windows/
-    arch = host[:ruby_arch] || 'x86'
-    step "#{host} Selected architecture #{arch}"
+step "Install puppet-runtime" do


this whole section is gross and probably should get cleaned up

I'd love to use artifactory to fetch the runtime package, but I think we should save that for the next iteration

Ticket opened to follow up on this, so it's not going to happen as a part of this PR

melissa · 2018-07-06T23:07:16Z

-          # note that this WILL NOT impact Beaker runs though
-          puppet_bundler_install_dir = on(host, "cd #{puppet_dir} && cmd.exe /c bundle show puppet").stdout.chomp
-        when /el-7/
-          create_remote_file(host, "#{puppet_dir}/Gemfile", gemfile_contents + "gem 'json'\n")


I couldn't find any detail around why we needed to add the json gem for el7, so I removed it, and nothing broke...

👍 The ruby package provided by the distro oddly didn't have json, so we had to add it back in. But now that we're using the runtime, we don't have to worry about that nonsense.

melissa · 2018-07-06T23:07:56Z

-          create_remote_file(host, "#{puppet_dir}/Gemfile", gemfile_contents)
-          on host, "cd #{puppet_dir} && bundle install --system --binstubs #{host['puppetbindir']} --shebang #{host['puppetbindir']}/ruby"
-          puppet_bundler_install_dir = on(host, "cd #{puppet_dir} && bundle show puppet").stdout.chomp
+          on host, "cd #{puppet_dir} && #{bundle_command(host)} install --system --binstubs #{host['puppetbindir']} --shebang #{host['puppetbindir']}/ruby"


I'm not convinced we need to branch here for Solaris, but I haven't gotten around to testing that theory out yet

can confirm, do not need

melissa · 2018-07-12T20:35:18Z

This failure is very strange, but I'm happy with the work at this point if anyone wants to start reviewing it.

12:58:54 Fetching upstream changes from git@github.com:puppetlabs/beaker-puppet.git
12:58:54  > git fetch --tags --progress git@github.com:puppetlabs/beaker-puppet.git +refs/pull/*:refs/remotes/origin/pr/*
12:58:54 Checking out Revision 28fe7d3f471051df260f2b8bc87530383430b742 (detached)
12:58:54  > git config core.sparsecheckout # timeout=10
12:58:54  > git checkout -f 28fe7d3f471051df260f2b8bc87530383430b742
12:58:54 FATAL: Could not checkout 28fe7d3f471051df260f2b8bc87530383430b742
12:58:54 hudson.plugins.git.GitException: Command "git checkout -f 28fe7d3f471051df260f2b8bc87530383430b742" returned status code 128:
12:58:54 stdout: 
12:58:54 stderr: fatal: reference is not a tree: 28fe7d3f471051df260f2b8bc87530383430b742

kevpl · 2018-07-16T16:44:14Z

@melissa git failures pop up on occasion for us after force pushes, but tend to resolve themselves one or two builds later. Re-kicking this now.

Jenkins, test this please

jhelwig · 2018-07-24T21:25:47Z


-# override incorrect FOSS (git) defaults from Beaker with AIO applicable ones
-#
-# Remove after PUP-4867 breaks distmoduledir and sitemoduledir into individual


Guessing the comment about PUP-4867 and the related code became irrelevant due to other refactoring?

previously we were using the moduledirs defined in the foss defaults file, but we're not anymore with https://github.com/puppetlabs/beaker-puppet/pull/52/files#diff-7e0996ffe23c8cf4e3bc5558f32f7aa8L24

This will load the aio defaults, so we don't need to keep overwriting them here. I'm actually not sure how PUP-4867 relates to this, other than they both deal with the moduledir.... but by changing which defaults are loaded for git testing, we no longer have to override distmoduledir or sitemoduledir

jhelwig · 2018-07-24T21:32:45Z

+  `git clone --depth 1 git@github.com:puppetlabs/puppet-runtime.git #{runtime_dir}`
+  runtime_tag = nil
+  Dir.chdir runtime_dir do
+    runtime_tag = `git describe --abbrev=0`.chomp


Might want to use --first-parent here. There's been some discussion about merge-ups from older branches causing issues with git-describe, where describe will pick up the "older" branch because it happens "first" in the history (Eg: Getting a 5.5.x related tag on the master branch).

--first-parent Follow only the first parent commit upon seeing a merge commit. This is useful when you wish to not match tags on branches merged in the history of the target commit.

--first-parent doesn't really get me what I want here. I'm looking for the last tag on the branch

[0] Melissa@bubba:~/beaker-puppet:(maint/master/git-based-acceptance)$ git describe 0.16.0-2-g078284b [0] Melissa@bubba:~/beaker-puppet:(maint/master/git-based-acceptance)$ git describe --first-parent 0.16.0-2-g078284b [0] Melissa@bubba:~/beaker-puppet:(maint/master/git-based-acceptance)$ git describe --abbrev=0 0.16.0

So I want 0.16.0 here, without the -2-g078284b

I think you want both --abbrev and --first-parent.

ooooh, yeah, that makes more sense. Updated!

joshcooper · 2018-08-07T22:52:17Z

        # we assume that an invocation with `puppet()` will have it's first argument
        # a face or sub command
-        if @options[:type] == 'git'
+        if @options && @options[:type] == 'git'


We can drop this commit if we don't install the runtime package on the master node

joshcooper · 2018-08-07T22:56:00Z

        end

+        def ruby_command(host, type = 'aio')
+          get_command('ruby', host, type)


This will cause us to execute cmd /c ruby on windows which I guess works. Normally we only need to wrap cmd for puppet batch files due to the way that environment.bat sets up the PATH.

We were doing this for bundler in some cases, and gem, so I figured it wouldn't be a bad idea to add it for consistency with these different commands? Do you think it'd be better to change it back?

joshcooper · 2018-08-07T22:56:53Z

          when /(\A|-)pe(\Z|-)/
            'pe'
-          when /(\A|-)aio(\Z|-)/
+          when /(\A|-)(git)|(aio)(\Z|-)/


I think we can avoid doing this and conflating git/aio by calling these methods directly:

add_aio_defaults_on(hosts) add_puppet_paths_on(hosts)

joshcooper · 2018-08-07T22:57:56Z

+          cmd = "bundle exec puppet #{args.shift}"
+        else
+          cmd = "puppet #{args.shift}"
+        end


So long as we don't install the runtime on the master we can skip this (as there won't be confusion about which puppet to run). Also the puppet-agent package won't clobber the untarred runtime.

Should we not install the runtime on a host that's defined as both a master and an agent?

melissa · 2018-09-07T17:33:01Z

It looks like things were broken. I'm picking this up at intervals between other work, and it's been a while since I've touched it. It'd like to take another close look at this before it moves forward, but it's only blocked on getting my attention, not other work

This commit includes a lot of changes that enable git-based acceptance testing. First and foremost, we're satisfying runtime dependencies with the new puppet-runtime tarball. This provides a host of third party packages, namely ruby. Once this is in place, we can remove quite a bit of unnecessary logic that was previously required when using system-provided packages (or hand built packages). This also changes which defaults are loaded for git-based testing. Previously, we used the foss defaults. We need to switch to using the same modern paths that are set with the aio defaults.

Apparently there are some potential issues with `git describe` where it will potentially pick up the "older" branch because it happens "first" in the history (i.e., from a merge-up). This will ensure we stay on the branch we have checked out.

kevpl · 2018-09-24T16:53:39Z

last comment still current

melissa · 2018-09-24T17:20:02Z

I picked this up again recently, so it's currently in progress.

gotta support both puppet 5.5.x and puppet 6 :(

This reverts commit 2add620.

This reverts commit 524601e.

mchllweeks · 2018-10-08T16:43:26Z

last comment still current

This reverts commit 02e9ff7.

…" (#74) This reverts commit 02e9ff7.

melissa commented Jun 26, 2018

View reviewed changes

melissa force-pushed the maint/master/git-based-acceptance branch from 9847b4d to 4d50b2a Compare July 6, 2018 17:42

melissa commented Jul 6, 2018

View reviewed changes

melissa force-pushed the maint/master/git-based-acceptance branch from 494737f to 28fe7d3 Compare July 6, 2018 21:30

melissa added the blocked label Jul 6, 2018

melissa force-pushed the maint/master/git-based-acceptance branch from 28fe7d3 to f516887 Compare July 6, 2018 23:03

melissa requested review from jhelwig, joshcooper and kevpl July 6, 2018 23:03

melissa commented Jul 6, 2018

View reviewed changes

melissa changed the title ~~(maint) Use the runtime package for git-based acceptance~~ (PUP-8305) Git testing should consume the runtime package Jul 6, 2018

melissa force-pushed the maint/master/git-based-acceptance branch from f516887 to 6267254 Compare July 12, 2018 19:57

melissa removed the blocked label Jul 12, 2018

melissa closed this Jul 13, 2018

melissa reopened this Jul 13, 2018

jhelwig reviewed Jul 24, 2018

View reviewed changes

jhelwig approved these changes Jul 24, 2018

View reviewed changes

mchllweeks approved these changes Jul 30, 2018

View reviewed changes

joshcooper reviewed Aug 7, 2018

View reviewed changes

melissa added 7 commits September 14, 2018 14:46

(maint) We do not always have @options set when running commands

3b85d93

(maint) Cleanup type regex

f5a1c72

updates

eb31446

Setup git-based projects only on agents

474930d

update setup task paths

260dcaf

melissa added 8 commits September 25, 2018 12:00

(maint) Install windows into the expected path

6a9b50c

remove pry

3e51102

(maint) Ensure state is preserved

29dfa6e

(maint) Go back to finding the latest runtime tag

cc4642f

wip

2eb9c3e

windows path changes

8dec4ba

gotta support both puppet 5.5.x and puppet 6 :(

(maint) remove byebug call

905ac2e

do we need these lines?

f3f80e1

melissa force-pushed the maint/master/git-based-acceptance branch from f659649 to f3f80e1 Compare October 3, 2018 23:58

melissa mentioned this pull request Oct 4, 2018

(PUP-8305) Enable git-based acceptance testing puppetlabs/puppet#7142

Merged

melissa added 5 commits October 4, 2018 16:14

(maint) Remove calls to install.rb

2add620

we shouldn't need this anymore

8984146

Revert "(maint) Remove calls to install.rb"

524601e

This reverts commit 2add620.

Revert "Revert "(maint) Remove calls to install.rb""

dbb621b

This reverts commit 524601e.

add RUBYLIB so we can load puppet via ruby/irb

22194ac

kevpl merged commit 02e9ff7 into puppetlabs:master Oct 8, 2018

kevpl added a commit that referenced this pull request Oct 8, 2018

Revert "(PUP-8305) Git testing should consume the runtime package (#52)"

f811070

This reverts commit 02e9ff7.

kevpl mentioned this pull request Oct 8, 2018

Revert "(PUP-8305) Git testing should consume the runtime package" #74

Merged

kevpl added a commit that referenced this pull request Oct 8, 2018

Revert "(PUP-8305) Git testing should consume the runtime package (#52)…

56ec8ed

…" (#74) This reverts commit 02e9ff7.

Conversation

melissa commented Jun 26, 2018

Uh oh!

melissa commented Jun 26, 2018

Uh oh!

melissa commented Jun 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melissa commented Jul 12, 2018

Uh oh!

kevpl commented Jul 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhelwig Jul 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melissa commented Sep 7, 2018

Uh oh!

kevpl commented Sep 24, 2018

Uh oh!

melissa commented Sep 24, 2018

Uh oh!

mchllweeks commented Oct 8, 2018

Uh oh!

Reviewers

Assignees

Labels

jhelwig Jul 24, 2018 •

edited

Loading