Skip to content

(FACT-2721) Added Solaris virtual fact#2033

Merged
oanatmaria merged 1 commit intopuppetlabs:4.xfrom
sebastian-miclea:FACT-2721
Aug 20, 2020
Merged

(FACT-2721) Added Solaris virtual fact#2033
oanatmaria merged 1 commit intopuppetlabs:4.xfrom
sebastian-miclea:FACT-2721

Conversation

@sebastian-miclea
Copy link
Copy Markdown
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
Comment thread lib/facter/facts/solaris/virtual.rb Outdated

return if zone_name == 'global'

zone_name
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks!

Comment thread lib/facter/facts/solaris/virtual.rb Outdated
def call_the_resolver
@log.debug('Solaris Virtual Resolver')

fact_value = check_ldom || check_zone || 'physical'
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

CLA signed by all contributors.

Copy link
Copy Markdown

@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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor

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

@sebastian-miclea
Copy link
Copy Markdown
Contributor Author

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

@sebastian-miclea
Copy link
Copy Markdown
Contributor Author

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

@sebastian-miclea
Copy link
Copy Markdown
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