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

devxp: Replace forem/ruby image with open-source Debian-based analogue. #19632

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

klardotsh
Copy link
Contributor

This builds a Debian-derived base image that targets Ruby 3.0.2 (our current version lock) to replace quay.io/forem/ruby, which is based on Fedora 35 (and as such, lacks ARM64 support).

The building of this image is not yet automated, I intend to add that as a follow-up some time vaguely soon-ish. This has been built and pushed to a temporary tag,
ghcr.io/forem/ruby:klardotsh-test. Once this merges, I'll build and push the result in main (pending any revisions that come up in PR review) with the following incantation, which will generate a multiarch manifest:

docker buildx build --platform linux/amd64,linux/arm64 -f Containerfile.base . -t ghcr.io/forem/ruby:latest -t ghcr.io/forem/ruby:$(git rev-parse --short HEAD) --push

This refs, but does not complete, #19626, and is one of several blockers on the path to getting #19603 merged.

This builds a Debian-derived base image that targets Ruby 3.0.2 (our
current version lock) to replace `quay.io/forem/ruby`, which is based on
Fedora 35 (and as such, lacks ARM64 support).

The building of this image is not yet automated, I intend to add that as
a follow-up some time vaguely soon-ish. This has been built and pushed
to a temporary tag,
[ghcr.io/forem/ruby:klardotsh-test](https://github.com/orgs/forem/packages/container/ruby/103882793?tag=klardotsh-test).
Once this merges, I'll build and push the result in main (pending any
revisions that come up in PR review) with the following incantation,
which will generate a multiarch manifest:

```sh
docker buildx build --platform linux/amd64,linux/arm64 -f Containerfile.base . -t ghcr.io/forem/ruby:latest -t ghcr.io/forem/ruby:$(git rev-parse --short HEAD) --push
```

This refs, but does not complete, #19626, and is one of several blockers
on the path to getting #19603 merged.
@klardotsh klardotsh requested a review from a team as a code owner June 23, 2023 22:39
@klardotsh klardotsh requested review from rt4914 and removed request for a team June 23, 2023 22:39
@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2023

Uffizzi Preview deployment-29339 was deleted.

echo "deb https://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list && \
apt update && \
apt install -y \
imagemagick \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IM's inclusion here is because the quay.io/forem/ruby this image is replacing, installed it as part of the base image. I'm open to the idea that this actually belongs in the app-specific containerfile - honestly I suspect I'll be moving various dependencies between the two as time goes on (as I prepare the app-specific containerfile's rebase onto this image, I'm discovering dependencies that probably need moved into the base, too)

klardotsh added a commit that referenced this pull request Jun 24, 2023
This "rebases" our application images off of the new Ruby base image
added in #19632, and fixes numerous problems and quirks with how the
images were built along the way. Notably:

- Issues where layers attempted to delete files in prior layers have
  been resolved (this caused build failures on some Docker filesystem
  drivers, notably overlay2).

- Bundler is no longer allowed to deviate from or modify the lockfile
  (`BUNDLE_FROZEN` is now `true`).

- `git(1)` is no longer required to live inside the container and `.git`
  is no longer required to be copied into the Docker build context, as
  these were only used to calculate `FOREM_BUILD_SHA`, which is now
  passed in as a Build Argument to the container build context.

- The entire source tree is no longer `chmod` in one giant swing, which
  ran so long on my system (as just one example) that I gave up after
  15-20 minutes and issued it a `SIGTERM`. Instead, `COPY --chown` is
  used more heavily and ensures the `APP_USER` will have access to the
  requisite files.

This new container image appears to build successfully for
`linux/arm64`, which refs (but does not complete) #19626. Currently,
such builds aren't automated , and must be built on a developer
workstation. For example:

```sh
docker buildx build --platform linux/amd64,linux/arm64 -f Containerfile . -t ghcr.io/forem/forem:klardotsh-test --push --build-arg VCS_REF=$(git rev-parse --short HEAD)
```

In the meantime, the existing `linux/amd64`-only BuildKite scripts have
been updated to allow this PR to merge as a separate unit, and CI
refactors to enable the multiarch builds of `linux/arm64,linux/amd64`
can come later when more time is available.

This is one of several blockers on the path to getting #19603 merged.
The next step in that chronology will be rebasing that work on top of
this work, which *should* be, on the containerization side, as
straightforward as bumping `Containerfile.base` to reference the new
upstream image, rebuilding the base container, and then bumping the
reference in `Containerfile`.
@rt4914 rt4914 removed their request for review June 24, 2023 06:04
@rt4914
Copy link
Contributor

rt4914 commented Jun 24, 2023

Un-assigning myself as I don't have enough context how to review this PR.

@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +5.66 🎉

Comparison is base (f0e99c6) 89.27% compared to head (e04081b) 94.94%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #19632      +/-   ##
==========================================
+ Coverage   89.27%   94.94%   +5.66%     
==========================================
  Files        1208      913     -295     
  Lines       27456    18409    -9047     
  Branches     2117        0    -2117     
==========================================
- Hits        24511    17478    -7033     
+ Misses       2792      931    -1861     
+ Partials      153        0     -153     
Flag Coverage Δ
cypress ∅ <ø> (∅)
javascript ∅ <ø> (∅)
jest ∅ <ø> (∅)
ruby 94.94% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
app/services/users/remove_role.rb 100.00% <ø> (ø)
app/models/role.rb 84.61% <100.00%> (+4.61%) ⬆️
app/policies/role_policy.rb 100.00% <100.00%> (ø)

... and 298 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@maestromac maestromac left a comment

Choose a reason for hiding this comment

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

🙌

klardotsh added a commit that referenced this pull request Jun 28, 2023
This "rebases" our application images off of the new Ruby base image
added in #19632, and fixes numerous problems and quirks with how the
images were built along the way. Notably:

- Issues where layers attempted to delete files in prior layers have
  been resolved (this caused build failures on some Docker filesystem
  drivers, notably overlay2).

- Bundler is no longer allowed to deviate from or modify the lockfile
  (`BUNDLE_FROZEN` is now `true`).

- `git(1)` is no longer required to live inside the container and `.git`
  is no longer required to be copied into the Docker build context, as
  these were only used to calculate `FOREM_BUILD_SHA`, which is now
  passed in as a Build Argument to the container build context.

- The entire source tree is no longer `chmod` in one giant swing, which
  ran so long on my system (as just one example) that I gave up after
  15-20 minutes and issued it a `SIGTERM`. Instead, `COPY --chown` is
  used more heavily and ensures the `APP_USER` will have access to the
  requisite files.

This new container image appears to build successfully for
`linux/arm64`, which refs (but does not complete) #19626. Currently,
such builds aren't automated , and must be built on a developer
workstation. For example:

```sh
docker buildx build --platform linux/amd64,linux/arm64 -f Containerfile . -t ghcr.io/forem/forem:klardotsh-test --push --build-arg VCS_REF=$(git rev-parse --short HEAD)
```

In the meantime, the existing `linux/amd64`-only BuildKite scripts have
been updated to allow this PR to merge as a separate unit, and CI
refactors to enable the multiarch builds of `linux/arm64,linux/amd64`
can come later when more time is available.

This is one of several blockers on the path to getting #19603 merged.
The next step in that chronology will be rebasing that work on top of
this work, which *should* be, on the containerization side, as
straightforward as bumping `Containerfile.base` to reference the new
upstream image, rebuilding the base container, and then bumping the
reference in `Containerfile`.
klardotsh added a commit that referenced this pull request Jun 28, 2023
This "rebases" our application images off of the new Ruby base image
added in #19632, and fixes numerous problems and quirks with how the
images were built along the way. Notably:

- Issues where layers attempted to delete files in prior layers have
  been resolved (this caused build failures on some Docker filesystem
  drivers, notably overlay2).

- Bundler is no longer allowed to deviate from or modify the lockfile
  (`BUNDLE_FROZEN` is now `true`).

- `git(1)` is no longer required to live inside the container and `.git`
  is no longer required to be copied into the Docker build context, as
  these were only used to calculate `FOREM_BUILD_SHA`, which is now
  passed in as a Build Argument to the container build context.

- The entire source tree is no longer `chmod` in one giant swing, which
  ran so long on my system (as just one example) that I gave up after
  15-20 minutes and issued it a `SIGTERM`. Instead, `COPY --chown` is
  used more heavily and ensures the `APP_USER` will have access to the
  requisite files.

This new container image appears to build successfully for
`linux/arm64`, which refs (but does not complete) #19626. Currently,
such builds aren't automated , and must be built on a developer
workstation. For example:

```sh
docker buildx build --platform linux/amd64,linux/arm64 -f Containerfile . -t ghcr.io/forem/forem:klardotsh-test --push --build-arg VCS_REF=$(git rev-parse --short HEAD)
```

In the meantime, the existing `linux/amd64`-only BuildKite scripts have
been updated to allow this PR to merge as a separate unit, and CI
refactors to enable the multiarch builds of `linux/arm64,linux/amd64`
can come later when more time is available.

This is one of several blockers on the path to getting #19603 merged.
The next step in that chronology will be rebasing that work on top of
this work, which *should* be, on the containerization side, as
straightforward as bumping `Containerfile.base` to reference the new
upstream image, rebuilding the base container, and then bumping the
reference in `Containerfile`.
@andygeorge
Copy link
Contributor

LGTM!

@klardotsh klardotsh merged commit b8a33e0 into main Jun 28, 2023
36 checks passed
@klardotsh klardotsh deleted the klardotsh/oss-ruby-image-with-arm64 branch June 28, 2023 17:44
klardotsh added a commit that referenced this pull request Jun 28, 2023
This "rebases" our application images off of the new Ruby base image
added in #19632, and fixes numerous problems and quirks with how the
images were built along the way. Notably:

- Issues where layers attempted to delete files in prior layers have
  been resolved (this caused build failures on some Docker filesystem
  drivers, notably overlay2).

- Bundler is no longer allowed to deviate from or modify the lockfile
  (`BUNDLE_FROZEN` is now `true`).

- `git(1)` is no longer required to live inside the container and `.git`
  is no longer required to be copied into the Docker build context, as
  these were only used to calculate `FOREM_BUILD_SHA`, which is now
  passed in as a Build Argument to the container build context.

- The entire source tree is no longer `chmod` in one giant swing, which
  ran so long on my system (as just one example) that I gave up after
  15-20 minutes and issued it a `SIGTERM`. Instead, `COPY --chown` is
  used more heavily and ensures the `APP_USER` will have access to the
  requisite files.

This new container image appears to build successfully for
`linux/arm64`, which refs (but does not complete) #19626. Currently,
such builds aren't automated , and must be built on a developer
workstation. For example:

```sh
docker buildx build --platform linux/amd64,linux/arm64 -f Containerfile . -t ghcr.io/forem/forem:klardotsh-test --push --build-arg VCS_REF=$(git rev-parse --short HEAD)
```

In the meantime, the existing `linux/amd64`-only BuildKite scripts have
been updated to allow this PR to merge as a separate unit, and CI
refactors to enable the multiarch builds of `linux/arm64,linux/amd64`
can come later when more time is available.

This is one of several blockers on the path to getting #19603 merged.
The next step in that chronology will be rebasing that work on top of
this work, which *should* be, on the containerization side, as
straightforward as bumping `Containerfile.base` to reference the new
upstream image, rebuilding the base container, and then bumping the
reference in `Containerfile`.
dukegreene pushed a commit that referenced this pull request Jun 28, 2023
…e. (#19632)

This builds a Debian-derived base image that targets Ruby 3.0.2 (our
current version lock) to replace `quay.io/forem/ruby`, which is based on
Fedora 35 (and as such, lacks ARM64 support).

The building of this image is not yet automated, I intend to add that as
a follow-up some time vaguely soon-ish. This has been built and pushed
to a temporary tag,
[ghcr.io/forem/ruby:klardotsh-test](https://github.com/orgs/forem/packages/container/ruby/103882793?tag=klardotsh-test).
Once this merges, I'll build and push the result in main (pending any
revisions that come up in PR review) with the following incantation,
which will generate a multiarch manifest:

```sh
docker buildx build --platform linux/amd64,linux/arm64 -f Containerfile.base . -t ghcr.io/forem/ruby:latest -t ghcr.io/forem/ruby:$(git rev-parse --short HEAD) --push
```

This refs, but does not complete, #19626, and is one of several blockers
on the path to getting #19603 merged.
klardotsh added a commit that referenced this pull request Jun 29, 2023
…#19633)

This "rebases" our application images off of the new Ruby base image
added in #19632, and fixes numerous problems and quirks with how the
images were built along the way. Notably:

- Issues where layers attempted to delete files in prior layers have
  been resolved (this caused build failures on some Docker filesystem
  drivers, notably overlay2).

- Bundler is no longer allowed to deviate from or modify the lockfile
  (`BUNDLE_FROZEN` is now `true`).

- `git(1)` is no longer required to live inside the container and `.git`
  is no longer required to be copied into the Docker build context, as
  these were only used to calculate `FOREM_BUILD_SHA`, which is now
  passed in as a Build Argument to the container build context.

- The entire source tree is no longer `chmod` in one giant swing, which
  ran so long on my system (as just one example) that I gave up after
  15-20 minutes and issued it a `SIGTERM`. Instead, `COPY --chown` is
  used more heavily and ensures the `APP_USER` will have access to the
  requisite files.

This new container image appears to build successfully for
`linux/arm64`, which refs (but does not complete) #19626. Currently,
such builds aren't automated , and must be built on a developer
workstation. For example:

```sh
docker buildx build --platform linux/amd64,linux/arm64 -f Containerfile . -t ghcr.io/forem/forem:klardotsh-test --push --build-arg VCS_REF=$(git rev-parse --short HEAD)
```

In the meantime, the existing `linux/amd64`-only BuildKite scripts have
been updated to allow this PR to merge as a separate unit, and CI
refactors to enable the multiarch builds of `linux/arm64,linux/amd64`
can come later when more time is available.

This is one of several blockers on the path to getting #19603 merged.
The next step in that chronology will be rebasing that work on top of
this work, which *should* be, on the containerization side, as
straightforward as bumping `Containerfile.base` to reference the new
upstream image, rebuilding the base container, and then bumping the
reference in `Containerfile`.
rt4914 pushed a commit that referenced this pull request Jun 29, 2023
…#19633)

This "rebases" our application images off of the new Ruby base image
added in #19632, and fixes numerous problems and quirks with how the
images were built along the way. Notably:

- Issues where layers attempted to delete files in prior layers have
  been resolved (this caused build failures on some Docker filesystem
  drivers, notably overlay2).

- Bundler is no longer allowed to deviate from or modify the lockfile
  (`BUNDLE_FROZEN` is now `true`).

- `git(1)` is no longer required to live inside the container and `.git`
  is no longer required to be copied into the Docker build context, as
  these were only used to calculate `FOREM_BUILD_SHA`, which is now
  passed in as a Build Argument to the container build context.

- The entire source tree is no longer `chmod` in one giant swing, which
  ran so long on my system (as just one example) that I gave up after
  15-20 minutes and issued it a `SIGTERM`. Instead, `COPY --chown` is
  used more heavily and ensures the `APP_USER` will have access to the
  requisite files.

This new container image appears to build successfully for
`linux/arm64`, which refs (but does not complete) #19626. Currently,
such builds aren't automated , and must be built on a developer
workstation. For example:

```sh
docker buildx build --platform linux/amd64,linux/arm64 -f Containerfile . -t ghcr.io/forem/forem:klardotsh-test --push --build-arg VCS_REF=$(git rev-parse --short HEAD)
```

In the meantime, the existing `linux/amd64`-only BuildKite scripts have
been updated to allow this PR to merge as a separate unit, and CI
refactors to enable the multiarch builds of `linux/arm64,linux/amd64`
can come later when more time is available.

This is one of several blockers on the path to getting #19603 merged.
The next step in that chronology will be rebasing that work on top of
this work, which *should* be, on the containerization side, as
straightforward as bumping `Containerfile.base` to reference the new
upstream image, rebuilding the base container, and then bumping the
reference in `Containerfile`.
jaw6 added a commit that referenced this pull request Jun 30, 2023
…o-comment-notifications

* origin/main:
  Standalone "admin" page for each comment (#19644)
  devxp: Base the application container images off new Ruby base image. (#19633)
  devxp: Base the application container images off new Ruby base image. (#19633)
  Add index to billboard placement area (#19648)
  Follow-ups for suggested organizations follows during onboarding (#19619)
  Fix bug: Unable to unfollow topics (#19637)
  Remove unnecessary fragment cache (#19653)
  [ruby] Update carrierwave 2.2.3 → 2.2.4 (patch) (#19614)
  devxp: Replace forem/ruby image with open-source Debian-based analogue. (#19632)
  Remove API dependency on fetching tweets and shift to frontend (#19641)
@mirie mirie mentioned this pull request Jul 3, 2023
2 tasks
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.

None yet

5 participants