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

PEREL-2701: build for ubuntu jammy #16

Merged
merged 1 commit into from
Dec 30, 2021

Conversation

jvperrin
Copy link
Member

I think the main controversial thing in this review is pulling in libssl-dev (1.1.1l-1ubuntu1) from impish for jammy, since the version of libssl-dev in jammy currently is already on openssl 3.0 and Ruby doesn't support that just yet. I'm open to other suggestions (could build openssl from source instead, but that sounds painful), but this seemed like the best way to go for now.

After building this, I copied the package to a new 22.04 AMI and ran puppet there. It eventually got to this stage, so everything seems to be working well enough:

amazon-ebs: Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Evaluation Error: Error while evaluating a Function Call, I'm not programmed to install python on jammy [...]

Other things done:

  • I upgraded ruby-build to a newer version (mostly to test with Ruby 3.0.3, which didn't end up working).
  • I combined all the itests into one, since they all were symlinks to itest/ubuntu.sh anyway, so there was no difference between them.

I don't think this should need a new release or anything, since nothing much is changing for the releases that are already built for (trusty/xenial/bionic/focal), so only jammy needs a new package actually built and uploaded.


ADD vendor/ruby-build-20211203.tar.gz /tmp

# PEREL-2701: Download and install libssl-dev 1.1 from impish, since jammy

Choose a reason for hiding this comment

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

I am not really a fan of this. Do we have an idea of if/when this might be fixed upstream? I'd rather wait a couple weeks than merge this now. If we do merge this we probably want a ticket to clean this up that blocks finishing the jammy project

Choose a reason for hiding this comment

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

...I guess if this is just to build the package it's not too bad, as long as we don't need to pull in the impish package to run this

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it's only as a build-time dependency (it uses libssl1.1 at runtime), and I'm definitely not a fan either... I wish they published both libssl1.1-dev and libssl3-dev or something like they do for runtime packages (libssl1.1 and libssl3 do both exist alongside each other).

The problem is that I'm not sure how long this will take before it's fixed, and if it is fixed I expect it will only be for the latest ruby version (3.1.0 I suppose?). Since ruby releases come out on Christmas, I'm happy to wait until after this one and try out 3.1.0, but without ruby/openssl#369 and something like ruby/ruby#4904 merged in and released as a version I don't think it'll work yet. There's active work on this though, as ruby/openssl#399 (comment) was added just yesterday (!)

Another option I was toying with was unvendoring ruby itself from this package and using the system ruby instead. I think that'd be nice, but seems like a big undertaking to do (and especially if doing so for all OS releases, not just jammy). I think that also somewhat defeats the original purpose of using an omnibus package here to include all the dependencies all together, although there are things like libssl/libxml/libxslt/virt-what that this depends on anyway, so maybe that's not that big of a difference. As far as I know, ruby doesn't have something like virtualenvs to make a more self-contained runtime with a specific set of gems, but maybe bundler into a separate directory would accomplish something similar.

Copy link

@bobtfish bobtfish left a comment

Choose a reason for hiding this comment

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

I'm okay with this to get a working package for now, but it would be great to ticket and revisit depending upon what happens in upstream ruby

Copy link

@mtbentley mtbentley left a comment

Choose a reason for hiding this comment

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

ship, with the same comment as @bobtfish

@jvperrin
Copy link
Member Author

I'm okay with this to get a working package for now, but it would be great to ticket and revisit depending upon what happens in upstream ruby

Sounds good! I'm going to leave this until Ruby 3.1.0 comes out (should be in ~ 3 days?) to see if that makes any difference, but otherwise I'll merge + ticket. I do expect this will all be figured out by the actual Jammy release in April 2022 though, so it's definitely worth revisiting this.

Copy link

@ahaswell ahaswell left a comment

Choose a reason for hiding this comment

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

Nothing to add that hasn't already been said other than nice to see that this gets us to the first of the linear blocks for Q1 and thanks for getting it working!

@jvperrin
Copy link
Member Author

jvperrin commented Dec 29, 2021

So I did get this building with Ruby 3.1.0 since that does work with OpenSSL 3.0, but I've kept it in a separate branch: u/jvperrin/PEREL-2701-use-openssl-3.

While it builds into a package, it doesn't actually work yet, this is the first error I encountered that would need a patch to our puppet fork:

Error: Could not load /etc/puppet/puppet.conf: no implicit conversion of Hash into Integer
Error: Could not autoload puppet/type/file: undefined method `escape' for URI:Module

    params[:path] = URI.escape(path)
                       ^^^^^^^
Error: Could not create resources for managing Puppet's files and directories in sections [:main, :agent, :ssl]: Could not autoload puppet/type/file: undefined method `escape' for URI:Module

    params[:path] = URI.escape(path)
                       ^^^^^^^
Error: Could not prepare for execution: Could not create resources for managing Puppet's files and directories in sections [:main, :agent, :ssl]: Could not autoload puppet/type/file: undefined method `escape' for URI:Module

    params[:path] = URI.escape(path)
                       ^^^^^^^
Could not autoload puppet/type/file: undefined method `escape' for URI:Module

    params[:path] = URI.escape(path)
                       ^^^^^^^

(URI.escape was removed in Ruby 3.0)

The issue I think will be that I don't know how many more patches would be needed to get this working (on top of all the changes made in that branch), and from https://puppet.com/docs/puppet/7/platform_lifecycle.html#about_agent-component-version-numbers even the latest puppet-agent release (7.x) is still built containing Ruby 2.7 and doesn't support Ruby 3.x or OpenSSL 3.0 yet, so it might not even be something that has any support from even newer puppet versions yet.

I'm going to merge this instead and go with OpenSSL 1.1 for now. I think upgrading to Puppet 7.x (or 8.x?) when it supports Ruby 3.0 might be best, instead of trying to get our Puppet 4.5.3 working with much newer stuff. Another option would be to use the version of puppet (5.5) packaged with Ubuntu (https://packages.ubuntu.com/jammy/puppet), which is probably going to be 6.16 or above soon, if https://metadata.ftp-master.debian.org/changelogs//main/p/puppet/puppet_6.16.0-1_changelog is any indication. Then at least it should work with the OS version out of the box, but things like the puppet SSL certs, state, etc. will be in different locations, and something different would need to be done for puppetmasters without using the nginx bundled in this package, so that'd be a decent amount of work.

@jvperrin jvperrin merged commit 45a6f45 into master Dec 30, 2021
@jvperrin jvperrin deleted the u/jvperrin/PEREL-2701-build-for-ubuntu-jammy branch December 30, 2021 00:18
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.

4 participants