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

Add support for OmniOS (OpenSolaris derivative) #2691

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ptrrkssn
Copy link

A couple of small fixes that enables "facter os" to give relevant info in the release hash instead of null, null, null (so other facts won't fail) and gives the OS name as "OmniOS" instead of "Solaris"..

Before:

facter os

{
architecture => "i86pc",
family => "Solaris",
hardware => "i86pc",
name => "Solaris",
release => {
full => null,
major => null,
minor => null
}
}

After:

facter os

{
architecture => "i86pc",
family => "Solaris",
hardware => "i86pc",
name => "OmniOS",
release => {
full => "11.r151048",
major => "11",
minor => "r151048"
}
}

cat /etc/release

OmniOS v11 r151048m
Copyright (c) 2012-2017 OmniTI Computer Consulting, Inc.
Copyright (c) 2017-2024 OmniOS Community Edition (OmniOSce) Association.
All rights reserved. Use is subject to licence terms.

uname -a

SunOS servername 5.11 omnios-r151048-4a265be889 i86pc i386 i86pc

@ptrrkssn ptrrkssn requested a review from a team as a code owner March 21, 2024 07:59
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@CLAassistant
Copy link

CLAassistant commented Mar 21, 2024

CLA assistant check
All committers have signed the CLA.

@joshcooper joshcooper added the enhancement New feature or enhancement label Apr 11, 2024
@@ -9,7 +9,8 @@ class Name

def call_the_resolver
value = Facter::Resolvers::Uname.resolve(:kernelname)
fact_value = value == 'SunOS' ? 'Solaris' : value
version = Facter::Resolvers::Uname.resolve(:kernelversion)
fact_value = value == 'SunOS' ? (version =~ /^omnios-/ ? 'OmniOS' : 'Solaris') : value
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fact_value = value == 'SunOS' ? (version =~ /^omnios-/ ? 'OmniOS' : 'Solaris') : value
fact_value = case value
when 'SunOS'
'Solaris'
when /^omnios-/
'OmniOS'
else
value
end

Also could you add unit a test?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ptrrkssn could you take a look at ^ You may want to rebase too.

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.

Also some tests are failing, could you take a look?

@@ -9,7 +9,15 @@ class Name

def call_the_resolver
value = Facter::Resolvers::Uname.resolve(:kernelname)
fact_value = value == 'SunOS' ? 'Solaris' : value
version = Facter::Resolvers::Uname.resolve(:kernelversion)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is duplicate

Suggested change
version = Facter::Resolvers::Uname.resolve(:kernelversion)

Copy link
Author

@ptrrkssn ptrrkssn left a comment

Choose a reason for hiding this comment

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

That update doesn't work - For OmniOS kernel name is SunOS so we need to look for kernelversion starting with omnios-

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