feat(cli): enable zstd compression for deployment images #2773
Conversation
🦋 Changeset detectedLatest commit: 09ce3b3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
nicktrn
changed the title
WalkthroughThis change introduces configurable zstd and gzip compression support for Docker image builds during deployment. The changeset modifies the CLI deploy command to accept compression-related options (compression algorithm, compression level for cache layers, and force-compression behavior) and propagates these through the build pipeline. A Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/cli-v3/src/deploy/buildImage.ts (1)
1128-1167: Minor behavioral nuance: forceCompression applies even when compression is unset or gzip
getOutputOptionswill addforce-compression=truewheneverforceCompressionis truthy, regardless of whethercompressionis"zstd","gzip", orundefined. Given the CLI help text (“Enabled by default when using zstd”), that’s slightly broader than advertised if someone explicitly switches to gzip but doesn’t override the flag.Functionally this is safe (it just forces recompression with the default algorithm) but may have a small performance cost on gzip builds.
If you want behavior to strictly match the help text, you could gate this on
compression === "zstd"or clarify the description. Example tweak:- if (forceCompression) { - outputOptions.push("force-compression=true"); - } + if (forceCompression && compression === "zstd") { + outputOptions.push("force-compression=true"); + }packages/cli-v3/src/commands/deploy.ts (1)
181-197: Force-compression flags: confirm Commander semantics or simplifyYou define both
--force-compressionand--no-force-compressionas separateCommandOptions for the same underlying property:new CommandOption("--force-compression", ...) new CommandOption("--no-force-compression", ...)Commander typically handles “no-” negation automatically when only the negative form is declared, and having both positive and negative options for the same name can be a little opaque.
Behavior here is likely fine (last flag on the command line wins), but you might simplify by:
- Keeping only
--no-force-compression(with docs stating “enabled by default”), or- Using a single
.option("--force-compression", "...")with.default(true)and letting Commander’s negation--no-force-compressionbe inferred if that works in your current version.Not urgent, but worth double-checking for clarity and to avoid any future surprises if Commander changes how duplicate option names are handled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/great-pillows-look.md(1 hunks)packages/cli-v3/src/commands/deploy.ts(5 hunks)packages/cli-v3/src/deploy/buildImage.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/cli-v3/src/deploy/buildImage.tspackages/cli-v3/src/commands/deploy.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
packages/cli-v3/src/deploy/buildImage.tspackages/cli-v3/src/commands/deploy.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
packages/cli-v3/src/deploy/buildImage.tspackages/cli-v3/src/commands/deploy.ts
🧬 Code graph analysis (1)
packages/cli-v3/src/deploy/buildImage.ts (1)
apps/coordinator/src/exec.ts (1)
push(167-173)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
.changeset/great-pillows-look.md (1)
1-5: Changelog entry is well-formatted and accurately describes the change.The Changesets file correctly specifies a minor version bump for the new zstd compression feature and uses a clear, descriptive commit-style message that aligns with the PR objectives.
packages/cli-v3/src/deploy/buildImage.ts (3)
16-27: Compression options plumbed correctly through buildImage and into local/remote buildsThe new
compression,cacheCompression,compressionLevel, andforceCompressionfields are consistently typed acrossBuildImageOptionsand forwarded correctly into both the local and remote build paths. The destructuring and call sites forlocalBuildImageandremoteBuildImageline up with the option names, so there’s no obvious wiring bug here.Also applies to: 58-91, 93-121
156-179: Remote build output configuration: semantics look sound, but worth validating against Depot/BuildKit behaviorUsing
getOutputOptionsto construct--output type=image,...with optionalcompression=zstd,compression-level, andforce-compressionkeeps the logic centralized and matches BuildKit’s expected keys. Passingpush=truehere and relying on--saveplus Depot’s handling of the image tag looks reasonable, but it does couple behavior to Depot’s support for the same output options and their interaction with--save.I’d keep this as-is but explicitly validate against your target Depot/BuildKit versions that:
--output type=image,...,push=trueworks as expected with Depot’sbuildsubcommand.- Combining
--savewith--output type=image,...does not change semantics or cause warnings/errors.Also applies to: 196-208, 209-245
325-352: Local build: cache compression and output options integration looks correctFor self‑hosted builds:
compressionandforceCompressionare passed intogetOutputOptions, so image-layer compression is handled uniformly with the remote path.cacheCompressionis only applied to the registry cache via--cache-to ...${cacheCompression === "zstd" ? ",compression=zstd" : ""}, which is a safe opt-in for zstd while preserving gzip as the implicit default.--outputand the constructedoutputOptionsare appended once, and the-t imageTagflag is still present, so tagging and compression configuration remain explicit.This all aligns with the stated goal of defaulting to zstd while keeping gzip behavior unchanged when requested.
Also applies to: 354-365, 532-555, 560-562
packages/cli-v3/src/commands/deploy.ts (3)
63-90: Cache and compression defaults align with PR intentDefining:
cache: z.boolean().default(true)and later usingnoCache: !options.cachepreserves the previous “cached by default, opt-out with--no-cache” behavior while giving you a positively named flag in the typed options.compressionandcacheCompressiondefaulting to"zstd"matches the stated goal of enabling zstd by default, and leaves room for callers to opt back to"gzip".Given Commander’s semantics for
--no-cache/--cacheand--compression/--cache-compressionchoices, this schema is consistent with the CLI surface.
518-563: buildImage invocation: cache and compression options are wired through correctlyThe call to
buildImagenow:
- Derives
noCacheas!options.cache, matching the new positive cache flag.- Forwards
compression,cacheCompression,compressionLevel, andforceCompressiondirectly from the parsed CLI options.This matches the shape of
BuildImageOptionsand the wiring inbuildImage.ts, so the deploy command is correctly propagating the new compression controls into the build pipeline.
164-197: Type mismatch risk forcompressionLevel: defined but unusedThe
--compression-leveloption is defined with no.argParser()for numeric coercion, which means Commander.js will pass a string value. WhilecompressionLevelis not currently used in the deploy handler, if it were to be used as a number (e.g., in a future implementation), it would cause a type error at runtime.To prevent future issues, add numeric coercion to the option definition:
new CommandOption( "--compression-level <level>", "The compression level to use when building the image." ) .argParser((v) => Number(v)) .hideHelp()Alternatively, ensure any future usage of
compressionLevelexplicitly coerces it viaNumber()orparseInt().
This will speed up ice cold starts (*) for two reasons:
This is a minor release because zstd compression will now be enabled by default for all deployments.
(*) ice cold starts happen when deploy images are not cached on the worker node yet. These cold start durations are highly dependent on image size and as it turns out, also the type of compression used.