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

Make gitrest docker build install with a frozen lockfile #23535

Merged
merged 14 commits into from
Jan 14, 2025

Conversation

Abe27342
Copy link
Contributor

@Abe27342 Abe27342 commented Jan 13, 2025

Description

Main desirable change: updates the gitrest docker build to copy .npmrc within the docker container before installing, which ensures the subsequent install includes frozen-lockfile=true.

Fallout from this desire: the previous CI setup for docker builds involved using flub to set the package version before the docker build. This makes pnpm install with frozen lockfile fail, since the dependency versions of packages within the workspace will be whatever that package version was set to, rather than the version within source control. To fix this, I've updated the docker build to support setting the package version from within the docker container using flub, and plumbed through necessary variables in CI to make that possible.

Also includes some minor quality improvements to the docker build, such as fixing up deprecated syntax warnings, using a cache mount for the pnpm dependencies, and reducing the footprint of the runner build slightly (there's still much more that could be done in that area if we care to invest: we keep around dev deps, for example).

AB#26779

@Copilot Copilot bot review requested due to automatic review settings January 13, 2025 22:21
@github-actions github-actions bot added area: build Build related issues area: server Server related issues (routerlicious) base: main PRs targeted against main branch labels Jan 13, 2025

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 7 changed files in this pull request and generated no comments.

Files not reviewed (2)
  • server/gitrest/Dockerfile: Language not supported
  • server/gitrest/package.json: Language not supported
@Abe27342
Copy link
Contributor Author

Sample build here:

https://dev.azure.com/fluidframework/internal/_build/results?buildId=316564&view=results

Plan is to transition other server pipelines to a similar structure, but I want a bit more e2e verification first.

@Abe27342 Abe27342 requested review from alexvy86 and znewton January 13, 2025 23:05
tools/pipelines/templates/build-docker-service.yml Outdated Show resolved Hide resolved
tools/pipelines/server-gitrest.yml Show resolved Hide resolved
server/gitrest/Dockerfile Outdated Show resolved Hide resolved
server/gitrest/Dockerfile Show resolved Hide resolved
server/gitrest/Dockerfile Outdated Show resolved Hide resolved
server/gitrest/Dockerfile Outdated Show resolved Hide resolved
Abe27342 and others added 3 commits January 14, 2025 09:19
Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com>
Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com>
Copy link
Contributor

@alexvy86 alexvy86 left a comment

Choose a reason for hiding this comment

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

LGTM

@Abe27342 Abe27342 force-pushed the test/absander/bump-server-versions-within-docker-build branch from f87a4bf to c5ed19f Compare January 14, 2025 18:39
@Abe27342
Copy link
Contributor Author

ok, finally got e2e tests working. looks healthy--https://dev.azure.com/fluidframework/internal/_build/results?buildId=316917&view=results (note the LTS build is from a previous attempt that didn't get requeued as I didn't bother, therefore pass rate much lower than usual)

@Abe27342 Abe27342 merged commit 40d5824 into main Jan 14, 2025
64 checks passed
@Abe27342 Abe27342 deleted the test/absander/bump-server-versions-within-docker-build branch January 14, 2025 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues area: server Server related issues (routerlicious) base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants