Skip to content

(PE-43839) Add acceptance test for puppet8 to puppet9 agent upgrade#838

Open
span786 wants to merge 3 commits into
mainfrom
PE-43839-4-update-puppet-acceptance-tests-for-mac-os-26-intel
Open

(PE-43839) Add acceptance test for puppet8 to puppet9 agent upgrade#838
span786 wants to merge 3 commits into
mainfrom
PE-43839-4-update-puppet-acceptance-tests-for-mac-os-26-intel

Conversation

@span786
Copy link
Copy Markdown
Contributor

@span786 span786 commented May 22, 2026

Summary

  • Adds acceptance/tests/test_upgrade_puppet8_to_puppet9.rb, mirroring the existing 7→8 test, with adjustments for the puppet9-nightly track (pre-release 8.99.99.x.gSHA versions, dev_builds routing for newer agent platforms like macOS 26 Intel that have no build at downloads.puppet.com/mac/puppet8/<os>).
  • acceptance/Rakefile: new UPGRADE_TARGET_COLLECTION env-var dispatch so the test selected is decoupled from MASTER_COLLECTION. Needed because puppetserver-9 nightlies aren't yet published, so the master has to stay on puppet8-nightly while still exercising the 8→9 upgrade.
  • acceptance/helpers.rb: fix dormant bug in collection_cmpSymbol#casecmp(String) returns nil on Ruby 3.2, crashing the min:/max: form of require_master_collection. The 7→8 test only uses the String form so this code path was never exercised.

Related: PE-43839

Test plan

  • Validated end-to-end via rake ci against ABS-provisioned puppet8-nightly master + macOS 26 Intel agent — Passed: 1, Failed: 0, Errored: 0
  • Confirmed acceptance/tests/test_upgrade_puppet7_to_puppet8.rb still works (Rakefile dispatch defaults to MASTER_COLLECTION when UPGRADE_TARGET_COLLECTION is unset, preserving existing behaviour)
  • ruby -c syntax-check on the new test file
  • Reviewer: when puppetserver-9 nightlies land at internal_nightly__local/yum, drop UPGRADE_TARGET_COLLECTION and switch MASTER_COLLECTION=puppet9-nightly to test the full puppet9 master path

How to run

cd acceptance
ssh-add ~/.ssh/id_rsa_jenkins   # one-time per session
MASTER_COLLECTION=puppet8-nightly \
UPGRADE_TARGET_COLLECTION=puppet9-nightly \
HOSTS=hosts.yaml \
  bundle exec rake ci

🤖 Generated with Claude Code

Adds tests/test_upgrade_puppet8_to_puppet9.rb modeled on the existing
7→8 test, with adjustments for the puppet9-nightly track:

- require_master_collection min: 'puppet8' (puppet_collection_for strips
  the -nightly suffix, so compare against the bare collection name)
- Version assertion accepts 8.99.99.x.gSHA (Puppet's next-major
  pre-release versioning) alongside the eventual 9.x.y stable form
- curl_passing_sha helper with retries; fails the test rather than
  silently degrading on a VPN blip
- Routes the initial puppet-agent install through dev_builds_url by
  passing the full SHA from passing-agent-SHAs/puppet-agent-8.x, so
  newer agent platforms (e.g. macOS 26) that have no build at
  downloads.puppet.com/mac/puppet8/<os> can still be tested
- Dumps full puppet apply output on non-2 exit (beaker default
  truncates to last 10 lines)

Rakefile: add UPGRADE_TARGET_COLLECTION env-var dispatch so test
selection is decoupled from MASTER_COLLECTION. Lets the master run a
puppet8 puppetserver (since puppetserver-9 nightlies aren't published
yet) while still running the 8→9 upgrade test.

helpers.rb: fix dormant bug in collection_cmp — Symbol#casecmp(String)
returns nil on Ruby 3.2, crashing the min:/max: form of
require_master_collection. The 7→8 test uses the String form so this
code path was never exercised before.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@span786 span786 requested review from a team and bastelfreak as code owners May 22, 2026 15:23
- Replace `lambda do |name| ... end` with `->(name) do ... end`
- Single-quote the non-interpolated `=== end output ===` log line

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Beaker acceptance test to exercise upgrading agents from Puppet 8 to Puppet 9 nightlies, and updates the acceptance harness to select the appropriate upgrade scenario independently of the master’s collection. It also fixes a Ruby 3.2 compatibility issue in collection comparison used by require_master_collection range checks.

Changes:

  • Add test_upgrade_puppet8_to_puppet9.rb, mirroring the existing 7→8 upgrade test with puppet9-nightly specifics and stronger SHA/version fetch handling.
  • Update acceptance/Rakefile to select the upgrade test based on UPGRADE_TARGET_COLLECTION (fallback to MASTER_COLLECTION).
  • Fix collection_cmp in acceptance/helpers.rb by comparing collections as strings (avoids Symbol#casecmp returning nil on Ruby 3.2).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
acceptance/tests/test_upgrade_puppet8_to_puppet9.rb New acceptance test for puppet-agent 8→9 upgrade, including SHA/version lookup and version assertions.
acceptance/Rakefile Add env-var dispatch to choose the correct upgrade test for CI runs.
acceptance/helpers.rb Fix collection comparison to avoid Ruby 3.2 Symbol#casecmp behavior in range-based collection gating.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread acceptance/tests/test_upgrade_puppet8_to_puppet9.rb Outdated
Comment thread acceptance/Rakefile Outdated
- Replace shell-out to `curl` with Ruby `open-uri` for the
  passing-agent-SHAs fetch. `curl --retry-connrefused` isn't supported
  on older curl (e.g. 7.29 on CentOS 7) and silently fails to retry,
  producing a misleading "check VPN" message. The Ruby version retries
  in-process and surfaces the actual error (HTTP status, timeout,
  SocketError) in the failure message.
- Normalize blank env vars in the Rakefile dispatch. An empty string
  is truthy in Ruby, so `ENV['UPGRADE_TARGET_COLLECTION'] ||
  ENV['MASTER_COLLECTION']` would have selected no test when the first
  was set but blank. Now both are `.to_s.strip`'d and treated as unset
  when empty.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants