(MAINT) Fixing issue with OS not being in FacterDB's fact set#83
(MAINT) Fixing issue with OS not being in FacterDB's fact set#83
Conversation
Tu2607
force-pushed
the
update-facterdb-ver
branch
2 times, most recently
from
5764471 to
24d5c34
Compare
This commit updates the pinned version for facterdb so that there is support for Ubuntu 24.04. This is the latest version of facterdb before it diverged from supporting open-source Puppet.
Tu2607
force-pushed
the
update-facterdb-ver
branch
15 times, most recently
from
5825903 to
1f6744a
Compare
Tu2607
force-pushed
the
update-facterdb-ver
branch
4 times, most recently
from
4961e79 to
e501219
Compare
…d OS This commit removes the dependency on facterdb in the Benchmark class and instead uses its own hardcoded mapping of suported OSes of sce_linux and sce_windows modules.
Tu2607
force-pushed
the
update-facterdb-ver
branch
from
e501219 to
187f173
Compare
| spec_task.pattern = 'spec/abide_dev_utils_spec.rb,spec/abide_dev_utils/**/*_spec.rb' | ||
| spec_task.pattern = ['spec/abide_dev_utils_spec.rb', | ||
| 'spec/abide_dev_utils/**/*_spec.rb'] |
There was a problem hiding this comment.
pattern needs to be a string, I believe.
There was a problem hiding this comment.
I tried as a string and then an array. Both didn't make the command bundle exec rake spec happy. Currently trying to figure out why.
| # Going to use `rpsec` instead of `bundle exec rake spec` and manually feed it the pattern of spec files to run. | ||
| # `bundle exec rake spec` is running into some issue with the pattern defined in the Rakefle and was only running 8 tests. |
There was a problem hiding this comment.
Typically want to include the error name and context when you comment stuff like this
There was a problem hiding this comment.
It's a strange error. Not even a name to it. Just this
AMER-Tu-Vu:abide_dev_utils tuvu$ bundle exec rake spec
3ede072ed4e5c4a829aa377f788ff268e800cc92 HEAD
dae6cce198b012e2ebcd71b676e8997afb10be71 HEAD
/Users/tuvu/.asdf/installs/ruby/3.3.6/bin/ruby -I/Users/tuvu/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/rspec-core-3.13.6/lib:/Users/tuvu/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/rspec-support-3.13.7/lib /Users/tuvu/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/rspec-core-3.13.6/exe/rspec spec/abide_dev_utils/cli_spec.rb spec/abide_dev_utils/ppt/facter_utils_spec.rb spec/abide_dev_utils/ppt/new_obj_spec.rb spec/abide_dev_utils/sce/benchmark_spec.rb spec/abide_dev_utils/xccdf/diff/benchmark_spec.rb spec/abide_dev_utils/xccdf/parser/objects_spec.rb spec/abide_dev_utils/xccdf/parser_spec.rb spec/abide_dev_utils/xccdf_spec.rb spec/abide_dev_utils_spec.rb
/Users/tuvu/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/puppet-8.16.0-universal-darwin/lib/puppet.rb:38: warning: syslog was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.4.0.
You can add syslog to your Gemfile or gemspec to silence this warning.
.......
Finished in 0.03705 seconds (files took 16.6 seconds to load)
8 examples, 0 failures
/Users/tuvu/.asdf/installs/ruby/3.3.6/bin/ruby -I/Users/tuvu/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/rspec-core-3.13.6/lib:/Users/tuvu/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/rspec-support-3.13.7/lib /Users/tuvu/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/rspec-core-3.13.6/exe/rspec spec/abide_dev_utils/cli_spec.rb spec/abide_dev_utils/ppt/facter_utils_spec.rb spec/abide_dev_utils/ppt/new_obj_spec.rb spec/abide_dev_utils/sce/benchmark_spec.rb spec/abide_dev_utils/xccdf/diff/benchmark_spec.rb spec/abide_dev_utils/xccdf/parser/objects_spec.rb spec/abide_dev_utils/xccdf/parser_spec.rb spec/abide_dev_utils/xccdf_spec.rb spec/abide_dev_utils_spec.rb failed
Looked like it did not like the spec pattern provided through the Rakefile so I poke around but haven't yet find the accurate fix. Doing bundle exec rspec and passed the pattern in worked so it's a bit confusing.
| OS_FAMILY_MAP = { | ||
| 'redhat' => 'RedHat', | ||
| 'oraclelinux' => 'RedHat', | ||
| 'almalinux' => 'RedHat', | ||
| 'rocky' => 'RedHat', | ||
| 'ubuntu' => 'Debian', | ||
| 'windows' => 'Windows', | ||
| }.freeze | ||
|
|
||
| SUPPORT_OS_MAJ_VER_MAP = { | ||
| "redhat" => ['7', '8', '9'], | ||
| "oraclelinux" => ['7', '8', '9'], | ||
| "almalinux" => ['8', '9'], | ||
| "rocky" => ['8', '9'], | ||
| "ubuntu" => ['20.04', '22.04', '24.04'], | ||
| "windows" => ['2016', '10', '2019', '2022', '2025'] | ||
| } |
There was a problem hiding this comment.
I don't think this is the best place for this information. This stuff will need to be updated relatively frequently, and should probably not be buried in a large file full of other stuff.
There was a problem hiding this comment.
Instead of a bunch of workarounds in this specific file, would it be better to look at adding failsafes to FacterUtils instead? Something like providing stubbed fact sets if FacterDB doesn't have the facts?
There was a problem hiding this comment.
I did think of that at first but then concluded it would take more time and effort to spin up a VM and generate facts for a new OS and then format it to Facterdb's format so that our existing code would work. I settled with this approach because it just pulled from the latest mappings and resource data from sce_linux and sce_windows and replicate the objects that we use to load the mapping files and the resource data. I'm open to work on expanding FacterUtils once we have the Ubuntu 24.04 release out of the way since it is a way less hacky way fixing the issue we have with facterdb.
There was a problem hiding this comment.
What I'm getting at is couldn't we put this workaround code into FacterUtils instead of this file? We don't need to return full fact sets, just the facts that we use here.
|
@Tu2607 You should really update the PR title and message to reflect all the changes in this PR |
Tu2607
changed the title
2 participants
This commit updates the pinned version for facterdb so that there is support for Ubuntu 24.04. This is the latest version of facterdb before it diverged from supporting open-source Puppet. Also added failsafes for the event of a new OS being supported by sce_linux and sce_windows but FacterDB does not have fact set for it.