Skip to content

Conversation

@natemccurdy
Copy link
Contributor

@natemccurdy natemccurdy commented Apr 12, 2022

Similar to #2475 , this allows the cloud.provider fact to resolve on Google Compute Engine nodes to gce.

For example:

[user@a_gce_node]> facter cloud.provider
gce

This partially resolves https://tickets.puppetlabs.com/browse/FACT-1557

Note that I chose to have this resolve to the string "gce" because that's what the current virtual fact resolves to, so Facter/Puppet users are probably most familiar with "gce" meaning Google Compute Engine.

@natemccurdy natemccurdy requested a review from a team as a code owner April 12, 2022 18:26
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@joshcooper
Copy link
Contributor

@natemccurdy could you prepend (FACT-1557) to your commit summary and force push to your feature branch?

@natemccurdy natemccurdy force-pushed the FACT-1557/cloud_provider_gce branch from fb031d8 to 0f5df2e Compare April 27, 2022 18:31
@natemccurdy
Copy link
Contributor Author

natemccurdy commented Apr 27, 2022

could you prepend (FACT-1557) to your commit summary and force push to your feature branch?

@joshcooper Done. Thanks for looking!

@natemccurdy natemccurdy force-pushed the FACT-1557/cloud_provider_gce branch from 0f5df2e to cbbda1c Compare May 17, 2022 19:17
@natemccurdy
Copy link
Contributor Author

Offering given to the Rubocop gods, tests updated for lint failures, and branch force pushed. The rubocop tests should pass now.

@natemccurdy natemccurdy force-pushed the FACT-1557/cloud_provider_gce branch from cbbda1c to 4e7db5e Compare May 17, 2022 21:18
@natemccurdy
Copy link
Contributor Author

Guess I didn't make the right offering, sorry RuboCop gods.
This time I fixed the lint issue, and verified locally with:

~/src/puppetlabs/facter [FACT-1557/cloud_provider_gce] be rubocop --parallel
Inspecting 1730 files
...............................................................................
...............................................................................
...............................................................................
...............................................................................
...............................................................................
...............................................................................

1730 files inspected, no offenses detected

@natemccurdy
Copy link
Contributor Author

@joshcooper What do you recommend I do about the new RuboCop warnings?

New offenses:
  - lib/facter/facts/linux/cloud/provider.rb:
    - Metrics/CyclomaticComplexity: changed from 0 to 1
  - lib/facter/facts/windows/cloud/provider.rb:
    - Metrics/CyclomaticComplexity: changed from 0 to 1

@joshcooper
Copy link
Contributor

@natemccurdy could you add exclusions for those files to .rubocop.yml and add that as a second commit? If the rubocop checks are performed for each commit, you may need to switch the order so the files are excluded first?

@natemccurdy natemccurdy force-pushed the FACT-1557/cloud_provider_gce branch from 4e7db5e to 71be25d Compare June 13, 2022 18:20
@natemccurdy
Copy link
Contributor Author

natemccurdy commented Jun 13, 2022

...add exclusions for those files to .rubocop.yml

Done 👍

Copy link
Contributor

@joshcooper joshcooper left a comment

Choose a reason for hiding this comment

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

Verified this behaves as expected on GCE:

~/facter# bundle exec facter cloud.provider
gce

and it has parity with other cloud providers facts like https://tickets.puppetlabs.com/browse/FACT-1441

@joshcooper joshcooper merged commit b29fe99 into puppetlabs:main Jun 14, 2022
@natemccurdy natemccurdy deleted the FACT-1557/cloud_provider_gce branch June 14, 2022 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants