Rails 7 upgrade

evgeni · November 6, 2024, 8:29am

I took the liberty to poke packaging:

github.com/theforeman/foreman-packaging

Rails 7.0.8.6

theforeman:rpm/develop ← theforeman:rails70

opened 08:28AM - 06 Nov 24 UTC

evgeni

+65 -26

There is an all-in-one copr at:
https://copr.fedorainfracloud.org/coprs/evgeni/rails70/

ekohl · November 12, 2024, 5:17pm

During today’s session @ofedoren shared that he thinks we’ve pretty much done all we can and are now mostly blocked by plugin owners.

In Fixes #37997 - Support Rails 7.0 by ofedoren · Pull Request #644 · theforeman/foreman_discovery · GitHub there’s a small change that waits for an entry to be present. We also saw that was needed somewhere else so it’s a question whether the application got slower or if this is just something in the test stack. It could even be that the test stack got faster, uncovering a race condition. We don’t have good infrastructure to test this on production builds. @ekohl made a note to speak to Red Hat’s performance team to see what they have.

Then there’s the change in Rails 7 defaults: it changes the default hashing algorithm from SHA1 to SHA256: Fixes #37825 - Upgrade Rails to 7.0 by ofedoren · Pull Request #10299 · theforeman/foreman · GitHub. According to Rails’ notes this impacts various things like caching, cookies, encrypted messages and etags. Those are probably not an issue for us, but something we need to keep an eye on and include a release note that users will be logged out when upgrading. We may want to add tasks to explicitly purge existing sessions and caches just to clean them up. Etags probably don’t affect us since Apache serves the assets and they have a unique timestamp in their filename with a long expiration time, so we don’t rely on etags.

It may be an issue for Katello which uses the hashing to construct Candlepin IDs. It’s unclear to us if this is a concern, but this is where we need the plugin team to weigh in (hello @katello).

Then for foreman_theme_satellite there’s the first big Rails 6.1 vs 7.0 difference: Support Rails 7.0 by ofedoren · Pull Request #97 · RedHatSatellite/foreman_theme_satellite · GitHub. While not required for the upstream community, the people currently working on this effort do care. It’s also worth highlighting it for other plugin authors.

We’ve agreed that the goal for next week is to get in a state where we can build RPMs (even for PRs that aren’t merged) and perform end-to-end testing. The week after we should know pretty well if we’re good to pull the trigger and merge all the patches. So stay tuned for at least the next 2 updates.

jeremylenz · November 12, 2024, 5:29pm

Do you mean this SecureRandom.random_number? As long as that syntax isn’t changing, I don’t think it should be an issue.

ekohl · November 12, 2024, 5:42pm

Quoting the relevant Katello bits from the linked discussion:

github.com

Katello/katello/blob/c03c7e292639a6b8e4af4b38886d947d3067ad56/app/lib/katello/util/data.rb#L9


      
          module Katello
            module Util
              module Data
                def self.array_with_indifferent_access(variable)
                  variable.map { |x| x.with_indifferent_access }
                end
          
                def self.hexdigest(string)
                  defined?(ActiveSupport::Digest) ? ActiveSupport::Digest.hexdigest(string) : Digest::MD5.hexdigest(string)
                end
          
                def self.ostructize(obj, options = {})
                  options[:prefix_keys] ||= []
                  options[:prefix] ||= '_'
          
                  case obj
                  when Hash
          
                    ostructized_hash = {}

This is called later:

github.com

Katello/katello/blob/c03c7e292639a6b8e4af4b38886d947d3067ad56/app/models/katello/content_view_environment.rb#L116-L122


      
          if default_environment?
            self.label ||= environment.label
            self.cp_id ||= Katello::Util::Data.hexdigest(environment.organization.label)
          else
            self.label ||= [environment.label, content_view.label].join('/')
            self.cp_id ||= Katello::Util::Data.hexdigest([environment.id, content_view.id].join('-'))
          end

Changing how cp_id is generated can have significant implications, but it could also be completely harmless. That’s what we need to figure out.

jeremylenz · November 12, 2024, 5:55pm

When creating a Candlepin environment (which corresponds to a Katello content view environment) we do this:

github.com

Katello/katello/blob/a444a646bf3bfa326d563edd894574f1df176159/app/services/katello/content_view_manager.rb#L15-L21


      
            ::Katello::Resources::Candlepin::Environment.create(
              content_view_environment.content_view.organization.label,
              content_view_environment.cp_id,
              content_view_environment.label,
              content_view_environment.content_view.description.try(:truncate, 255)
            )
          end

which calls katello/app/lib/katello/resources/candlepin/environment.rb at a444a646bf3bfa326d563edd894574f1df176159 · Katello/katello · GitHub

which calls Candlepin’s API POST /owners/{owner_key}/environments and feeds Candlepin whatever ID we generated. Candlepin just saves the object with the ID we give it.

On my system it creates IDs that look like this:

Katello::ContentViewEnvironment.all
=> [#<Katello::ContentViewEnvironment:0x00007f85b9bd58d0
  id: 1,
  name: "Library",
  label: "Library",
  cp_id: "64c2cce3716a5bf67b0413fac95e5daa",
  content_view_id: 1,
  created_at: Wed, 06 Nov 2024 16:30:22.091071000 UTC +00:00,
  updated_at: Wed, 06 Nov 2024 16:30:22.091071000 UTC +00:00,
  environment_id: 1,
  content_view_version_id: 1>,
 #<Katello::ContentViewEnvironment:0x00007f85b9b97670
  id: 2,
  name: "Library/cv1",
  label: "Library/cv1",
  cp_id: "380e1992321843ee64331a37f9a440a2",
  content_view_id: 2,
  created_at: Tue, 12 Nov 2024 13:04:03.211110000 UTC +00:00,
  updated_at: Tue, 12 Nov 2024 13:07:24.883810000 UTC +00:00,
  environment_id: 1,
  content_view_version_id: 3>]

So I don’t immediately foresee any problems, unless I’m missing something. Will double-check with other Katello folks too.

ekohl · November 12, 2024, 6:06pm

That’s sort of what I expected, but if they’re opaque IDs, would it be better to use UUIDs?

Partha_Aji · November 12, 2024, 6:14pm

So as long as we can create a digest or key based of name and id we should be ok. The cp_ids used to be random numbers before and then got changed to a computed value when it was impacting vcr tests. ( look at Fixes #19099 - Use consistent candlepin requests by jlsherrill · Pull Request #6717 · Katello/katello · GitHub )

ekohl · November 19, 2024, 2:47pm

Going over the board we have a few issues in progress.

Most notable is Katello. There was a follow up of the above discussion and that needs to be reflected. To make packaging easier we briefly talked about the angular-rails-templates dependency. During the meeting we weren’t sure if it should be split off, but since I’ve decided that’s best. Fixes #38018 - Rails 7 compatible angular-rails-templates by ekohl · Pull Request #11222 · Katello/katello · GitHub has already been merged and Refs #38018 - Update Katello's angular-rails-templates version by ekohl · Pull Request #11489 · theforeman/foreman-packaging · GitHub should be done soon as well. That means we can just update it in nightly and not maintain it as a separate build in our COPR.

Then Fixes #37848 - Support Rails 7.0 by ofedoren · Pull Request #739 · theforeman/foreman_ansible · GitHub & Fixes #37991 - Support Rails 7.0 by ofedoren · Pull Request #78 · theforeman/foreman_google · GitHub can both be merged & released and by the time I write these notes they’re already merged. Again, having those in nightly simplifies testing.

Then there’s the core PR. Foreman itself (including develop) is currently red and Fixes #38017 - Update libvirt gems & fix tests by stejskalleos · Pull Request #10377 · theforeman/foreman · GitHub aims to resolve that.

Our aim is to get anything that can be merged & released in nightly by tomorrow. Then we just have Foreman, Rails itself and possibly Katello. That should simplify end-to-end testing. We aim to get a good answer on that before next week’s meeting. If all goes well, we can press the big green buttons then.

We will need help from @katello to get Refs #38018 - Update Katello's angular-rails-templates version by ekohl · Pull Request #11489 · theforeman/foreman-packaging · GitHub reviewed. Additionally, @lstejska is already working on the foreman_ansible & foreman_google tasks.

ekohl · November 26, 2024, 12:56pm

In the past week @ofedoren found that the constraints we had set on routes in foreman_ansible and foreman_openscap in the past weren’t quite right. By now he found the fix:

Fixes #38031 - Use the same routes constraints as Foreman by ofedoren · Pull Request #743 · theforeman/foreman_ansible · GitHub
Fixes #38032 - Use the same routes constraints as Foreman by ofedoren · Pull Request #586 · theforeman/foreman_openscap · GitHub

The Katello update PR has been split and all compatible preparations have been merged while the main PR has been ACKed.

For various reasons Katello nightly hasn’t been updated since 4.15 branched and we’re currently working on getting that green. That allows us to get all changed merged & released.

The current goal is to make sure that by Thursday all preparation changes are available in nightly. By then we should also update evgeni/rails70 Copr to include patched Foreman and Katello RPMs to perform end-to-end testing. Then by next week’s meeting we should be ready to plan the actual merge.

ekohl · December 3, 2024, 2:02pm

Last week we finally got nightly green again, which means updates are properly flowing again. The goal for this week is to make sure all changes make it into releases where possible and build a copr repository with those that can’t (Foreman & Katello). @ofedoren will make sure we know which releases are missing and ping people where possible while @ekohl takes care of the latter. Once we have that, @ekohl will post testing instructions here so we can take it for a spin before merging all changes.

ekohl · December 10, 2024, 2:10pm

We made changes and even though we didn’t manage to completely end-to-end test with packages, we feel confident enough to merge:

Goal is to get this all stable before the holidays, which for some means it needs to happen this week.

ekohl · December 20, 2024, 4:34pm

I’m going to mark this as resolved since it’s been merged to nightly and nightly has been passing since. While some plugins may still need work, it’s IMHO separate from the main effort now.

quba42 · December 30, 2024, 11:24am

Can you still post what the first Foreman/Katello version to use Rails 7 will be?

ekohl · December 30, 2024, 12:51pm

Current nightly, which will be Foreman 3.14 and Katello 4.16.