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.

Sign up for GitHub

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

Jump to bottom

(MODULES-11138) - Fix mac_source Facter.fact().value() issue with Facter 3 #1002

Merged
merged 2 commits into from
Jul 23, 2021
Merged

(MODULES-11138) - Fix mac_source Facter.fact().value() issue with Facter 3 #1002

merged 2 commits into from
Jul 23, 2021

Conversation

adrianiurca
Copy link
Contributor

@adrianiurca adrianiurca commented Jul 21, 2021 •
edited
Loading

According to the firewall module HISTORY.md Facter.fact().value() is not supported by Facter 3 - Replace Facter.fact().value() calls with Facter.value() to support Facter 3
I added by mistake trying to repair our nightly CI. This PR should repair users problems created by the previous commit and not block our nightly CI

kind regards,
@adrianiurca

@adrianiurca adrianiurca requested a review from a team as a code owner July 21, 2021 12:59
@puppet-community-rangefinder
Copy link

firewall is a type

Breaking changes to this file WILL impact these 120 modules (exact match):
Breaking changes to this file MAY impact these 143 modules (near match):

This module is declared in 108 of 577 indexed public Puppetfiles.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@adrianiurca
Copy link
Contributor Author

Testing this on server-agent environment

$: cat /etc/puppetlabs/code/environments/production/manifests/site.pp
node default {}
  
node 'salty-beadle.delivery.puppetlabs.net' {
  include firewall
  firewall { '001 accept all to lo interface':
    proto   => 'all',
    iniface => 'lo',
    action  => 'accept',
  }
}

running puppet agent -t on agent machine results:

Info: Retrieving locales
Info: Loading facts
Info: Caching catalog for salty-beadle.delivery.puppetlabs.net
Info: Applying configuration version '1626871967'
Notice: /Stage[main]/Firewall::Linux::Debian/Package[iptables-persistent]/ensure: created
Notice: /Stage[main]/Main/Node[salty-beadle.delivery.puppetlabs.net]/Firewall[001 accept all to lo interface]/ensure: created
Notice: Applied catalog in 2.87 seconds

@pmcmaw
Copy link
Contributor

pmcmaw commented Jul 21, 2021

I would suggest adding a test to avoid a regression like this happening in the future as our current test suite did not catch it.

@adrianiurca
Copy link
Contributor Author

I would suggest adding a test to avoid a regression like this happening in the future as our current test suite did not catch it.

Hi @pmcmaw,

I tested in 4 different scenarios:

Scenario 1

server-agent environment
Puppetserver v6.16.1
Puppet-agent v6.17.0
Facter v3.14.12 (commit 70d26da13c9fee27973c1eb22200871e21445d97) - on agent machine
*** running using puppet agent -t
*** this environment is similar with the ticket environment
Result: the bug explained in the ticket is reproducing

Scenario 2

server-agent environment
Puppetserver v7.2.0
Puppet-agent v7.9.0
Facter v4.2.2 - on agent machine
*** running using puppet agent -t
Result: the bug is not reproducing

Scenario 3

Puppet-agent v6.17.0
Facter v3.14.12 (commit 70d26da13c9fee27973c1eb22200871e21445d97)
*** running the manifest using puppet apply
Result: the bug is not reproducing

Scenario 4

Puppet-agent v7.9.0
Facter v4.2.2
*** running the manifest using puppet apply
Result: the bug is not reproducing

Because our CI running tests using puppet apply this problem couldn't be caught.
So in conclusion something weird it's happening. I will ask someone from the facter team to explain to me better what is happening and how we can test fast, and maybe if is possible to add this kind of scenario in our CI.

kind regards,
@adrianiurca

@carabasdaniel carabasdaniel changed the title (MODULES-11138) - Facter.fact().value() not supported by facter 3 (MODULES-11138) - Fix mac_source Facter.fact().value() issue with Facter 3 Jul 22, 2021
@daianamezdrea daianamezdrea merged commit 9b5b082 into main Jul 23, 2021
@daianamezdrea daianamezdrea deleted the modules-11138-Facter-fact-error branch July 23, 2021 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carabasdaniel carabasdaniel carabasdaniel approved these changes

Assignees
No one assigned
Labels
bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@adrianiurca @pmcmaw @carabasdaniel @daianamezdrea