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

[ci] Fix 2.1, 2.2 and 2.3 Dockerfiles #3534

Merged
merged 2 commits into from
Mar 19, 2024
Merged

Conversation

AlexJF
Copy link
Contributor

@AlexJF AlexJF commented Mar 18, 2024

What does this PR do?
Make Dockerfiles for 2.1, 2.2 and 2.3 work with debian bookworm (2.2 with bullseye due to ruby/setup-ruby#496) rather than stretch, whose repos have been archived.

@ivoanjo pointed out we could have just rerouted the repos to debian archive but this way we stop relying on such old setups which makes things like docker setup and any eventual changes much easier since we're less likely to run into more unsupported/broken things.

Motivation:
Make github CI jobs work for 2.1, 2.2 and 2.3 again.

Additional Notes:

How to test the change?

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@github-actions github-actions bot added the dev/ci Involves CircleCI, GitHub Actions, or GitLab label Mar 18, 2024
@AlexJF AlexJF force-pushed the alexjf/fix-old-ruby-workflows branch from 7d0bb4f to b1ddeb4 Compare March 18, 2024 18:59
@AlexJF AlexJF force-pushed the alexjf/fix-old-ruby-workflows branch from 2068909 to 4edb9d8 Compare March 19, 2024 11:41
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.24%. Comparing base (0ca28c1) to head (4edb9d8).
Report is 7 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3534   +/-   ##
=======================================
  Coverage   98.24%   98.24%           
=======================================
  Files        1275     1275           
  Lines       75329    75329           
  Branches     3559     3559           
=======================================
  Hits        74005    74005           
  Misses       1324     1324           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AlexJF AlexJF marked this pull request as ready for review March 19, 2024 12:29
@AlexJF AlexJF requested a review from a team as a code owner March 19, 2024 12:29
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thanks for picking this up!

Note that although images are built on every change, the new images are not yet published/used for CI.

TL;DR: Remember to go to https://github.com/DataDog/dd-trace-rb/actions/workflows/build-ruby.yml after merging this (or before?) and triggering a manual run with "Push images" selected, so the new images start getting used in CI.

FROM buildpack-deps:stretch AS ruby-2.1.10-stretch
FROM buildpack-deps:bookworm AS ruby-2.1.10-bookworm
Copy link
Member

Choose a reason for hiding this comment

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

If anyone else is wondering stretch is debian 9, and bookworm is debian 12 so this buys us a lot of time! Nice :D

Comment on lines +7 to +11
# taken from https://hub.docker.com/layers/library/ruby/2.2.10/images/sha256-6c8e6f9667b21565284ef1eef81086adda867e936b74cc74a119ac380d9f00f9?context=explore
# NOTE: Why bullseye and not bookworm like 2.1 and 2.3? Something fails with Ruby 2.2 compilation with newer images leading to segfaults and
# errors like `Marshal.load reentered at marshal_load`: https://github.com/ruby/setup-ruby/issues/496#issuecomment-1520667671
# These do not show up with bullseye and ubuntu-20.
FROM buildpack-deps:bullseye AS ruby-2.2.10-bullseye
Copy link
Member

Choose a reason for hiding this comment

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

...and bullseye is debian 11 ;)

Comment on lines -63 to -72
# Install RubyGems and Bundler
# NOTE: Rubygems 3.0.6 is the last version that seems to work fine in this image AND not drag bundler 2 along.
# Later versions are either broken or they force the use of bundler 2, which we can't use because some of our
# dependencies (e.g. rails 3.0.20) don't like it.
RUN gem update --system '3.0.6'
# Ruby 2.3 can support Bundler 2+
# But hold back to < 2 for now, because some dependencies require it.
RUN gem install bundler -v '1.17.3'
ENV BUNDLE_SILENCE_ROOT_WARNING 1

Copy link
Member

Choose a reason for hiding this comment

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

I noticed that, unlike the other images, we're not installing a specific bundler version. This should probably be fine, but let's keep an eye out for it :)

Copy link
Contributor Author

@AlexJF AlexJF Mar 19, 2024

Choose a reason for hiding this comment

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

Yeah I only noticed this specific part after the green ✅ but the build tests do show that the used bundler version should match the described requirements and I didn't want to wait another 40 mins:

Run docker run --platform linux/aarch64 --rm ghcr.io/datadog/dd-trace-rb/ruby:2.3.8-dd ruby -e 'puts RUBY_DESCRIPTION'
ruby 2.3.[8](https://github.com/DataDog/dd-trace-rb/actions/runs/8342360895/job/22830332417#step:10:9)p45[9](https://github.com/DataDog/dd-trace-rb/actions/runs/8342360895/job/22830332417#step:10:10) (2018-10-18 revision 65136) [aarch64-linux]
2.7.11
Bundler version 1.16.6

1.17.3 vs 1.16.6 but as you say probably fine and if it turns out something breaks should be an easy fix afterwards anyway.

@AlexJF
Copy link
Contributor Author

AlexJF commented Mar 19, 2024

TL;DR: Remember to go to DataDog/dd-trace-rb/actions/workflows/build-ruby.yml after merging this (or before?) and triggering a manual run with "Push images" selected, so the new images start getting used in CI.

Yeah I'll follow the steps in https://github.com/DataDog/dd-trace-rb/blob/ab0601e51402a2db910f473bc43f780e751c4667/.circleci/images/primary/README.md now

@AlexJF AlexJF merged commit 20ea0f3 into master Mar 19, 2024
219 checks passed
@AlexJF AlexJF deleted the alexjf/fix-old-ruby-workflows branch March 19, 2024 14:56
@github-actions github-actions bot added this to the 1.21.1 milestone Mar 19, 2024
ivoanjo added a commit that referenced this pull request Mar 20, 2024
…a base

**What does this PR do?**

This PR changes our Ruby 2.3 docker image to use Debian 11 "bullseye"
as a base instead of Debian 12 "buster".

In #3534 we updated this
image but this broke system-tests
(https://github.com/DataDog/dd-trace-rb/actions/runs/8346827939/job/22845592152)
because they use an old nokogiri version that fails to build on
modern systems (sparklemotion/nokogiri#2105):

```
$ gem install nokogiri -v '= 1.10.10'

xml_document.c:495:14: error: conflicting types for 'canonicalize'; have 'VALUE(int,  VALUE *, VALUE)' {aka 'long unsigned int(int,  long unsigned int *, long unsigned int)'}
  495 | static VALUE canonicalize(int argc, VALUE* argv, VALUE self)
      |              ^~~~~~~~~~~~
In file included from /usr/include/features.h:489,
                 from /usr/include/x86_64-linux-gnu/bits/libc-header-start.h:33,
                 from /usr/include/stdlib.h:26,
                 from ./nokogiri.h:4,
                 from ./xml_document.h:4,
                 from xml_document.c:1:
/usr/include/x86_64-linux-gnu/bits/mathcalls.h:370:1: note: previous declaration of 'canonicalize' with type 'int(double *, const double *)'
  370 | __MATHDECL_1 (int, canonicalize,, (_Mdouble_ *__cx, const _Mdouble_ *__x));
      | ^~~~~~~~~~~~
make: *** [Makefile:240: xml_document.o] Error 1

make failed, exit code 2

Gem files will remain installed in /usr/local/bundle/gems/nokogiri-1.10.10 for inspection.
Results logged to /usr/local/bundle/extensions/x86_64-linux/2.3.0/nokogiri-1.10.10/gem_make.out
```

Rather than fighting nokogiri or the system-tests app, let's instead
gently step back from Debian 12 to 11, which should still buy us plenty
of time until we deprecate Ruby 2.3.

**Motivation:**

Fix CI/system-tests for Ruby 2.3.

**Additional Notes:**

N/A

**How to test the change?**

I've locally built the image with

```bash
$ docker build . -f Dockerfile-2.3.8 -t some-testing-name
```

and confirmed that nokogiri sucessfully installs.
ivoanjo added a commit that referenced this pull request Mar 20, 2024
…ort mysql gem

**What does this PR do?**

This PR is a slightly tweaked revert of
#3534 .

TL;DR after updating our Ruby 2.1/2.2/2.3 images to a newer debian,
we discovered that some of our CI testing requires the `mysql` gem,
which breaks on newer Debian versions (due to a newer
libmysqlclient/libmariadb).

With these reverts, I was able to locally rebuild the images and install
both mysql and nokogiri:

```
$ docker run -ti -v `pwd`:/working ivoanjo/docker-library:ddtrace_rb_2_3_8_testing_t6  /bin/bash
root@255fe2dc2374:/app# ruby -v
ruby 2.3.8p459 (2018-10-18 revision 65136) [x86_64-linux]
root@255fe2dc2374:/app# docker-compose -v
docker-compose version 1.8.0, build unknown
root@255fe2dc2374:/app# bundle -v
Bundler version 1.17.3
root@255fe2dc2374:/app# gem install mysql
Fetching mysql-2.9.1.gem
Building native extensions. This could take a while...
Successfully installed mysql-2.9.1
1 gem installed
root@255fe2dc2374:/app# gem install nokogiri -v '= 1.10.10'
Fetching nokogiri-1.10.10.gem
Fetching mini_portile2-2.4.0.gem
Successfully installed mini_portile2-2.4.0
Building native extensions. This could take a while...
Successfully installed nokogiri-1.10.10
2 gems installed
```

I was able to reproduce similar successful output on 2.1 and 2.2
as well (using nokogiri 1.9.1; 1.10.10 requires 2.3).

**Motivation:**

**Additional Notes:**

In the `Dockerfile` that #3534 initially changed there was a comment
stating that we were installing `docker-compose 1.21` from
stretch-backports, which confused me initially, since I couldn't
install that version from backports.

I checked an image build log from 2 months ago
(https://github.com/DataDog/dd-trace-rb/actions/runs/7386478002/job/20093153619)
and copied its image sha
`ghcr.io/datadog/dd-trace-rb/ruby:2.3.8-dd@sha256:b940776f41a5993e728f813c92734eaa82070ba74626ad8d2768a0242de269b3`
and got the same `docker-compose 1.8.0` I'm getting now:

```
$ docker run -ti -v `pwd`:/working ghcr.io/datadog/dd-trace-rb/ruby:2.3.8-dd@sha256:b940776f41a5993e728f813c92734eaa82070ba74626ad8d2768a0242de269b3 /bin/bash
root@23481c358c5b:/app# docker-compose -v
docker-compose version 1.8.0, build unknown
```

Unless I'm doing something wrong, either this comment was
never correct OR somehow the package we were using got purged.

Either way, it seems to me that this indicates we can still
use this version, so let's move forward (and hope to really
stop supporting these Rubies soon).

**How to test the change?**

Check CI builds for the images and/or build locally. Note that
our overall CI is a bit undisposed with incorrect images, so I'm
expecting red unrelated CI jobs that will only be fixed after we
publish these new images.
ivoanjo added a commit that referenced this pull request Mar 20, 2024
…ort mysql gem

**What does this PR do?**

This PR is a slightly tweaked revert of
#3534 .

TL;DR after updating our Ruby 2.1/2.2/2.3 images to a newer debian,
we discovered that some of our CI testing requires the `mysql` gem,
which breaks on newer Debian versions (due to a newer
libmysqlclient/libmariadb).

With these reverts, I was able to locally rebuild the images and install
both mysql and nokogiri:

```
$ docker run -ti -v `pwd`:/working ivoanjo/docker-library:ddtrace_rb_2_3_8_testing_t6  /bin/bash
root@255fe2dc2374:/app# ruby -v
ruby 2.3.8p459 (2018-10-18 revision 65136) [x86_64-linux]
root@255fe2dc2374:/app# docker-compose -v
docker-compose version 1.8.0, build unknown
root@255fe2dc2374:/app# bundle -v
Bundler version 1.17.3
root@255fe2dc2374:/app# gem install mysql
Fetching mysql-2.9.1.gem
Building native extensions. This could take a while...
Successfully installed mysql-2.9.1
1 gem installed
root@255fe2dc2374:/app# gem install nokogiri -v '= 1.10.10'
Fetching nokogiri-1.10.10.gem
Fetching mini_portile2-2.4.0.gem
Successfully installed mini_portile2-2.4.0
Building native extensions. This could take a while...
Successfully installed nokogiri-1.10.10
2 gems installed
```

I was able to reproduce similar successful output on 2.1 and 2.2
as well (using nokogiri 1.9.1; 1.10.10 requires 2.3).

**Motivation:**

**Additional Notes:**

In the `Dockerfile` that #3534 initially changed there was a comment
stating that we were installing `docker-compose 1.21` from
stretch-backports, which confused me initially, since I couldn't
install that version from backports.

I checked an image build log from 2 months ago
(https://github.com/DataDog/dd-trace-rb/actions/runs/7386478002/job/20093153619)
and copied its image sha
`ghcr.io/datadog/dd-trace-rb/ruby:2.3.8-dd@sha256:b940776f41a5993e728f813c92734eaa82070ba74626ad8d2768a0242de269b3`
and got the same `docker-compose 1.8.0` I'm getting now:

```
$ docker run -ti -v `pwd`:/working ghcr.io/datadog/dd-trace-rb/ruby:2.3.8-dd@sha256:b940776f41a5993e728f813c92734eaa82070ba74626ad8d2768a0242de269b3 /bin/bash
root@23481c358c5b:/app# docker-compose -v
docker-compose version 1.8.0, build unknown
```

Unless I'm doing something wrong, either this comment was
never correct OR somehow the package we were using got purged.

Either way, it seems to me that this indicates we can still
use this version, so let's move forward (and hope to really
stop supporting these Rubies soon).

**How to test the change?**

Check CI builds for the images and/or build locally. Note that
our overall CI is a bit undisposed with incorrect images, so I'm
expecting red unrelated CI jobs that will only be fixed after we
publish these new images.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/ci Involves CircleCI, GitHub Actions, or GitLab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants