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

(FACT-2740) Add Gce fact #2035

Merged
merged 1 commit into from
Aug 21, 2020
Merged

Conversation

Filipovici-Andrei
Copy link
Contributor

Added Gce fact for Windows and Linux platforms

@Filipovici-Andrei Filipovici-Andrei added the enhancement New feature or enhancement label Aug 19, 2020
@Filipovici-Andrei Filipovici-Andrei requested review from a team August 19, 2020 06:28
Copy link

@igalic igalic left a comment

Choose a reason for hiding this comment

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

🙋🏻‍♀️

def call_the_resolver
bios_vendor = Facter::Resolvers::Linux::DmiBios.resolve(:bios_vendor)

fact_value = bios_vendor&.include?('Google') ? Facter::Resolvers::Gce.resolve(:metadata) : nil
Copy link

Choose a reason for hiding this comment

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

this reads like it should be a general Unix fact

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And it is, because Linux is the root facts platform for Debian, Rhel and Sles flavors. This relation is exemplified in lib/facter/os_hierarchy.json. I know it's confusing with the way facts files are structured now, but we'll move all the common ones into the Linux facts folder.

Copy link

Choose a reason for hiding this comment

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

sorry, what i meant is: this looks like it would work the same for Solaris and FreeBSD, without a change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't add this fact on Solaris and FreeBSD because the virtual fact is not yet implemented on them. So, I'm not sure if it would work as it is.

Copy link

Choose a reason for hiding this comment

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

aye,
see also #2033 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And also I want to add that the DmiBios fact for Linux doesn't work on Solaris.

@puppetcla
Copy link

CLA signed by all contributors.

spec/fixtures/gce Outdated Show resolved Hide resolved
@BogdanIrimie BogdanIrimie merged commit b7fa483 into puppetlabs:4.x Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants