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

Conversation

ciprianbadescu
Copy link

@ciprianbadescu ciprianbadescu commented Jan 5, 2021

In PR #149 a call to deprecated method get_puppet_collection
was replaced with a call puppet_collection_for. Unfortunatelly, the new method
does not have the same behavior for puppet_agent package.

This PR attempts to fix the issue

In PR puppetlabs#149 a call to deprecated method `get_puppet_collection`
was replaced with a call `puppet_collection_for`. Unfortunatelly, the new method
does not have the same behavior for puppet_agent package.

This PR attempts to fix the issue
@ciprianbadescu ciprianbadescu force-pushed the maint/puppet-collection-fix branch from 387cde4 to b9c3bfc Compare January 5, 2021 11:05
@ciprianbadescu
Copy link
Author

ciprianbadescu commented Jan 5, 2021

@cthorn42, I tested the code and it looks to fix #155.
Remaining open-point is bellow.

@@ -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.

@ciprianbadescu ciprianbadescu changed the title (maint) attempt to fix puppet_collection_for [do not merge] (maint) attempt to fix puppet_collection_for Jan 5, 2021
@ciprianbadescu ciprianbadescu changed the title [do not merge] (maint) attempt to fix puppet_collection_for (maint) attempt to fix puppet_collection_for Jan 5, 2021
Copy link

@nmburgan nmburgan left a comment

Choose a reason for hiding this comment

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

This has been impacting our PE pipelines for a bit now. Gave this change a test run and it fixes the issue.

@ciprianbadescu
Copy link
Author

Probably this is not needed anymore, closing

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.

4 participants