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.

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

Use confine to check the existence of a command needed for the fact to resolve #603

Closed
wants to merge 1 commit into from

Conversation

willaerk
Copy link

An initial puppet run (eg. in a vagrant box) will produce an error while trying to resolve the mongodb_version fact:

Facter: error while resolving custom fact "mongodb_version": undefined method '[]' for nil:NilClass

The reason is that the check for the existence of the mongo binary is inside the setcode block.

This pull request uses the fact confinement mechanism to pre-empt the fact resolution if the mongo binary is not available.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fail to see how this is different. It looks like which is working so something is on the system. This really looks like something that shouldn't work and shouldn't be done.

Have you checked why Facter::Core::Execution.which('mongo') returns something truthy but Facter::Core::Execution.execute('mongo --version 2>&1') returns something that doesn't match %r{MongoDB shell version:?\s+v?([\w\.]+)}?

Perhaps it should log a warning if that returns nil and not fail hard.

@@ -1,8 +1,8 @@
Facter.add(:mongodb_version) do
confine { Facter::Core::Execution.which('mongo') }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://puppet.com/docs/facter/3.11/custom_facts.html#confining-facts doesn't mention this form at all. I'm hesitant to rely on something that doesn't appear to be what it was intended to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants