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.

Sign up for GitHub

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

Jump to bottom

chore(docker): use node:18-slim base image #728

Merged
merged 6 commits into from
Jan 26, 2023
Merged

chore(docker): use node:18-slim base image #728

merged 6 commits into from
Jan 26, 2023

Conversation

wilsonianb
Copy link
Contributor

@wilsonianb wilsonianb commented Nov 4, 2022 •
edited
Loading

Changes proposed in this pull request

  • Update repo to Node 18
    This will require updating node locally and possibly reinstalling pnpm
  • Removes Docker build action timeouts. This may result in longer Docker builds (up to ~1 hour), but presumably only when dependencies change. It seems like we should be able to improve (caching) performance for:

    rafiki/packages/auth/Dockerfile

    Line 13 in 0259918

    RUN pnpm fetch
  • Limits building arm64 Docker images and pushing any Docker images to pushes to main & tags, which should help with:
    Docker workflow sporadic failures #985

Context

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass

@github-actions github-actions bot added pkg: auth Changes in the GNAP auth package. pkg: backend Changes in the backend package. pkg: map Changes in mock-account-provider package type: ci Changes to the CI labels Nov 4, 2022
@wilsonianb wilsonianb marked this pull request as ready for review November 4, 2022 19:35
@wilsonianb
Copy link
Contributor Author

wilsonianb commented Nov 4, 2022 •
edited
Loading

Converting to draft until I can test subsequent Docker build times (using cache from these sloooow builds)

@wilsonianb wilsonianb marked this pull request as draft November 4, 2022 20:22
@wilsonianb wilsonianb marked this pull request as ready for review January 24, 2023 19:32
@wilsonianb wilsonianb mentioned this pull request Jan 24, 2023
Closed
4 tasks
sabineschaller
sabineschaller reviewed Jan 25, 2023
View reviewed changes
Copy link
Member

@sabineschaller sabineschaller 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 happy to abandon my PR in favor for yours.

packages/auth/Dockerfile Outdated
@@ -1,12 +1,13 @@
FROM node:16.17.1-slim as builder
FROM node:18-slim as builder
Copy link
Member

Choose a reason for hiding this comment

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

Should we specify a specific version to make use of the cache? I think 18-slim resolves to the latest version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

latest is actually 19 now.
https://hub.docker.com/_/node
Do you mean node:18.13-slim or specifying the sha256?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I meant the latest version of 18. I was actually thinking of something like 18.13.0-slim to have a very specific version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm ok with that.
Can we get renovatebot to help us update it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://docs.renovatebot.com/modules/manager/dockerfile/#open-feature-requests

Copy link
Member

Choose a reason for hiding this comment

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

Looks like the guy who wanted to do the pinning feature hasn't gotten to it yet.

@wilsonianb wilsonianb merged commit 21d3fe4 into main Jan 26, 2023
@wilsonianb wilsonianb deleted the bw-node-18 branch January 26, 2023 13:55
@huijing huijing added the type: documentation (archived) Improvements or additions to documentation label Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sabineschaller sabineschaller sabineschaller approved these changes

@mkurapov mkurapov Awaiting requested review from mkurapov

Assignees
No one assigned
Labels
pkg: auth Changes in the GNAP auth package. pkg: backend Changes in the backend package. pkg: map Changes in mock-account-provider package type: ci Changes to the CI type: documentation (archived) Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docker build workflows timing out
3 participants
@wilsonianb @sabineschaller @huijing