From 8191e6d57cb25747d7780de7ec2db4ab3d891d2d Mon Sep 17 00:00:00 2001 From: "Vlad A. Ionescu" <446771+vladaionescu@users.noreply.github.com> Date: Wed, 12 Oct 2022 21:17:46 -0700 Subject: [PATCH] GA `--use-registry-with-docker` and make the Satellite warnings less prominent (#2266) Closes https://github.com/earthly/earthly/issues/1268 Note that the feature flag `--use-registry-with-docker` is backward compatible, hence we can enable it retroactively for all versions, now that we have confidence in the way it functions. For earthly-in-earthly used in our integration tests, `WITH DOCKER` doesn't work correctly with the embedded registry, due to the use of `NETWORK_MODE=host`. In such cases, we force the old behavior via the override feature flags functionality. Co-authored-by: Vlad A. Ionescu --- .earthly_version_flag_overrides | 2 +- CHANGELOG.md | 4 ++++ Earthfile | 7 +++---- cmd/earthly/build_cmd.go | 4 ++-- cmd/earthly/buildkit.go | 12 +++++------- features/features.go | 11 +++++++++++ 6 files changed, 26 insertions(+), 14 deletions(-) diff --git a/.earthly_version_flag_overrides b/.earthly_version_flag_overrides index bbff75afc0..878cb439d6 100644 --- a/.earthly_version_flag_overrides +++ b/.earthly_version_flag_overrides @@ -1 +1 @@ -referenced-save-only,use-copy-include-patterns,earthly-version-arg,use-cache-command,use-host-command,check-duplicate-images,use-copy-link,parallel-load,shell-out-anywhere,new-platform,no-tar-build-output,wait-block,use-registry-for-with-docker +referenced-save-only,use-copy-include-patterns,earthly-version-arg,use-cache-command,use-host-command,check-duplicate-images,use-copy-link,parallel-load,shell-out-anywhere,new-platform,no-tar-build-output,wait-block diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f4b901b6c..79d777a30f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ All notable changes to [Earthly](https://github.com/earthly/earthly) will be doc - Some network operations were being incorrectly executed with a timeout of 0. +### Changed + +- Loading Docker images as part of `WITH DOCKER` is now faster through the use of an embedded registry in Buildkit. This functionality was previously hidden (`VERSION --use-registry-for-with-docker`) and was only auto-enabled for Earthly Satellite users. It is now enabled by default for all builds. [#1268](https://github.com/earthly/earthly/issues/1268) + ## v0.6.25 - 2022-10-04 ### Fixed diff --git a/Earthfile b/Earthfile index ef6c4a0523..22e3d5c272 100644 --- a/Earthfile +++ b/Earthfile @@ -312,7 +312,8 @@ earthly-docker: earthly-integration-test-base: FROM +earthly-docker ENV NO_DOCKER=1 - ENV NETWORK_MODE=host + ENV NETWORK_MODE=host # Note that this breaks access to embedded registry in WITH DOCKER. + ENV EARTHLY_VERSION_FLAG_OVERRIDES=no-use-registry-for-with-docker # Use tar-based due to above. WORKDIR /test # The inner buildkit requires Docker hub creds to prevent rate-limiting issues. @@ -324,7 +325,7 @@ earthly-integration-test-base: IF [ -z $DOCKERHUB_MIRROR ] # No mirror, easy CI and local use by all - ENV GLOBAL_CONFIG="{disable_analytics: true, local_registry_host: 'tcp://127.0.0.1:8371', conversion_parallelism: 5}" + ENV GLOBAL_CONFIG="{disable_analytics: true}" IF [ "$DOCKERHUB_AUTH" = "true" ] RUN --secret USERNAME=$DOCKERHUB_USER_SECRET \ --secret TOKEN=$DOCKERHUB_TOKEN_SECRET \ @@ -345,8 +346,6 @@ earthly-integration-test-base: # NOTE: newlines+indentation is important here, see https://github.com/earthly/earthly/issues/1764 for potential pitfalls # yaml will convert newlines to spaces when using regular quoted-strings, therefore we will use the literal-style (denoted by `|`) ENV GLOBAL_CONFIG="disable_analytics: true -local_registry_host: 'tcp://127.0.0.1:8371' -conversion_parallelism: 5 buildkit_additional_config: | $(echo "$EARTHLY_ADDITIONAL_BUILDKIT_CONFIG" | sed "s/^/ /g") " diff --git a/cmd/earthly/build_cmd.go b/cmd/earthly/build_cmd.go index 0ba4446625..c5ad872869 100644 --- a/cmd/earthly/build_cmd.go +++ b/cmd/earthly/build_cmd.go @@ -209,8 +209,8 @@ func (app *earthlyApp) actionBuildImp(cliCtx *cli.Context, flagArgs, nonFlagArgs isLocal := containerutil.IsLocal(app.buildkitdSettings.BuildkitAddress) if !isLocal && app.ci { - app.console.Warnf("Please note that --use-inline-cache and --save-inline-cache are currently disabled when using --ci on Satellites or remote Buildkit.") - app.console.Warnf("") // newline + app.console.Printf("Please note that --use-inline-cache and --save-inline-cache are currently disabled when using --ci on Satellites or remote Buildkit.") + app.console.Printf("") // newline app.useInlineCache = false app.saveInlineCache = false } diff --git a/cmd/earthly/buildkit.go b/cmd/earthly/buildkit.go index 94bd326620..066d358030 100644 --- a/cmd/earthly/buildkit.go +++ b/cmd/earthly/buildkit.go @@ -147,8 +147,6 @@ func (app *earthlyApp) configureSatellite(cliCtx *cli.Context, cloudClient cloud return nil } - app.console.Warnf("Note: inline caching does not yet work on Earthly Satellites.") - // Set up extra settings needed for buildkit RPC metadata if app.satelliteName == "" { app.satelliteName = app.cfg.Satellite.Name @@ -170,15 +168,15 @@ func (app *earthlyApp) configureSatellite(cliCtx *cli.Context, cloudClient cloud app.analyticsMetadata.isSatellite = true app.analyticsMetadata.satelliteVersion = "" // TODO - app.console.Warnf("") // newline - app.console.Warnf("The following feature flags are recommended for use with Satellites and will be auto-enabled:") - app.console.Warnf(" --new-platform, --use-registry-for-with-docker") - app.console.Warnf("") // newline + app.console.Printf("") // newline + app.console.Printf("The following feature flag is recommended for use with Satellites and will be auto-enabled:") + app.console.Printf(" --new-platform") + app.console.Printf("") // newline if app.featureFlagOverrides != "" { app.featureFlagOverrides += "," } - app.featureFlagOverrides += "new-platform,use-registry-for-with-docker" + app.featureFlagOverrides += "new-platform" token, err := cloudClient.GetAuthToken(cliCtx.Context) if err != nil { diff --git a/features/features.go b/features/features.go index 4b9a3bc0cc..fcc3fceb2b 100644 --- a/features/features.go +++ b/features/features.go @@ -44,6 +44,8 @@ type Features struct { UseProjectSecrets bool `long:"use-project-secrets" description:"enable project-based secret resolution"` UsePipelines bool `long:"use-pipelines" description:"enable the PIPELINE and TRIGGER commands"` + NoUseRegistryForWithDocker bool `long:"no-use-registry-for-with-docker" description:"disable use-registry-for-with-docker"` + Major int Minor int } @@ -134,6 +136,7 @@ func ApplyFlagOverrides(ftrs *Features, envOverrides string) error { return fmt.Errorf("unable to set %s: only boolean fields are currently supported", key) } } + processNegativeFlags(ftrs) return nil } @@ -193,6 +196,7 @@ func GetFeatures(version *spec.Version) (*Features, bool, error) { if versionAtLeast(ftrs, 0, 5) { ftrs.ExecAfterParallel = true ftrs.ParallelLoad = true + ftrs.UseRegistryForWithDocker = true } if versionAtLeast(ftrs, 0, 6) { ftrs.ReferencedSaveOnly = true @@ -213,6 +217,7 @@ func GetFeatures(version *spec.Version) (*Features, bool, error) { ftrs.UseNoManifestList = true ftrs.UseChmod = true } + processNegativeFlags(&ftrs) return &ftrs, hasVersion, nil } @@ -222,3 +227,9 @@ func GetFeatures(version *spec.Version) (*Features, bool, error) { func versionAtLeast(ftrs Features, majorVersion, minorVersion int) bool { return (ftrs.Major > majorVersion) || (ftrs.Major == majorVersion && ftrs.Minor >= minorVersion) } + +func processNegativeFlags(ftrs *Features) { + if ftrs.NoUseRegistryForWithDocker { + ftrs.UseRegistryForWithDocker = false + } +}