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

Add support for Ruby 3.0 #30

Merged
merged 3 commits into from
May 6, 2023

Conversation

billthompson
Copy link
Contributor

What/Why

First, thanks for the gem! We have been using it for about 2 years with no issues and it has been really helpful for our background processing workflows. However, it's time for us to upgrade to Ruby > 2.7 and have ran into an issue here.

This is an alternative implementation of #28.

In our app, we were getting the following error after upgrading to Ruby 3:

ERROR: wrong number of arguments (given 3, expected 1..2) - /opt/app/vendor/bundle/ruby/3.1.0/gems/kubeclient-4.11.0/lib/kubeclient.rb:22:in ...

Passing the keyword argument as the last hash parameter is deprecated in Ruby 3.0 and this requires a change in this repo as Resque::Kubernetes::ContextFactory::Context declares the client options to be a hash and sends them over as a hash when building the kubeclient. Also see Kubeclient::Client#initialize. To make it clear, going forward we are explicitly using hashes for the options internally and double splatting them when instantiating a kubeclient.

While Kubeclient added Ruby 3.0 support in version 4.9.2, they also fixed a major security vulnerability in 4.9.3. Given that neither of these were backported to 3.x, I think the project will need to drop the dual support to move forward. I have removed 3.x from the appraisals, but left the structure in place in case we want to support 4.x and 5.x in the future; I can completely remove the dependency if you would prefer.

Additionally updating all get_ENTITY methods to explicitly use a hash for its arguments as that aligns with the method signature.

Upgrades to bundler 2.x since that is a default gem for Ruby 3.

If there's anything you need from me to get this merged, just let me know!

How was it tested?

Ran the test suite locally using Ruby 3.0.5.

Additionally deployed this branch to our integration environment that creates GCE resources via the workflow: App -> Resque -> Kubernetes Job -> Create GCE VM and confirmed that the Kubernetes Jobs were started and stopped as expected based on workload.

@jeremywadsack
Copy link
Member

@billthompson Thanks for this PR and the excellent write-up and reasoning. Also really appreciate the clear details on how you tested this.

I much prefer this to the approach in #28 that depended on checking the Ruby version number. Originally, I wanted to ensure that this was backwards compatible with Ruby 2.x, but as that's reaching EOL on Mar 31, 2023, I think we can move the repo forward. Especially, as this is a breaking change with the dependency update to Ruby and Kubeclient. (Folks running ruby 2.7 in legacy systems can continue to use the older versions of the gem.)

I want to test this in our set up before final approval. While I'm doing that can you update the CHANGELOG.md with a note about the change? I don't think there are any needed changes to the README.md, unless you see something I missed.

@jeremywadsack jeremywadsack self-assigned this Mar 6, 2023
@billthompson
Copy link
Contributor Author

Thanks for the quick response @jeremywadsack! I appreciate the double check on this.

I have updated the changelog and noticed that the ownership of the kubeclient gem has transitioned over to ManageIQ so I updated that repo reference in the readme. Otherwise, everything else looks good to me.

@jeremywadsack jeremywadsack merged commit da389e7 into keylimetoolbox:master May 6, 2023
@jeremywadsack jeremywadsack mentioned this pull request May 6, 2023
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.

2 participants