-
Notifications
You must be signed in to change notification settings - Fork 495
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
Conversation
03adf43
to
19b89a1
Compare
lib/facter/facts/solaris/virtual.rb
Outdated
|
||
return if zone_name == 'global' | ||
|
||
zone_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return vm::zone; |
facter/lib/inc/facter/facts/vm.hpp
Line 139 in 4844068
constexpr static char const* zone = "zone"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, thanks!
lib/facter/facts/solaris/virtual.rb
Outdated
def call_the_resolver | ||
@log.debug('Solaris Virtual Resolver') | ||
|
||
fact_value = check_ldom || check_zone || 'physical' |
There was a problem hiding this comment.
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 =>
return get_fact_vm(facts); |
Method definition :
string virtualization_resolver::get_fact_vm(collection& facts) |
this is already implemented in linux/virtual (check_other_facts method)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, thanks!
19b89a1
to
e46cba4
Compare
CLA signed by all contributors. |
There was a problem hiding this 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') |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
e46cba4
to
9a658e3
Compare
Jenkins please test this on solaris11-64a,solaris114-64a,solaris-11-sparca |
9a658e3
to
5d7b20e
Compare
Jenkins please test this on solaris11-64a,solaris114-64a,solaris-11-sparca |
Jenkins please test this on solaris11-64a,solaris114-64a,solaris11-sparca |
Jenkins please test this on solaris11-64a,solaris114-64a,solaris11-SPARCa |
No description provided.