Jira (PUP-8543) Puppet partly relies on legacy facts

[Philippe Muller (JIRA)]

Philippe Muller created an issue

Puppet / PUP-8543 Puppet partly relies on legacy facts Issue Type: Bug Affects Versions: PUP 5.4.0 Assignee: Unassigned Components: Types and Providers Created: 2018/03/12 10:37 AM Priority: Normal Reporter: Philippe Muller Puppet Version: 5.4.0 OS Name/Version: All Reading the documentation, I believe the recommended way to access facts is through $facts. However, some parts of the code base still relies on legacy facts. For example, see provider/service/init.rb. When writing a module which strictly relies on new-style facts, this is bothering: the only way to make them work is to provide both legacy and new-style facts.  Desired Behavior: Only relies new-style facts. Actual Behavior: Still relies on legacy facts. Gives weird errors, like this one: Puppet::PreformattedError: Evaluation Error: Error while evaluating a Resource Statement, Could not autoload puppet/type/service: Could not autoload puppet/provider/service/gentoo: Could not autoload puppet/provider/service/init: undefined method `downcase' for nil:NilClass Add Comment

This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)

[Philippe Muller (JIRA)]

Philippe Muller commented on PUP-8543

Re: Puppet partly relies on legacy facts For reference: ~/dev/oss/puppet master= $ git grep -E 'Facter\.value\(:?"?(osfamily|operatingsystem|hostname|domain|operatingsystemmajrelease|lsbdistrelease|kernelrelease|kernelmajversion)"?\)' | wc -l 75 As I am a newbie in this code base, I guess I missed a lot of others.

Add Comment

[Josh Cooper (JIRA)]

Josh Cooper commented on PUP-8543

Re: Puppet partly relies on legacy facts

Hi Philippe Muller, the $facts hash is available when writing a manifest, but not in a provider, so it's ok to call Facter(:fact). Can you provide more information about your puppetserver and puppet-agent versions? I am guessing that the service type is loaded in puppetserver, and the osfamily fact is not getting resolved for some reason. /cc Justin Stoller, Maggie Dreyer

Add Comment

[Justin Stoller (JIRA)]

Justin Stoller commented on PUP-8543

Re: Puppet partly relies on legacy facts

As Josh mentions Phillippe, the $facts accessor is the preferred way to reference facts from the Puppet Language in manifests, when writing custom types and providers in Ruby a different way to access the facts is needed, which is the Facter[:<fact_name>] method that you're seeing. The error you're seeing is definitely a problem and it looks like Facter cannot resolve either the operatingsystem or osfamily fact for your Gentoo server. Does this occur when you run the server or that agent? What does facter -dtp operatingsystem and facter -dtp osfamily return on that server?

Add Comment

[Philippe Muller (JIRA)]

Philippe Muller updated an issue

Puppet / PUP-8543

Puppet partly relies on legacy facts

Change By: Philippe Muller *Puppet Version: 5.4.0* *OS Name/Version: All* Reading the documentation, I believe the recommended way to access facts is through {{$facts}} , using [modern facts|https://puppet . com/docs/facter/3.9/core_facts.html#modern-facts]. However, some parts of the code base still relies on legacy facts. For example, see [provider/service/init.rb|https://github.com/puppetlabs/puppet/blob/master/lib/puppet/provider/service/init.rb#L7]. When writing a module which strictly relies on new [modern facts|https://puppet.com/docs/facter/3.9/core_facts.html#modern - style facts ] , this is bothering: the only way to make them work is to provide both [ legacy and new |https://puppet.com/docs/facter/3.9/core_facts.html#legacy - style facts ] and modern facts .  *Desired Behavior:* Only relies new-style modern facts. *Actual Behavior:*

Still relies on legacy facts. Gives weird errors, like this one:

{noformat}     Puppet::PreformattedError:

Evaluation Error: Error while evaluating a Resource Statement, Could not autoload puppet/type/service: Could not autoload puppet/provider/service/gentoo: Could not autoload puppet/provider/service/init: undefined method `downcase' for nil:NilClass

{noformat}

Add Comment

[Philippe Muller (JIRA)]

Philippe Muller commented on PUP-8543

Re: Puppet partly relies on legacy facts I updated the issue description, trying to improve it. My point is: Since Facter 3 (I believe), there are both "legacy" and "modern" facts (as described in Core Facts) The issue is that while the documentation seems to recommend using modern facts, Puppet is still relying a lot on legacy facts I stumbled upon this issue while writing tests for a Puppet module. I wanted to rely strictly on modern facts. So I only mocked modern facts. But when using the service type, I got the weird error specified in the description. So no, I am not using Gentoo, nor a Puppet Server. Just Puppet 4 and 5, running with rspec-puppet. As it appears that legacy facts use is generalized in the Puppet code base, I did not make this issue specific to the service type.

Add Comment

[Philippe Muller (JIRA)]

Philippe Muller updated an issue

Puppet / PUP-8543

Puppet partly relies on legacy facts

Change By: Philippe Muller

*Puppet Version: 5.4.0* *OS Name/Version: All*

Reading the documentation, I believe the recommended way to access facts is through {{$facts}}, using [modern facts|https://puppet.com/docs/facter/3.9/core_facts.html#modern-facts].

However, some parts of the code base still relies on legacy facts. For example, see [provider/service/init.rb|https://github.com/puppetlabs/puppet/blob/master/lib/puppet/provider/service/init.rb#L7].

When writing a module which strictly relies on [modern facts|https://puppet.com/docs/facter/3.9/core_facts.html#modern-facts], this is bothering: the only way to make them work is to provide both [legacy|https://puppet.com/docs/facter/3.9/core_facts.html#legacy-facts] and modern facts.  *Desired Behavior:* Only relies modern facts. *Actual Behavior:* Still relies on legacy facts. Gives weird _weird_ errors, like this one:

{noformat}     Puppet::PreformattedError:        Evaluation Error: Error while evaluating a Resource Statement, Could not autoload puppet/type/service: Could not autoload puppet/provider/service/gentoo: Could not autoload puppet/provider/service/init: undefined method `downcase' for nil:NilClass {noformat}

(i) Not using Gentoo: this misleading error is just a side effect of [this all to downcase|https://github.com/puppetlabs/puppet/blob/master/lib/puppet/provider/service/init.rb#L24] on an undefined legacy fact in my rspec-puppet mocked facts.

Add Comment

[Josh Cooper (JIRA)]

Josh Cooper commented on PUP-8543

Re: Puppet partly relies on legacy facts Philippe Muller facter still resolves both legacy and modern facts. So for example, you can run facter operatingsystem on the command line. The only difference between the two are modern facts are namespaced and can be structured, e.g. mountpoints.<name>.device. I think you're going to have a bad time trying to run puppet without valid operatingsystem and osfamily facts. Checkout https://github.com/mcanevet/rspec-puppet-facts which can help you stub out different OS facts. I'm going to close this as puppet requires some basic facts to work.

Add Comment

[Josh Cooper (Jira)]

Josh Cooper commented on PUP-8543

Re: Puppet partly relies on legacy facts

Philippe Muller yes you're correct. Puppet's builtin types and providers should not reference legacy facts in their confine, defaultfor, etc statements, as doing so makes it impossible to block legacy facts. For example we tried that in https://github.com/puppetlabs/puppet/commit/8660b32739ada447e32722f1e92e7086ce28737c and had to revert in https://github.com/puppetlabs/puppet/commit/16d765db7e64d504bfe3d1570352f5226beb4924 (see PUP-10853) This change has the potential to break modules' tests if a module is stubbing out legacy facts as a way of selecting a provider. In other words, so I think this change needs to land in 8.0.

Add Comment

This message was sent by Atlassian Jira (v8.20.11#820011-sha1:0629dd8)

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-8543

Puppet partly relies on legacy facts

Change By: Josh Cooper Team: Phoenix

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-8543 Puppet partly relies on legacy facts

Change By: Josh Cooper Epic Link: PA-4664

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-8543 Puppet partly relies on legacy facts

Change By: Josh Cooper Fix Version/s: PUP 8.0.0

Add Comment

[Morgan Rhodes (Jira)]

Morgan Rhodes updated an issue

Puppet / PUP-8543 Puppet partly relies on legacy facts

Change By: Morgan Rhodes Story Points: 3

Add Comment

[Josh Cooper (Jira)]

Josh Cooper updated an issue

Puppet / PUP-8543 Puppet partly relies on legacy facts

Change By: Josh Cooper

*Puppet Version: 5.4.0* *OS Name/Version: All* Reading the documentation, I believe the recommended way to access facts is through {{$facts}}, using [modern facts|https://puppet.com/docs/facter/3.9/core_facts.html#modern-facts]. However, some parts of the code base still relies on legacy facts. For example, see [provider/service/init.rb|https://github.com/puppetlabs/puppet/blob/master/lib/puppet/provider/service/init.rb#L7]. When writing a module which strictly relies on [modern facts|https://puppet.com/docs/facter/3.9/core_facts.html#modern-facts], this is bothering: the only way to make them work is to provide both [legacy|https://puppet.com/docs/facter/3.9/core_facts.html#legacy-facts] and modern facts.  *Desired Behavior:* Only relies modern facts. *Actual Behavior:*

Still relies on legacy facts. Gives _weird_ errors, like this one:

{noformat}     Puppet::PreformattedError:        Evaluation Error: Error while evaluating a Resource Statement, Could not autoload puppet/type/service: Could not autoload puppet/provider/service/gentoo: Could not autoload puppet/provider/service/init: undefined method `downcase' for nil:NilClass {noformat} (i) Not using Gentoo: this misleading error is just a side effect of [this all to downcase|https://github.com/puppetlabs/puppet/blob/master/lib/puppet/provider/service/init.rb#L24] on an undefined legacy fact in my rspec-puppet mocked facts.

* Update * For this ticket, update puppet source and test files to no longer reference legacy facts. I believe beaker tests have already been updated see https://github.com/puppetlabs/puppet/commit/7dbc49bb31c36b10b47bb5181438ca562055586c Build a puppet-agent containing these changes and verify facter and puppet-agent beaker tests pass: {noformat} $ bundle exec rake ci:test:setup SHA=<SHA> HOSTS=redhat7-64a $ bundle exec beaker exec tests {noformat} Do the same for puppet beaker tests (you'll need puppetserver to be installed): {noformat} $ bundle exec rake ci:test:setup SHA=<SHA> HOSTS=redhat7-64a-redhat7-64m SERVER_VERSION=7.9.4.SNAPSHOT.2023.01.18T0732 $ bundle exec beaker exec tests {noformat} Update tests as needed an rerun tests iteratively.

Add Comment

[Josh Cooper (Jira)]

[Christopher Thorn (Jira)]

Christopher Thorn assigned an issue to Christopher Thorn

Puppet / PUP-8543 Puppet partly relies on legacy facts

Change By: Christopher Thorn Assignee: Christopher Thorn

Add Comment

[Christopher Thorn (Jira)]

Christopher Thorn updated an issue

Puppet / PUP-8543 Puppet partly relies on legacy facts

Change By: Christopher Thorn Sprint: Phoenix 2023-02-01

Add Comment

[Christopher Thorn (Jira)]

Christopher Thorn updated an issue

Puppet / PUP-8543 Puppet partly relies on legacy facts

Change By: Christopher Thorn Sprint: Phoenix 2023-02-01 , Phoenix 2023-02-15

Add Comment

[Aria Li (Jira)]

Aria Li updated an issue

Puppet / PUP-8543 Puppet partly relies on legacy facts

Change By: Aria Li Release Notes: Not Needed

Add Comment