-
Notifications
You must be signed in to change notification settings - Fork 24
Conversation
Can one of the admins verify this patch? |
385226c
to
010a16f
Compare
010a16f
to
5723eec
Compare
"Bsd": [ | ||
"Freebsd" | ||
] | ||
} |
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.
FreeBSD is definitively a descendant of BSD, yet BSD is not a decedent of Solaris (amusingly, early version of Sun OS (which became Solaris at some point) was based on BSD).
For now, this is convenient because Solaris facts and resolvers provide ZFS related facts which are working on FreeBSD, however, ZFS on Linux is a thing and some refactoring will be necessary at some point. Don't know how / where to log this so that it can be tracked…
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.
And maybe it is as simple as renaming the Solaris ZFS resolver to make it obvious it is not Solaris specific and use similar facts for FreeBSD, Solaris and maybe Linux to gather these facts… So basically the same code facts multiple times, but maybe it's not a problem?
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.
@smortex I talked with @oanatmaria and maybe we can have something like
...
{
"Unix": [
"Solaris",
{
"Bsd": [
"Freebsd"
]
}
]
},
...
Both Solaris
and Bsd
will inherit from Unix
. This is based on the diagram from https://upload.wikimedia.org/wikipedia/commons/7/77/Unix_history-simple.svg
@fact_list[:installed_kernel] = freebsd_version_results[0].strip | ||
@fact_list[:running_kernel] = freebsd_version_results[1].strip | ||
@fact_list[:installed_userland] = freebsd_version_results[2].strip |
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.
.strip will raise NoMethodError if output is empty string or if freebsd_version_results[0/1/2] is nil
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.
freebsd-version -kru
is expected to return exactly 3 lines (each flag produce a line), so I do not thing this is a problem for us here. Yet, some FreeBSD forks might be detected as "FreeBSD" while not providing freebsd-update
, so it's worth being sure that the command succeeded.
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.
HardenedBSD doesn't.
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.
@igalic did you have the opportunity to try it on HardenedBSD? I need to allocate some time to dig in this project…
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.
unfortunately, my current priority
canonical/cloud-init#363 is a few yak stacks away from testing HardenedBSD
This program is always supposed to be found and return the expected output on FreeBSD but make sure it runs successfully, catpure stderr in case it becomes used at some point, and do not raise exceptions if the resolver fails.
5edea77
to
ca9c35e
Compare
If the resolver fails to gather the fact, return an empty resolved fact instead of an empty array.
spec/facter/resolvers/utils/windows/network_utils_spec.rb:37:7: C: RSpec/EmptyHook: Empty hook detected. before do ^^^^^^
Great job with this PR @smortex and @oanatmaria ! |
This PR provide support for Facter 3.x
os.release
facts on FreeBSD to Facter 4:Before:
After:
Compare to: