Use modern os facts #964
Use modern os facts #964
Conversation
Codecov Report
@@ Coverage Diff @@
## main #964 +/- ##
==========================================
- Coverage 72.24% 71.86% -0.39%
==========================================
Files 5 5
Lines 263 263
==========================================
- Hits 190 189 -1
- Misses 73 74 +1
Continue to review full report at Codecov.
|
kenyon
force-pushed
the
use-modern-os-facts
branch
from
dfa2f49 to
00feadf
Compare
|
I don't see why code coverage would change. 🤷♂️ |
|
@kenyon I raised https://tickets.puppetlabs.com/browse/FACT-2906 to get lsb packages as dependency added |
kenyon
force-pushed
the
use-modern-os-facts
branch
2 times, most recently
from
e7ff3d0 to
f7d27cb
Compare
|
🛠️ rebased 🌪️ |
|
Hi @kenyon, This PR looks great, but can you please squash the commits and then I'll merge it. Thanks |
It's a class, not a defined type. I missed this in puppetlabs#962
Per https://github.com/camptocamp/facterdb/blob/master/facts/3.14/linuxmint-19-x86_64.facts#L324 (assuming LinuxMint 17 used the same capitalization as newer versions).
kenyon
force-pushed
the
use-modern-os-facts
branch
from
f7d27cb to
45341ae
Compare
Thanks. I did some squashing. I think the remaining commits stand on their own. |
|
Hi @kenyon, Thanks for your contribution, looks great! 👍 |
This turned into a bigger change than I originally imagined...
Background: I'm developing a module using PDK and Litmus. My module has an option to use the backport version of the Debian package. During acceptance testing with the
litmusimage/debian:10Docker image, I found that a simpleinclude apt::backportsfailed becausepuppetlabs-aptthought it wasn't running on Debian. This turned out to be because thelsb-releasepackage wasn't installed. I noticed that, with Puppet 7 and Facter 4.0.47 anyway, the legacy OS facts aren't populated unlesslsb-releaseis installed. However, the modernosfact structure is populated even withoutlsb-release. So rather than have my module ensurelsb-releaseis installed, I decided to updatepuppetlabs-apt, and this pull request is the result.Removing this package dependency makes the module more robust and simpler to use.Update: I see now that only Puppet 7 resolves the
osfact properly withoutlsb-releaseinstalled, so I've updated this to still install that package for acceptance tests, so that testing on Puppets <7 still works, and updated the readme to note that it's not needed with Puppet 7.I didn't figure out how to stop using
osfamilyin theapt_*custom facts and theapt_keyprovider. Not sure if that is even necessary.There are a few other minor fixes in this PR, in individual commits for easier review.