Skip to content

Conversation

@nmaludy
Copy link
Member

@nmaludy nmaludy commented Feb 18, 2020

When working on revamping the integration tests, i noticed that the PackageCloud module was not idempotent (yuck). Every time Puppet would run it would force a refresh of the Yum and Apt caches causing issues when testing for idempotence.

This PR:

  • Removes the reliance on the PackageCloud module
  • Creates a <module>::repo class that is idiomatic to many of the mainstream Puppet repos
  • Utilize the yumrepo and apt modules to install the appropriate repository for the given OS

@nmaludy nmaludy requested a review from arm4b February 18, 2020 02:39
@nmaludy
Copy link
Member Author

nmaludy commented Feb 18, 2020

Closes #236

@nmaludy nmaludy self-assigned this Feb 18, 2020
@nmaludy nmaludy linked an issue Feb 18, 2020 that may be closed by this pull request
@nmaludy
Copy link
Member Author

nmaludy commented Feb 20, 2020

@armab this one is ready when you have some free time (no rush)

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Do we have idempotence check in CI or is it a strong requirement for Puppet module?

I'm not confident rewrite like that is the best approach. It would be really nice to reuse the original packagecloud module instead of re-writing all the things.

Maybe there is a reason behind the way it works or a bug in PackageCloud module itself?
Would be nice at least to report a bug in their Puppet module.

@nmaludy
Copy link
Member Author

nmaludy commented Feb 21, 2020

@armab let me answer these in order:

  • We do not have an idempotence check right now, but it's easy to do in Litmus + ServerSpec, which is where i'm trying to get it. I"m trying to make that PR less of a "huge blast" and break it up into chunks so it's easier to digest.

  • The original packagecloud module is really terrible and does things in totally the wrong way (non-puppet way). All of the other mainstream modules do it like i did (where i got my inspiration), see links below:

https://github.com/voxpupuli/puppet-nodejs/blob/master/manifests/repo/nodesource.pp
https://github.com/voxpupuli/puppet-mongodb/blob/master/manifests/repo.pp
https://github.com/voxpupuli/puppet-jenkins/blob/master/manifests/repo.pp

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

OK, I made a quick research and computology/computology-packagecloud#26 answers my question with latest changes made to PackageCloud module 3 years ago.

LGTM 👍

@nmaludy nmaludy merged commit 937072e into master Feb 21, 2020
@nmaludy nmaludy deleted the feature/repo-refactor branch February 21, 2020 18:13
bishopbm1 pushed a commit to EncoreTechnologies/puppet-st2 that referenced this pull request May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor st2::profile::repos

3 participants