Breaking change named as improvement in facter 2.0

[Jayapandian Ponraj]

https://tickets.puppetlabs.com/browse/FACT-163

TLDR: The custom fact loading logic has been changed in facter-2.0 from recursive to only top level directories. This is definitely a change in behaviour and must be marked

as a breaking change instead marked as an improvement.

In puppet 2.7 and facter 1.6.7 we had defined custom facts in the following locations

/var/lib/puppet/lib/facter/network/cus_net_fact.rb

/var/lib/puppet/lib/facter/network/cus_net_fact2.rb

/var/lib/puppet/lib/facter/os/cus_os_fact.rb

But after moving to puppet 3.x and facter 2.4 these facts are not available due to the change https://tickets.puppetlabs.com/browse/FACT-163

The change in behaviour is not obvious and not documented clearly. While moving from puppet 2.7 to 3.x the facter version changes 

directly from 1.6.7 to 2.4 (skipping 2.0,2.1,2.2,2.3). Hence looking at the changelog of facter 2.4 doesn't give any information regarding this.

Even in facter 2.0 where this change was made its not obvious as its marked as improvement and not as a breaking change.

* The release notes for facter 2.0 need to be fixed. Also a note in release notes of facter 2.4 need to be made as users are more likely to jump

form facter 1.6 to 2.4 when changing from puppet 2.7 to 3.x

* The default assumption while doing the change according to facter 2.0 release notes is wrong.

Facter was doing a recursive search for both command line and in puppet. We have been depending on recursive lookup and I have checked again with a 

fresh puppet 2.7 installation that facter does a recursive lookup in both puppet and command line.

https://docs.puppetlabs.com/facter/2.0/release_notes.html#improvements

In Facter 1.x the fact search path would be recursively loaded, but only when using Facter via the command line. In Facter 2.0 only fact files at the top level of the search path will be loaded, which matches the behavior when loading facts with Puppet. 

* Since forge modules are not using this behaviour doesn't guarantee that its not used elsewhere. If the behaviour changes irrespective of 

whether its used in forge modules or whether its a documented behaviour, it needs to mentioned as a breaking change.

https://tickets.puppetlabs.com/browse/FACT-163

An inspection of all current forge modules shows that no existing module is using this behavior

* Is the change really needed?

Having the custom facts in different folders helps in categorization IMO. OS, network, application facts can be kept at separate folders which improves readability,

instead of all the scripts in the same folder.

* If the change is indeed required for performance reasons then it can be added as a note in upgrade notes of puppet, as most users ll not be 

able to link this change to facter.

[jcbollinger]

On Monday, March 23, 2015 at 7:30:10 AM UTC-5, Jayapandian Ponraj wrote:

https://tickets.puppetlabs.com/browse/FACT-163

TLDR: The custom fact loading logic has been changed in facter-2.0 from recursive to only top level directories. This is definitely a change in behaviour and must be marked

as a breaking change instead marked as an improvement.

I'm sorry you experienced this unpleasant surprise.  I agree that it is a bit odd for the release notes to categorize this change as an "improvement", since it is (considered to be) an improvement primarily in the internal organization of Facter's implementation.  Such a thing rarely rises to the level of a mention in a product's release notes at all.  That it is mentioned may signal that you have some traction on the issue, at least with respect to documentation.

 

In puppet 2.7 and facter 1.6.7 we had defined custom facts in the following locations

/var/lib/puppet/lib/facter/network/cus_net_fact.rb

/var/lib/puppet/lib/facter/network/cus_net_fact2.rb

/var/lib/puppet/lib/facter/os/cus_os_fact.rb

But after moving to puppet 3.x and facter 2.4 these facts are not available due to the change https://tickets.puppetlabs.com/browse/FACT-163

The change in behaviour is not obvious and not documented clearly.

On the other hand, the recursive lookup behavior you describe is not documented at all.  The Facter 1.7 docs are pretty similar to the Facter 2 docs here, and none of them would have lead me to suspect that Facter would perform recursive lookups for custom fact implementations.  I completely understand that you would have liked clearer, more prominent notice of this change, but I cannot personally find any fault here -- you already have more documentation than it is reasonable to expect for a change in (previously) undocumented behavior.  However, given that the change was, indeed, documented, you may be able to persuade PL to make that documentation more prominent.

[...]
 

* Is the change really needed?

Having the custom facts in different folders helps in categorization IMO. OS, network, application facts can be kept at separate folders which improves readability,

instead of all the scripts in the same folder.

I'm sure the change was not needed, but it was judged an improvement.

I suppose your angle here is to try to get recursive lookups restored to Facter.  I think the chances of that are slim, but it's a reasonable topic for discussion.  If you're serious, then you'd want also to file an RFE against Facter.  Substantial positive response to the idea might improve its chances.

John