Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(maint) attempt to fix puppet_collection_for #156

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/beaker-puppet/install_utils/foss_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1361,6 +1361,8 @@ def install_puppet_agent_pe_promoted_repo_on( hosts, opts )
opts[:copy_base_local] ||= File.join('tmp', 'repo_configs')
opts[:copy_dir_external] ||= host.external_copy_base
opts[:puppet_collection] ||= puppet_collection_for(:puppet_agent, opts[:puppet_agent_version])
opts[:puppet_collection] = 'PC1' if opts[:puppet_collection] == 'pc1'

add_role(host, 'aio') #we are installing agent, so we want aio role
release_path = opts[:download_url]
variant, version, arch, codename = host['platform'].to_array
Expand Down
2 changes: 2 additions & 0 deletions lib/beaker-puppet/install_utils/puppet_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ def puppet_collection_for(package, version)

return 'pc1' if x == pc1_x[package]

return 'pc1' if package == :puppet_agent && x > 4 && version_is_less(version, "5.5.4")
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks odd to me. I always thought pc1 was the Puppet 4 package and puppet5 was, well, Puppet 5.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is odd, I added [do no merge] until we clarify it, but this is what the deprecated method has:
https://github.com/voxpupuli/beaker-puppet/blob/b9c3bfcf8f51063805d31f5810737fa4e9f0adb8/lib/beaker-puppet/install_utils/puppet_utils.rb#L78-L88

Copy link

@nmburgan nmburgan Jan 27, 2021

Choose a reason for hiding this comment

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

Looks like pre-5.5.4, it still put it in a PC1 dir rather than puppet5 (this is with the current version of beaker-puppet, not with this fix)

  Fetching: http://pm.puppet.com/puppet-agent/2018.1.2/5.5.3/repos/puppet-agent-el-7-x86_64.tar.gz
    and saving to tmp/repo_configs/el/puppet-agent-el-7-x86_64.tar.gz
  localhost $ scp tmp/repo_configs/el/puppet-agent-el-7-x86_64.tar.gz redhat7-64-1:/root {:ignore => }

  slick-clapping.delivery.puppetlabs.net (redhat7-64-1) 22:10:58$ tar -zxvf /root/puppet-agent-el-7-x86_64.tar.gz -C /root
    repos/el/7/PC1/x86_64/
    repos/el/7/PC1/x86_64/repodata/
    repos/el/7/PC1/x86_64/repodata/2c10e8a7bb56d5d68606639c5b80d9aea7d65579-primary.sqlite.bz2
    repos/el/7/PC1/x86_64/repodata/0cd92b1be16ff703e099af9954b9a68911c83144-other.xml.gz
    repos/el/7/PC1/x86_64/repodata/82ce6e9701b12414242e2ed9c4348df4425e7b88-filelists.sqlite.bz2
    repos/el/7/PC1/x86_64/repodata/c0acc73930c8ef2d8c4540e48b0c85cc84e3e3ae-other.sqlite.bz2
    repos/el/7/PC1/x86_64/repodata/0d09db23006fa5dfdb992733d7428894f847a3cc-filelists.xml.gz
    repos/el/7/PC1/x86_64/repodata/repomd.xml
    repos/el/7/PC1/x86_64/repodata/e9f06eb1bd5c2e131372eab016aada323084fccb-primary.xml.gz
    repos/el/7/PC1/x86_64/repodata/repomd.xml.asc
    repos/el/7/PC1/x86_64/puppet-agent-5.5.3-1.el7.x86_64.rpm

  slick-clapping.delivery.puppetlabs.net (redhat7-64-1) executed in 0.15 seconds

  slick-clapping.delivery.puppetlabs.net (redhat7-64-1) 22:10:58$ yum --nogpgcheck localinstall -y /root/repos/el/7/puppet5/x86_64/puppet-agent-*.rpm
    Loaded plugins: product-id, search-disabled-repos, subscription-manager
    This system is not registered to Red Hat Subscription Management. You can use subscription-manager to register.
    Cannot open: /root/repos/el/7/puppet5/x86_64/puppet-agent-*.rpm. Skipping.
    Nothing to do

Copy link
Author

Choose a reason for hiding this comment

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

@ekohl, since some earlier versions of puppet 5 are delivered in PC1, the code from this PR should be good.
Is there anything else needed to get this PR merged and release a new beaker-puppet?
Thanks,
C

Copy link
Contributor

Choose a reason for hiding this comment

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

@ekohl, since some earlier versions of puppet 5 are delivered in PC1, the code from this PR should be good.

It looks very weird and really something internal to Puppet. On https://yum.puppet.com/el/7/PC1/x86_64/ I don't see any Puppet 5 packages. https://yum.puppet.com/puppet5/el/7/x86_64/ does contain version 5.0.0 and newer so isn't this really a bug on Puppet's internal delivery system? From end user perspective this patch looks incorrect.

Small rant: I keep being surprised (in a negative way) by Puppet's delivery pipeline and packaging. As a user, the packaging is really low quality to the point that I generally prefer distro packages, even if they're outdated. Sadly puppetserver is not packaged by any distro so that's where I do use the official packages.


# A y version >= 99 indicates a pre-release version of the next x release
x += 1 if y >= 99
"puppet#{x}" if x > 4
Expand Down
6 changes: 3 additions & 3 deletions spec/beaker-puppet/install_utils/foss_utils_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1424,10 +1424,10 @@ def test_fetch_http_file_no_ending_slash(platform)
subject.install_puppet_agent_pe_promoted_repo_on( host, opts )
end

it 'sets correct file paths for agent version 1.x.x' do
it 'sets correct file paths for agent version < 5.5.4' do
host['platform'] = platform
agentversion = '1.x.x'
collection = 'pc1'
agentversion = '5.3.3'
collection = 'PC1'
opts = { :puppet_agent_version => "#{agentversion}" , :pe_promoted_builds_url => "#{downloadurl}"}

expect(subject).to receive(:fetch_http_file).once.with(
Expand Down
6 changes: 4 additions & 2 deletions spec/beaker-puppet/install_utils/puppet_utils_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,10 @@ def logger
{
'1.10.14' => 'pc1',
'1.10.x' => 'pc1',
'5.3.1' => 'puppet5',
'5.3.x' => 'puppet5',
'5.3.1' => 'pc1',
'5.3.x' => 'pc1',
'5.5.3' => 'pc1',
'5.5.4' => 'puppet5',
'5.99.0' => 'puppet6',
'6.1.99-foo' => 'puppet6',
'6.99.99' => 'puppet7',
Expand Down