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

Allow ohai versions > 17 to be used #1040

Merged
merged 1 commit into from Nov 10, 2021

Conversation

balasankarc
Copy link

@balasankarc balasankarc commented Oct 8, 2021

Description

The restricton < 17 was originally added by 2777c5b in #1013 because omnibus-toolchain was still shipping Ruby 2.6 at that time, and Ohai 17 required Ruby 17. However, starting with chef/omnibus-toolchain#153, omnibus-toolchain is on Ruby 2.7+, and hence this restriction can be removed.

Without this, omnibus can't be used with Chef > 17 because of conflicting Ohai requirements between two.


Maintainers

Please ensure that you check for:

  • [] If this change impacts git cache validity, it bumps the git cache
    serial number
  • [] If this change impacts compatibility with omnibus-software, the
    corresponding change is reviewed and there is a release plan
  • [] If this change impacts compatibility with the omnibus cookbook, the
    corresponding change is reviewed and there is a release plan

@balasankarc
Copy link
Author

CI runs tests on Ruby 2.6. If we are to keep that, we might need something like the following in the gemspec.

diff --git a/omnibus.gemspec b/omnibus.gemspec
index 18b7f62e..6980d952 100644
--- a/omnibus.gemspec
+++ b/omnibus.gemspec
@@ -25,7 +25,15 @@ Gem::Specification.new do |gem|
   gem.add_dependency "chef-cleanroom",   "~> 1.0"
   gem.add_dependency "ffi-yajl",         "~> 2.2"
   gem.add_dependency "mixlib-shellout",  ">= 2.0", "< 4.0"
-  gem.add_dependency "ohai",             ">= 15"
+
+  # Ohai 17 requires Ruby 2.7 or later. If we are on an older Ruby, we pin Ohai
+  # to be < 17
+  if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.7')
+    gem.add_dependency "ohai",             ">= 15", "< 17"
+  else
+    gem.add_dependency "ohai",             ">= 15"
+  end
+
   gem.add_dependency "ruby-progressbar", "~> 1.7"
   gem.add_dependency "thor",             ">= 0.18", "< 2.0"
   gem.add_dependency "license_scout",    "~> 1.0"

@tas50
Copy link
Contributor

tas50 commented Oct 8, 2021

You'll want to remove the Ruby 2.6 testing here https://github.com/chef/omnibus/blob/main/.expeditor/verify.pipeline.yml#L13

omnibus.gemspec Outdated Show resolved Hide resolved
@tas50
Copy link
Contributor

tas50 commented Oct 8, 2021

It's probably a call for @lamont-granquist and @marcparadise, but it may just be time to nuke Ruby 2.6 support entirely.

@mttkay
Copy link

mttkay commented Oct 8, 2021

It's probably a call for @lamont-granquist and @marcparadise, but it may just be time to nuke Ruby 2.6 support entirely.

👍 Keep in mind we are merely 6 months away from EOL for Ruby 2.6: https://www.ruby-lang.org/en/downloads/branches/

So this should likely happen before end of March 2022 anyway.

The restricton `< 17` was originally added by 2777c5b in
chef#1013 because `omnibus-toolchain`
was still shipping Ruby 2.6 at that time, and Ohai 17 required Ruby 17.
However, starting with
chef/omnibus-toolchain#153, `omnibus-toolchain`
is on Ruby 2.7+, and hence this restriction can be removed.

To be on the safe side, we are bumping the upper limit to 18 to avoid
any surprises when next major version of Ohai comes out.

Signed-off-by: Balasankar "Balu" C <balasankar@gitlab.com>
@balasankarc
Copy link
Author

@lamont-granquist @marcparadise ping. Could you comment on whether we want Ruby 2.6 support retained or removed from CI?

@balasankarc
Copy link
Author

@tas50 @lamont-granquist @marcparadise Gentle reminder. :)

@lamont-granquist lamont-granquist merged commit e0d467f into chef:main Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants