GA --use-registry-with-docker and make the Satellite warnings less …

…prominent (#2266)

Closes #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 <vladaionescu@users.noreply.github.com>

.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

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

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")
 "

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
 	}

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 {

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
+	}
+}