Project

General

Profile

Actions

Refactor #27906

closed

Use modern Facter 3 facts

Added by Ewoud Kohl van Wijngaarden about 5 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Category:
Facts
Target version:
-
Fixed in Releases:
Found in Releases:

Description

Currently we still rely on various Facter 2 facts that are deprecated in Facter 3 and will be removed in Facter 4.

https://puppet.com/docs/facter/3.11/core_facts.html#legacy-facts is a list of legacy facts.


Related issues 3 (2 open1 closed)

Related to Foreman - Refactor #32130: Drop support of Facter 2 factsDuplicateActions
Related to Foreman - Feature #32337: Drop support for Facter versions older than 2.2Ready For TestingEwoud Kohl van WijngaardenActions
Related to Foreman - Refactor #32345: Provide non-DB model methods and deprecate DB model methods in FactParserNewEwoud Kohl van WijngaardenActions
Actions #1

Updated by The Foreman Bot about 5 years ago

  • Status changed from New to Ready For Testing
  • Assignee set to Ewoud Kohl van Wijngaarden
  • Pull request https://github.com/theforeman/foreman/pull/7059 added
Actions #2

Updated by Ewoud Kohl van Wijngaarden over 3 years ago

Actions #3

Updated by Ewoud Kohl van Wijngaarden over 3 years ago

  • Status changed from Ready For Testing to New
  • Assignee deleted (Ewoud Kohl van Wijngaarden)
Actions #4

Updated by Ewoud Kohl van Wijngaarden over 3 years ago

It should also be noted that Facter 4 ended up keeping the legacy facts. However, we should still update to the modern facts so at some point legacy facts can be turned off.

Actions #5

Updated by Marek Hulán over 3 years ago

from the other issue:

Today we still support so called legacy facts in places we parse puppet facts. The drop of the Facter 2 was announced a while ago at https://community.theforeman.org/t/the-road-to-making-puppet-optional/17983 and Foreman 2.6/3.0 would be a good time to clean up this code

While some work has been done, there are still facter 2 facts supported, they should be removed from the codebase.

Actions #6

Updated by Lukas Zapletal over 3 years ago

Guys, are we sure about dropping facter 2.x from the codebase? There was a 2.4 user complaining about 2.x facter broken after upgrade, I think we might still have some users. And this should probably be communicated in advance, we haven't announced or deprecated anything yet IIRC.

https://community.theforeman.org/t/puppet-fact-parser-is-failing-with-string-does-not-have-dig-method/23138

From discovery standpoint we are now good, it took me a while to upgrade to f4 but we are good. But honestly I think we should actually do the very opposite - fix f2 support, then do some announcement and deprecation warnings and schedule this to 3.1 or something like that.

Actions #7

Updated by Ewoud Kohl van Wijngaarden over 3 years ago

IMHO we should make sure the codebase 100% works when no legacy facts are present. That will be a first step to deprecation.

The particular case mentioned doesn't appear to be related. The structured os fact should be present in 2.4 already.

Actions #8

Updated by Lukas Zapletal over 3 years ago

While I agree, I think we should keep the status-quo for some time before we are ready for the switch. I suggest that we explore what exactly is wrong, since this was broken quite recently (the user reports 2.4) chances are we can fix that easily and buy some more time.

Actions #9

Updated by Ewoud Kohl van Wijngaarden over 3 years ago

In the linked post I have said that it should work if the user doesn't use stringify_facts = true. In Puppet 4 that setting has been removed and is always false. We have also dropped support for Puppet < 5 so it would be weird to document "this is how you make unsupported software work". Also note that the user reported they upgraded to 1.24 which is also unsupported.

What we can do is more graceful error handling. IMHO this isn't very helpful to a user. I'm thinking about adding abstractions for all facts with rescue statements everywhere that says "failed to read fact x" so it's at least easier to understand.

Actions #10

Updated by Marek Hulán over 3 years ago

Not a bad idea about improving the error reporting, Ewoud could you turn that into a new RM issue and describe the expected behavior on several examples please? Like if uploaded facts does not contain fact xyz, he should see an error saying "fact xyz is not present". Ideally also specify where (notification drawer, log, API response, host status or something else)

I agree we should go ahead and drop support for legacy facts, I asked few users in our community and there was no concern with that. People seem to be upgrading their Puppet infrastructure (good). If they have older facters deployed, they probably also have older Foreman installed.

Actions #11

Updated by Ewoud Kohl van Wijngaarden over 3 years ago

https://github.com/puppetlabs/facter/commit/58901c0101e8c8d7f1ec80bcb9356e61d7cadee3 did add a schema of facts. Its history can be used as a source of when a fact was introduced. That's the best way right now that I know to determine it.

Actions #12

Updated by The Foreman Bot over 3 years ago

  • Status changed from New to Ready For Testing
  • Assignee set to Ewoud Kohl van Wijngaarden
  • Pull request https://github.com/theforeman/foreman/pull/8449 added
Actions #13

Updated by Ewoud Kohl van Wijngaarden over 3 years ago

  • Related to Feature #32337: Drop support for Facter versions older than 2.2 added
Actions #14

Updated by Ewoud Kohl van Wijngaarden over 3 years ago

  • Category deleted (Facts)
  • Assignee deleted (Ewoud Kohl van Wijngaarden)

In the attached PR I updated all facts to their modern version where possible. I also annotated all facts and when they were introduced. It looks like if we raise the version to at least 2.2, we can simplify the parser so I opened #32337 for that.

Let's keep the scope of this PR to only make sure we can operate without any legacy facts present.

Actions #15

Updated by The Foreman Bot over 3 years ago

  • Assignee set to Ewoud Kohl van Wijngaarden
Actions #16

Updated by Ewoud Kohl van Wijngaarden over 3 years ago

  • Related to Refactor #32345: Provide non-DB model methods and deprecate DB model methods in FactParser added
Actions #17

Updated by The Foreman Bot almost 3 years ago

  • Pull request https://github.com/theforeman/foreman/pull/8450 added
Actions #18

Updated by The Foreman Bot almost 3 years ago

  • Fixed in Releases 3.2.0 added
Actions #19

Updated by Ewoud Kohl van Wijngaarden almost 3 years ago

  • Status changed from Ready For Testing to Closed
Actions #20

Updated by Amit Upadhye over 2 years ago

  • Category set to Facts
Actions

Also available in: Atom PDF