-
Notifications
You must be signed in to change notification settings - Fork 494
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-3121) Detect Illumos (SmartOS/OmniOS) LX virtualization #2492
Conversation
Can one of the admins verify this patch? |
f5d7556
to
31ba894
Compare
(Hmm, the test failures don't seem to be related to the changes in this PR.) |
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.
Hi, I’m one of the maintainers for SmartOS. It looks to me like the behavior presented here is desirable from our perspective. (Though, I have not evaluated this code functionally, I’ll leave that to those who are more familiar).
Hi, @joshcooper. I saw that you merged some PRs in this repo over the last few months. Are you available to review this PR sometime soon? |
Closing and reopening to retrigger tests |
Hi @smokris , thank you for your contribution and sorry for the delay. Would you be able to rebase you PR and resolve the conflict? Thank yoU! |
a6107cb
to
d959b40
Compare
@mhashizume, yes, I pushed d959b40 a few minutes ago, in which I've rebased onto current |
d959b40
to
430d549
Compare
GitHub CI is running the tests in a different random order each time, and the tests were failing intermittently depending on the order they ran. For example, If I understand correctly, the problem was that, if the test calls a resolver that isn't mocked, then the resolver will execute on the current system and cache its results (rather than merely returning the mocked results without caching), and the cached results break subsequent tests (even if mocked). I pushed 430d549, which I think adds all the necessary mocks to enable the tests to pass regardless of order. |
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.
Thank you for your contribution @smokris !
Closing and reopening to pick up new check |
See https://tickets.puppetlabs.com/browse/FACT-3121.
Facter uses
virt-what
's output which islxc
(onvirt-what
1.20) or null (onvirt-what
1.21+), so I addedcheck_illumos_lx
prior toretrieve_from_virt_what
.