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

POC: Add include_legacy_facts setting #53

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexjfisher
Copy link

When set to false (the default for Puppet 8+), all legacy facts are pruned before the catalog is compiled.

When set to false (the default for Puppet 8+), all legacy facts are
pruned before the catalog is compiled.
@alexjfisher
Copy link
Author

If this is the right direction, I should also incorporate @ekohl 's comment original made here. voxpupuli/rspec-puppet-facts#143 (comment)

@LukasAud
Copy link

Hey @alexjfisher, im looking at this and I think I understand the what, but not entirely the why. Is the aim here to remove legacy facts from spec files in modules? Simply removing and not replacing nor warning sounds like it would break things for the sake of breaking.

Can you give me an example of use case?

@ekohl
Copy link

ekohl commented Jul 13, 2023

The assumption we made was that in Puppet 8 you didn't have access to legacy facts at all, so if you could test on Puppet 7 how it would look like in the future then you could prepare your manifests. Now it looks like you do have legacy facts available it's probably not as useful.

@alexjfisher
Copy link
Author

Now it looks like you do have legacy facts available it's probably not as useful.

Really? Is this something that's being back tracked on? From the Puppet 8 release notes...

"Legacy facts are no longer collected on Puppet agent or sent to Puppet server. This saves network bandwidth and reduces Puppetserver's memory footprint when using templates.

Since legacy facts are not available during compilation, they cannot be referenced in Puppet code, ERB/EPP templates or Hiera configuration."

@ekohl
Copy link

ekohl commented Jul 14, 2023

I think Josh mentioned that they are available in the catalog compilation, but I think this is really confusing. IMHO Puppet should have made sure module authors can test their configurations and the use of rspec-puppet together with rspec-puppet-facts is really common. That this wasn't solved prior to Puppet 8's release is a failure because it may mean tests are now unreliable.

@joshcooper
Copy link

I think Josh mentioned that they are available in the catalog compilation

Yes, I said "Puppet 7 agents should still be able to resolve legacy facts during catalog application" This is because Puppet 7 agents send modern and legacy facts to the compiler when requesting a catalog. In Puppet 8, agents only send modern facts (by default based on the include_legacy_facts setting).

IMHO Puppet should have made sure module authors can test their configurations and the use of rspec-puppet together with rspec-puppet-facts is really common.

We documented how to identify modules relying on legacy facts in https://github.com/puppetlabs/puppet/wiki/Puppet-8-Compatibility#legacy-facts using the legacy-facts lint plugin. So it should be possible to test a module against Puppet 7, identify usages of legacy facts, convert the module to only using modern facts, and have the module run correctly on both Puppet 7 and 8.

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.

6 participants