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

(FACT-3163) Add support for OpenBSD #2642

Merged
merged 2 commits into from
Mar 12, 2024
Merged

Conversation

buzzdeee
Copy link
Contributor

(FACT-3163) Add support for OpenBSD

Most inspiration taken from FreeBSD, rest from other OS,
and some own creations. Not all core facts supported, but
makes puppet facts works well, as well as puppet agent --test

Developed/Tested on OpenBSD 7.2, AMD64, Puppet 7.20.0, Ruby 3.1.2

also disable RuboCop Metrics/PerceivedComplexity
and Metrics/CyclomaticComplexity
as suggested by @joshcooper

Additionally:
Case insensitive OS detection

ran into trouble when facter installed via puppetserver gem install,
there the OS returned seems to be OpenBSD instead of all lowercase.

@buzzdeee buzzdeee requested a review from a team as a code owner November 18, 2023 21:18
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@buzzdeee
Copy link
Contributor Author

@joshcooper

this is the follow-up of #2531

I believe what was missing, and what confused the random order was that the OpenBSD related tests were missing in ./spec/facter/util/facts/posix/virtual_detector_spec.rb because in lib/facter/util/facts/posix/virtual_detector.rb some OpenBSD related detections were added. Also seems, the "before do" has to be kind of in-order with how the fact checks are called in virtual_detector.rb

Now everything aligned, I can't reproduce the issue anymore. I did let the tests run for some time in random order.

@joshcooper
Copy link
Contributor

Thanks @buzzdeee can you squash your commits?

@buzzdeee
Copy link
Contributor Author

@joshcooper sure, squashed and rebased, hope all test going to pass again (:

@joshcooper joshcooper added the enhancement New feature or enhancement label Dec 11, 2023
fix order dependency of tests when run:
$ rspec -fd './spec/facter/resolvers/openbsd/virtual_spec.rb[1:2:1]' \
'./spec/facter/util/facts/posix/virtual_detector_spec.rb[1:1:12:1]' \
--order random:17181

additionally recognized, the 'virtual' fact didn't return 'physical'
when on a physical machine.
Most inspiration taken from FreeBSD, rest from other OS,
and some own creations. Not all core facts supported, but
makes puppet facts works well, as well as puppet agent --test

Developed/Tested on OpenBSD 7.2, AMD64, Puppet 7.20.0, Ruby 3.1.2

also disable RuboCop Metrics/PerceivedComplexity
and Metrics/CyclomaticComplexity
as suggested by @joshcooper

Additionally:
Case insensitive OS detection

ran into trouble when facter installed via puppetserver gem install,
there the OS returned seems to be OpenBSD instead of all lowercase.
@buzzdeee
Copy link
Contributor Author

there have been merge errors, rebased ...

@buzzdeee
Copy link
Contributor Author

buzzdeee commented Mar 8, 2024

OpenBSD went into ABI lock for 7.5 release, if this would be merged, and make it into the next release hopefully getting out before OpenBSD ports tree freeze, would be awesome.
As the patches are in the ports tree, this is not so much an issue for the agent, but it would make bootstrapping a puppetserver on OpenBSD way less error prone.

Copy link
Contributor

@smortex smortex left a comment

Choose a reason for hiding this comment

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

This look good and there seems to be decent test coverage for the new code so LGTM.

As somebody who did similar work for FreeBSD, I feel like this can be merged as-is. Maybe some corner cases are not correctly handled by this code, but it is a move in the right direction and will allow small and easy to review pull-request to fix them if needed in the future. OpenBSD not being officially supported by Puppet (just like FreeBSD), a "not-perfect-from-the-first-release" seems more acceptable than for officially supported operating systems.

Well done @buzzdeee ! 👏 🎉

@AriaXLi AriaXLi merged commit d26a217 into puppetlabs:main Mar 12, 2024
17 checks passed
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.

5 participants