Jira (PUP-10731) Setting facterng twice raises an exception

0 views
Skip to first unread message

Josh Cooper (Jira)

unread,
Oct 29, 2020, 6:13:03 PM10/29/20
to puppe...@googlegroups.com
Josh Cooper created an issue
 
Puppet / Bug PUP-10731
Setting facterng twice raises an exception
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2020/10/29 3:12 PM
Priority: Normal Normal
Reporter: Josh Cooper

Using 6.19.0, setting facterng to true twice programmatically results in a NameError:

[root@baneful-extreme ~]# /opt/puppetlabs/puppet/bin/irb
irb(main):001:0> require 'puppet'
=> true
irb(main):002:0> Puppet.initialize_settings
=> {}
irb(main):003:0> pp $LOADED_FEATURES.sort.grep /facter(-ng)?\.rb$/
["/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/did_you_mean-1.2.0/lib/facter.rb",
 "/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/facter.rb"]
=> ["/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/did_you_mean-1.2.0/lib/facter.rb", "/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/facter.rb"]
irb(main):004:0> Puppet[:facterng] = true
=> true
irb(main):005:0> pp $LOADED_FEATURES.sort.grep /facter(-ng)?\.rb$/
["/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/did_you_mean-1.2.0/lib/facter.rb",
 "/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/facter-ng-4.0.43/agent/lib/facter-ng.rb",
 "/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/facter-ng-4.0.43/lib/facter.rb",
 "/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/facter-ng-4.0.43/lib/facter/custom_facts/core/legacy_facter.rb",
 "/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/facter.rb"]
=> ["/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/did_you_mean-1.2.0/lib/facter.rb", "/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/facter-ng-4.0.43/agent/lib/facter-ng.rb", "/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/facter-ng-4.0.43/lib/facter.rb", "/opt/puppetlabs/puppet/lib/ruby/gems/2.5.0/gems/facter-ng-4.0.43/lib/facter/custom_facts/core/legacy_facter.rb", "/opt/puppetlabs/puppet/lib/ruby/vendor_ruby/facter.rb"]
irb(main):006:0> Puppet[:facterng] = true
Traceback (most recent call last):
        7: from /opt/puppetlabs/puppet/bin/irb:11:in `<main>'
        6: from (irb):6
        5: from /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet.rb:108:in `[]='
        4: from /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/settings.rb:178:in `[]='
        3: from /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/settings.rb:1494:in `set'
        2: from /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/defaults.rb:87:in `block in initialize_default_settings!'
        1: from /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/util/logging.rb:258:in `setup_facter_logging!'
NameError (uninitialized constant Puppet::Util::Logging::Facter)
Did you mean?  Facts

The first time Puppet[:facterng] = true is called, the hook for the setting calls require 'facter-ng' and ruby adds it to the special $LOADED_FEATURES hash. The next time Puppet[:facterng] = true is called, we remove the Facter constant and then we try to require 'facter-ng' again. But ruby "remembers" that it already required it once, and doesn't require it again. As a result, require 'facter-ng' returns false, and the Facter constant is not defined when we call into:

  def self.setup_facter_logging!
    # Only recent versions of Facter support this feature
    return false unless Facter.respond_to? :on_message
...

We then get a NameError trying to call Facter.respond_to?.

We may want to define the hook in such a way that if facterng is already enabled, we don't try to enable it a second time. The difficulty is when the hook is called, the new value is already "set", so AFAIK you can't ask for the previous value.

Alternative, you can call Kernel.load <path> to force facter-ng to be reloaded. But you need to specify a full path to the file and that might be hard when running as a gem or from packages.

Also a few things I noticed:

  • The hook needs to munge the value passed to it, as it may receive the string "false" which is truthy. This can occur when called programmatically or via puppet config set facterng true.
  • After calling require/load 'facter-ng' It would also be good to check that the Facter constant is defined before calling into setup_facter_logging!.
Add Comment Add Comment
 
This message was sent by Atlassian Jira (v8.5.2#805002-sha1:a66f935)
Atlassian logo

Josh Cooper (Jira)

unread,
Oct 29, 2020, 6:13:03 PM10/29/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Team: Night's Watch

Josh Cooper (Jira)

unread,
Oct 29, 2020, 6:16:03 PM10/29/20
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10731
 
Re: Setting facterng twice raises an exception

puppet-agent#master CI is blocked due to this and PUP-2178. The trick is to run puppet config set facterng true once, and then try to run it again or set the value to false:

* disable facterng feature flag
  
  back-logarithm.delivery.puppetlabs.net (back-logarithm.delivery.puppetlabs.net) 15:40:36$ puppet config set facterng false
    Error: uninitialized constant Puppet::Util::Logging::Facter
    Did you mean?  Puppet::Face
                   Facts
    Error: Try 'puppet help config set' for usage

Josh Cooper (Jira)

unread,
Oct 29, 2020, 6:17:03 PM10/29/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Issue Type: Bug CI Blocker

Josh Cooper (Jira)

unread,
Oct 29, 2020, 6:18:03 PM10/29/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Using 6.19.0, setting {{facterng}} to true twice programmatically results in a {{NameError}}:

{noformat}
{noformat}


The first time {{Puppet[:facterng] = true}} is called, the hook for the setting calls {{require 'facter-ng'}} and ruby adds it to the special {{$LOADED_FEATURES}} hash. The next time {{Puppet[:facterng] = true}} is called, we remove the {{Facter}} constant and then we try to {{require 'facter-ng'}} again. But ruby "remembers" that it already required it once, and doesn't require it again. As a result, {{require 'facter-ng'}} returns false, and the {{Facter}} constant is not defined when we call into:

{code:ruby}

  def self.setup_facter_logging!
    # Only recent versions of Facter support this feature
    return false unless Facter.respond_to? :on_message
...
{code}

We then get a {{NameError}} trying to call {{Facter.respond_to?}}.


We may want to define the hook in such a way that if facterng is already enabled, we don't try to enable it a second time. The difficulty is when the hook is called, the new value is already "set", so AFAIK you can't ask for the previous value.

Alternative, you can call {{Kernel.load <path>}} to force {{facter-ng}} to be reloaded. But you need to specify a full path to the file and that might be hard when running as a gem or from packages.

Also a few things I noticed:

* The hook needs to munge the value passed to it, as it may receive the string {{"false"}} which is truthy. This can occur when called programmatically or via {{puppet config set facterng true false }}.

* After calling {{require/load 'facter-ng'}} It would also be good to check that the {{Facter}} constant is defined before calling into {{setup_facter_logging!}}.

Mihai Buzgau (Jira)

unread,
Oct 30, 2020, 9:26:03 AM10/30/20
to puppe...@googlegroups.com
Mihai Buzgau updated an issue
Change By: Mihai Buzgau
Sprint: ghost-4.11

Mihai Buzgau (Jira)

unread,
Oct 30, 2020, 9:26:04 AM10/30/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Oct 30, 2020, 1:06:03 PM10/30/20
to puppe...@googlegroups.com
Josh Cooper commented on CI Blocker PUP-10731
 
Re: Setting facterng twice raises an exception

This was merged to master in bfd22200fc

Josh Cooper (Jira)

unread,
Oct 30, 2020, 1:07:03 PM10/30/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
 
Change By: Josh Cooper
CI Pipeline/s: platform puppet-agent suite
Release Notes: Bug Fix
Release Notes Summary: Calling "puppet config set facterng true" twice would result in a failure. Now facterng is only enabled if it isn't already.

Claire Cadman (Jira)

unread,
Jan 13, 2021, 9:16:04 AM1/13/21
to puppe...@googlegroups.com
Claire Cadman updated an issue
Change By: Claire Cadman
Labels: doc_reviewed
Reply all
Reply to author
Forward
0 new messages