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-2721) Added Solaris virtual fact #2033

Merged
merged 1 commit into from
Aug 20, 2020

Conversation

sebastian-miclea
Copy link
Contributor

No description provided.

@sebastian-miclea sebastian-miclea requested review from a team August 18, 2020 06:07
@sebastian-miclea sebastian-miclea force-pushed the FACT-2721 branch 2 times, most recently from 03adf43 to 19b89a1 Compare August 18, 2020 07:37

return if zone_name == 'global'

zone_name
Copy link
Contributor

Choose a reason for hiding this comment

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

vm::zone is 'zone' =>
constexpr static char const* zone = "zone";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks!

def call_the_resolver
@log.debug('Solaris Virtual Resolver')

fact_value = check_ldom || check_zone || 'physical'
Copy link
Contributor

@oanatmaria oanatmaria Aug 18, 2020

Choose a reason for hiding this comment

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

if ldom and zone fail to retrieve the fact this method is called =>

before stating that is physical.

Method definition :

string virtualization_resolver::get_fact_vm(collection& facts)

this is already implemented in linux/virtual (check_other_facts method)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks!

@oanatmaria oanatmaria added the enhancement New feature or enhancement label Aug 18, 2020
@sebastian-miclea sebastian-miclea changed the title (FACT-2721) Added ldom resolver (FACT-2721) Added Solaris virtual fact Aug 18, 2020
@puppetcla
Copy link

CLA signed by all contributors.

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.

🙋🏻‍♀️

product_name = Facter::Resolvers::Solaris.const_get(klass).resolve(:product_name)
bios_vendor = Facter::Resolvers::Solaris.const_get(klass).resolve(:bios_vendor)

return 'kvm' if bios_vendor&.include?('Amazon EC2')
Copy link

Choose a reason for hiding this comment

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

this looks like we're doing work that something else could do better, and more exhaustively

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've updated the PR to also check if the hypervisor is Xen, If that doesn't return anything and bios_vendor is Amazon EC2, then by exclusion, it can only be KVM. If that also fails, it will check hypervisors mapped here by product_name.

@oanatmaria
Copy link
Contributor

Jenkins please test this on solaris11-64a,solaris114-64a,solaris-11-sparca

@sebastian-miclea
Copy link
Contributor Author

Jenkins please test this on solaris11-64a,solaris114-64a,solaris-11-sparca

@sebastian-miclea
Copy link
Contributor Author

Jenkins please test this on solaris11-64a,solaris114-64a,solaris11-sparca

@sebastian-miclea
Copy link
Contributor Author

Jenkins please test this on solaris11-64a,solaris114-64a,solaris11-SPARCa

@oanatmaria oanatmaria merged commit 0a03f68 into puppetlabs:4.x Aug 20, 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