Openfoodnetwork: [Rails 5.0] Dual boot

Created on 18 Feb 2020  路  33Comments  路  Source: openfoodfoundation/openfoodnetwork

What we should change and why (this is tech debt)

we agreed we will try to set up dual boot. This means we would be able to make the upgrade changes directly in master without having to create a long-lived branch for it.

This is more challenging than a simple Rails dual boot because of the dependency on Spree and the fact that the DB schema depends on it too.

The goal is to investigate how that would look like for OFN and come up with a plan to discuss in the next Spree upgrade meeting.

Some resources from the Rails community I collected over time that might be helpful:

Context

This came up in https://openfoodnetwork.slack.com/archives/CAVTM01QB/p1578401534024500 and recent discussions. Long-lived branches bring a lot of pain.

Impact and timeline

This should make the upgrade be delivered earlier and more reliably as we'll be able to do it in smaller and more manageable chunks.

good first issue hackathon spike tech debt

Most helpful comment

thanks for your feedback Andy.

yes, if statements would go into the code. But only for "next version" code that is not compatible with "previous version".
This rails 4.2 code is compatible with rails 4.0 so no if statement necessary, just the commit to master:
image

But this rails 4.2 code is not compatible with rails 4.0 so we would need an if statement:
image
Like this

if ENV['DEPENDENCIES_NEXT']
    self.responder = ApplicationResponder
end

Here we need if and else:
image

if ENV['DEPENDENCIES_NEXT']
    include ActionView::Layouts
else
    include AbstractController::Layouts
end

things would be a bit more complicated with class definitions and inheritance like here:
image
But we would find a way with an if statement.

These examples are all from https://github.com/openfoodfoundation/openfoodnetwork/pull/6301

All 33 comments

It would be good to get a t-shirt size for this. I think it might be quite a lot of work, no?

This should be a few hours of investigation. That is why I marked it as a spike. I Updated the description to reflect that as we won't implement dual boot here.

I moved this to the new rails 4.1 upgrade epic :heart:
I think this can be it's first task.

Thank you!

This is a very generic rails task so I am marking it as a good first issue. For experienced rails devs only.

Github built their own dual_boot tooling and recommend something like that.
There are multiple approaches to dual booting: have multiple gemfiles or use if statements in gemfile.

Single Gemfile

Shopify built bootboot to support this second approach with a single Gemfile:
https://github.com/Shopify/bootboot

Here in "Parameterize your Gemfile" there are details about this same approach: one Gemfile with if statements and 2 Gemfile.lock files: http://recursion.org/incremental-rails-upgrade

and more details here on how to use bootboot:
https://blog.testdouble.com/posts/2019-09-03-3-keys-to-upgrading-rails/

including an example: https://github.com/testdouble/bootboot-example

There's an alternate way of keeping to one Gemfile without using bootboot described here, basically, a script on top of Gemfile:
https://dev.to/amplifr/how-to-migrate-monolith-to-the-scary-new-version-of-rails-3o52

Two Gemfiles

The approach with two Gemfiles and a command "next" is described here:
https://www.fastruby.io/blog/upgrade-rails/dual-boot/dual-boot-with-rails-6-0-beta.html

I will give bootboot a quick try now...

I gave bootboot a try, it's too simple. It just works.

You can run "bundle install" or "bundle exec rspec ..."
Or you can run "DEPENDENCIES_NEXT=1 bundle install" or "DEPENDENCIES_NEXT=1 bundle exec rspec ..."

In the Gemfile and code we can use "if ENV['DEPENDENCIES_NEXT']"
and there's an extra Gemfile_next.lock with the dependencies of "next".

That's it. Here's the PR: https://github.com/openfoodfoundation/openfoodnetwork/pull/6298
When the upgrade is finished and the "next" build is green and tested we would need a PR moving Gemfile_next.lock to Gemfile.lock and getting the next upgrade started :tada:

I only have one question now: we would need to switch the semaphore build jobs to run with this new flag to get build results... At github, on their blog about their upgrades, they said they had all builds run in both versions. But how can we afford that? Every build would take twice of the already long time it's taking now)... I am not sure how to make only some builds run with this flag in semaphore? I think we would need to setup a separate semaphore project for this?

bootboot

Best name ever! 馃槃

lol, yes, they have a logo :rofl: https://github.com/Shopify/bootboot#-bootboot-----

How many boots is too many? :trollface:
Screenshot from 2020-11-04 09-34-11

Moving this to rails 4-2 upgrade. It wont be used for 4.1 anyway.
@sauloperez @Matt-Yorkley @mkllnk how do you think we can get this bootboot thing working in semaphore?

Moving to rails 5 upgrade epic as rails 4.2 is mostly done :tada:

So, here's what I have found as part of this spike: I tried dualboot and thought it was great, it looks like it just works.

The setup is very simple, just a small change in the Gemfile and a new (next rails version) Gemfile_next.lock file. This is how the Gemfile would look like (this is the case where rails-4-1 is in master and rails-4-2 is the next version):
image

The rest of the Gemfile is the normal list of dependencies that dont change between versions.

Similar to the Gemfile we can also just use if ENV['DEPENDENCIES_NEXT'] in the code to fix problems in the "next" version.
By default we push changes to master and we use this if statement in cases where the "next version" code is not compatible with the previous version.

We can run bundle install and rpecs as usual, to run it in the "next" version we just need to add DEPENDENCIES_NEXT=1 before the commands: DEPENDENCIES_NEXT=1 bundle install will update Gemfile_next.lock.

Additionally, I wanted to verify how CI would look like. I reconfigured the spree project n semaphore and called it openfoodnetwork-next, this project also picked up all PRs in GH and builds them, but instead of building the normal setup it will build the "next" version. All I needed to do was to add DEPENDENCIES_NEXT=1 before some of the build commands.
With this, every PR in ofn was triggering 2 builds: one in the current rails version and one in the next rails version.

This makes it possible for all upgrade PRs to be submitted, reviewed, and tested against master. No long lived branch
And at the same time, when the upgrade is well advanced, like rails 4.1 right now, for all other normal PRs we can see if they break something on the next rails version build because every PR will trigger a build in the "next" CI setup.

So, lets imagine we set this up for rails 5 upgrade after rails 4.2 is done.
In semaphore we can have two builds per PR: one in rails 4.2 and one build in rails 5.
We can have PRs against master that fix rails 5 problems, these PRs should keep the rails 4.2 build green and improve the rails 5 build.
As the rails 5 build become greener and greener we can also start watching for the impact other PRs have on this build to avoid having normal non-upgrade PRs break the work done as part of the upgrade.

I'll do a quick pros and cons analysis for this approach:
Pros:

  • no more long lived branches
  • because we will commit all changes to master we will have a much better approach to process and tests. We will be able to follow the same process for these PRs as any other and all changes will be manually tested if appropriate. Something we dont do when we have long lived branches
  • easier process that enables volunteer devs to participate

Cons:

  • semaphore will have to handle more builds with the same 4 jobs. I tested and we can enable/disable the dual project setup in semaphore as we start or stop to work more intensely on the upgrade.
  • keeping up with if statements in the code and deleting them once the upgrade is done.

Personal opinion: I dont mind the current long lived branch approach but I think this is better and we should set it up immediately after rails 4.2 is merged (no rails-5 branch). For me the key point is the way it opens up the upgrades (I am thinking about all the future upgrades) to the community. I think we can really get these upgrades done by volunteers if we setup this dual boot properly.

Thoughts? @mkllnk @andrewpbrett @sauloperez @Matt-Yorkley

I am moving this (a spike) to Code Review.

I am moving this spike into the rails 4.2 epic: we can close this spike after a go/nogo decision is done and create a new issue (inside the rails 5 epic) to do the actual setup.

This would be really nice to have, and I agree it makes it more accessible for community contributions. I think it's preferable to long-lived branches.

Re: semaphore, I think the solution to only enable the "next" builds on demand when we want them is a good compromise. Have we looked at whether it's worth it to pay for more capacity on Semaphore? Their pricing page lists costs per second, but I'm not sure if it's different than our options since we're on Classic.

@luisramos0 the last point in your list of cons is that we'd have to keep up with if statements in the code - would they be anywhere outside the Gemfile and Gemfile.lock?

thanks for your feedback Andy.

yes, if statements would go into the code. But only for "next version" code that is not compatible with "previous version".
This rails 4.2 code is compatible with rails 4.0 so no if statement necessary, just the commit to master:
image

But this rails 4.2 code is not compatible with rails 4.0 so we would need an if statement:
image
Like this

if ENV['DEPENDENCIES_NEXT']
    self.responder = ApplicationResponder
end

Here we need if and else:
image

if ENV['DEPENDENCIES_NEXT']
    include ActionView::Layouts
else
    include AbstractController::Layouts
end

things would be a bit more complicated with class definitions and inheritance like here:
image
But we would find a way with an if statement.

These examples are all from https://github.com/openfoodfoundation/openfoodnetwork/pull/6301

IMO it's a big improvement as it mitigates the risks of Rails upgrades and reduces the costs of a long-lived branch. All that you mentioned looks good to me @luisramos0, including that if statement :+1:. As any kind of feature flag, we need to watch after the temporal extra complexity it brings, which we traded for flexibility.

Have we looked at whether it's worth it to pay for more capacity on Semaphore?

This also came to my mind. So you created a separate Semaphore project? If so, I guess it runs in parallel and we're not doubling the build time.

I'm not sure we agreed in the train delivery meeting but I'd go with this for Rails 4.2 already.

I'm not sure we agreed in the train delivery meeting but I'd go with this for Rails 4.2 already.

At first it was part of Rails 4.1 epic, so I don't think anyone would disagree to try it already now? Or are there disadvantages?

@RachL my tests where with rails 4.2 but rails 4.2 is already complete in a PR: https://github.com/openfoodfoundation/openfoodnetwork/pull/6354
We would have to redo everything with this approach.
This is now clearly something for the next upgrade only :+1:

@sauloperez I renamed the existing spree project, it's now semaphoreci.com/openfoodfoundation/openfoodnetwork-next and it does take from the same 4 jobs pool, these builds would have to wait for each other if we dont change the plan.

Lets think about how this would actually work, with 1000 code changes in 67 files:

Screenshot from 2020-11-10 09-41-48

We would need to add hundreds of conditionals in this case. Some would be single-liners, some would be the size of entire methods (with two different versions of the code in the if and else blocks), and some would potentially be the size of entire classes (with two versions of the class in the if and else blocks).

As soon as we wanted to merge we would have to go through and remove those hundreds of conditionals and branching code blocks again. I think this approach would significantly increase complexity and the total amount of work that needs doing, and I'm not sure how big the benefits would really be, in practice.

This approach makes a lot more sense in the context it actually comes from; huge tech companies with multiple dev teams delivering lots of different features at once whilst simultaneously upgrading a massive codebase, where the upgrade will take 4-5 months to complete.

I understand what you mean Matt.

About "hundreds of conditionals" I scanned rails 4.2 PR and this is what I think would be the number of ifs for the full rails 4.2 upgrade:

  • 5 ifs for perform_alter and equivalent 5 duplicate classes for ActiveJob::Base
  • 23 other ifs in the code plus maybe another 10/20 equivalent ifs to make specs work in both versions
    For rails 4.2 this would represent around 50 if statments.

These would be very easy to spot ENV['DEPENDENCIES_NEXT'] and easy to remove later on. I think this would be acceptable if we compare to the advantage: If we setup this right anyone can easily make these changes, these are changes anyone who has worked with rails upgrade will know how to do. I think the OFN rails upgrades should now start to take months BUT almost 100% done by volunteers. We can set this up now for rails 5 and target June 2021 to get have it done by volunteers. And rails 5.1 in 2020Q2. If we end 2021 on rails 5.1 I think that's acceptable.

My vote goes for dualboot here. I wonder what other devs think? I think Matt has a point!

I think the OFN rails upgrades should now start to take months

But in that case we would have to run two CI builds for every code change and potentially diagnose issues and change both versions in every PR. For months on end!

I think we would need to pay for double CI capacity as well, otherwise we would be cutting CI capacity in half for months. Build times per push would effectively go from ~25 minutes to ~50 minutes.

I'm still not seeing how this is a great idea :see_no_evil:

yes, I think we need to improve on the CI solution, maybe the "next" build should be on demand? We can probably find a way to trigger the "next" build only for some PRs.

I am not thinking rails 5. I am thinking all rails upgrades, currently 5.0, 5.1, 5.2, 6.0, soon 6.1 and then rails 7. All this will necessarily take several months.

If we decide to upgrade to Rails 5 (some time later next year?) I think the better strategy for us would be:

  1. wait until there aren't any big features half-way though the pipe
  2. have 2-3 devs intensively work on the upgrade, and get it done really quickly
  3. test and merge it, and return to normal delivery

I'm pretty confident in our collective ability to do part 2 now, and I think the overhead and costs would be much lower than doing it slowly over several months.

To put it another way @luisramos0: I'd be willing to bet 拢100 that you and me could knock out the 4.2 -> 5.0 upgrade in less than a week, if we were both on 30 hours.

:dollar: :smile:

ok, I agree: that is the alternative.

In terms of cost, that week of 2 devs is around 2k/eur if you multiply that by 5 to get to rails 6.1, that's 10k on our budget. If we add review and bug fixing that will be 15k minimum. And this is for a one week 2 devs estimate of each upgrade which I think is a very optimistic one. I'd double that estimate easily: bumping a rails version in OFN takes 2 weeks 2 devs full time (this is still quite optimistic!). In this perspective, I'd estimate the upgrade to rails 6.1 to be at least 30k eur out of our budget.

The alternative is to invest these 30k in other product initiatives and maybe an upgrade on the semaphore plan! Plus the extra engagement with community devs.

I think we have two strong cases here :-)

I'm still up for dual boot even at the cost of extra easy ifs. To me, it outweighs the benefits of running the build with every single commit and not having to maintain a branch in sync with the changes in master. That's lots of dev hours just redoing work. Even having the option to use the app in both versions just toggling an env var is huge. I think we tend to forget all the manual work previous upgrades required. If the problem is CI capacity, let's just pay for it. I'm pretty sure it's much cheaper than having to redo PRs.

If we go with the dual boot is not to slow down the upgrade but to make it more efficient, less error-prone and faster. What's totally unrealistic to me is to think we can afford to stop any other development, including bugfixing, for an uncertain period of time. IMO we are totally underestimating the time it'll take to upgrade to Rails 5.

Upgrades are a fact we cannot avoid and are happening even more often, so it makes sense to improve our practices and not treat them like exceptions so they have lower costs. It seems to me that dual boot goes in that direction.

Thanks @luisramos0 for the great summary. My thoughts about the downsides of dual-boot:

  • If-statements: There's a limited amount of breaking changes between Rails versions. For each breaking change, we can encapsulate the code and have the if-statement in one place (if needed). That kind of abstraction reduces code duplication and makes us less dependent on the Rails API. I don't think that this is a problem. It's also very easy to search for these statements to remove them all again. Many gems are compatible with multiple Rails versions and this is very common practice.
  • Additional CI runs: I actually think that this is an advantage. It means that every pull request against master is checked for compatibility with the next Rails version. That should save us a lot of work because we don't have to fix things after syncing a long-lived branch with master. It also creates awareness for the upgrade amongst developers who don't actively work on the upgrade.

I would introduce a new rails-next-specs-todo file which lists all specs that are still failing with the new Rails version. Or we have a skip statement in the spec files with a conditional. CI can skip those specs and we can have green builds while the upgrade is in progress. This way we detect regressions and can track the progress in the code.

Another advantage of dual boot is that it's much easier to test if a change is backwards compatible with the old Rails version. At the moment (upgrading Fair Food), I cherry-pick commits on master and then run the specs when I think that something can be merged to master already. Dual-boot makes the test quicker and avoids the syncing.

My vote is for dual boot.

I had one more idea that could help with dual boot: this is an alternative to having each PR to master trigger two builds in these two semaphore projects. I thought we could:

This way, each PR in master would only trigger the main build in the "current" rails version BUT, anyone at any point could see the "next" build of a given branch by simply pushing their branch to this new repo (no duplicate PRs, just the branch).
To do that, anyone would only need to setup the new remote (this only needs to be done once):
git remote add next [email protected]:openfoodfoundation/openfoodnetwork-next
and then, if I have a branch named rails5-whatever-fix, I just type:

git checkout rails5-whatever-fix
git push next rails5-whatever-fix

This would create a build with the "next" rails version here:
https://semaphoreci.com/openfoodfoundation/openfoodnetwork-next/branches/rails5-whatever-fix/builds/1

I think this would work really well.

It's a nice hack but it misses the point of continuous integration although we can surely give it a try.

So, @Matt-Yorkley created a branch with rails 5 working and I used the Gemfile and Gemfile.lock and made it dual bootable:
https://github.com/openfoodfoundation/openfoodnetwork/pull/6442

fyi, I have gone for the approach I describe above: setup parallel repo to connect to parallel semaphore project. I think it's working well!

New repo with main branch empty so that it doesnt confuse people and search engines:
github.com/openfoodfoundation/openfoodnetwork-next

Your local setup is as simple as adding a new remote called next:
git remote add next [email protected]:openfoodfoundation/openfoodnetwork-next

Then, for example for my PR #6681 with branch css in the main repo (what is called "upstream" on my computer).

git checkout css
git push -u next css
This will create branch https://github.com/openfoodfoundation/openfoodnetwork-next/tree/css
and trigger a rails 5 build here
https://semaphoreci.com/openfoodfoundation/openfoodnetwork-next-2/branches/css/builds/3

The only problem now is how to make the build run successfully in rails 5 in this new semaphore project.
Right now it's blowing up while installing mini_racer :man_shrugging:
https://semaphoreci.com/openfoodfoundation/openfoodnetwork-next-2/branches/css/builds/10

I suggest we give Github workflows a try which it'll make the extra repo unnecessary. I have started using it in a couple of projects last year and it works pretty well. Besides, we have it for free giving our GH account, if I'm not mistaken.

Was this page helpful?
0 / 5 - 0 ratings