Add --pull option to build command #3074

sondavidb · 2024-06-07T22:00:42Z

docker build initially had a --pull command that forced remote image lookup when building an image. This has been changed when docker switched to buildkit to be default behavior, with an option to not check for updates if the image is already cached.

Forcing nerdctl build to use local images may be desirable for a few edge cases, so I've implemented a solution via a --pull flag in nerdctl build that can be set to false to enable this behavior, and true to always pull from remote. Without it, it will use the buildkit default.

There's a caveat here, which is that buildkit must be set up to use the same namespace as the locally cached image, which the user must set up in their buildkit config file.

Tested in a similar workflow as the integration test:

Have a Dockerfile with just FROM ubuntu:latest
Pull an older image tag (for me, I chose ubuntu:18.04) into your buildkit namespace (default should be buildkit)
Tag it to latest version
nerdctl build --pull false .
Check image sha in output, it should be the same as the one locally cached
nerdctl build --pull true .
Check image sha in output, it should be a different one and should match the same as latest tag.

sondavidb · 2024-06-07T23:51:37Z

Does buildkit output to stderr? Integration tests were failing with AssertOutContains and passed with AssertErrContains, but it seems strange that buildkit is outputting to stdout instead of stderr. Makes me worried that I did something wrong.

Zheaoli · 2024-06-08T04:11:37Z

FYI docker/buildx#802

cmd/nerdctl/builder_build.go

AkihiroSuda · 2024-06-08T09:13:05Z

cmd/nerdctl/builder_build_test.go

+func TestBuildWithPull(t *testing.T) {
+	testutil.RequiresBuild(t)
+
+	oldImage := "ubuntu:18.04"


Any reason to use this very old version of Ubuntu?

Just for testing purposes. The idea is to pull an old version of ubuntu, tag it as latest, and see if --pull=false uses local image while --pull=true uses the remote image (latest vesion).

As we aren't actually running this built image, this doesn't pose any security risk should any vulnerabilities be found.

Can we just use the mirror registry, with updating the tag with another image before checking --pull behaviror?

Makes sense, I switched to using much slimmer images as well so this test will now run a little bit faster

cmd/nerdctl/builder_build_test.go

sondavidb · 2024-06-10T23:21:15Z

Last force-push revamped the integration test, so it should work properly. IMO it's a little bit ugly, since we're using os.Exec() to write directly to the config, but it's the only way I could think of approaching this, as buildctl doesn't support anything similar to a --config or a way to force the containerd worker instead of the OCI worker.

ktock

See comments

ktock · 2024-06-14T02:22:16Z

cmd/nerdctl/builder_build.go

@@ -51,6 +51,7 @@ If Dockerfile is not present and -f is not specified, it will look for Container
 	buildCommand.Flags().StringP("output", "o", "", "Output destination (format: type=local,dest=path)")
 	buildCommand.Flags().String("progress", "auto", "Set type of progress output (auto, plain, tty). Use plain to show container output")
 	buildCommand.Flags().String("provenance", "", "Shorthand for \"--attest=type=provenance\"")
+	buildCommand.Flags().String("pull", "", "On true, always attempt to pull latest image version from remote. Default uses buildkit's default.")


Can we make it a bool flag and just check if it's set when parsing?

Is this possible? From my understanding, the bool flag can only be true or false, and if none are provided then it will just set a default value.

Once parsed from a string, though, I can store it into a *bool instead of a string.

Have you tried Changed? https://pkg.go.dev/github.com/spf13/pflag#FlagSet.Changed

This is perfect, thanks. Just pushed a new set of changes.

ktock · 2024-06-14T02:27:54Z

cmd/nerdctl/builder_build_test.go

+func TestBuildWithPull(t *testing.T) {
+	testutil.RequiresBuild(t)
+
+	oldImage := "ubuntu:18.04"


Can we just use the mirror registry, with updating the tag with another image before checking --pull behaviror?

ktock · 2024-06-14T02:28:27Z

pkg/api/types/builder_types.go

+	// Pull determines if we should attempt to pull the latest image from remote.
+	// "true" will pull from remote while "false" will always use locally cached image
+	Pull string


Can we just make it *bool if we only take true/false/default ?

sondavidb · 2024-06-14T18:27:33Z

Had to move the test to builder_linux_test.go to make use of testutil. It probably makes more logical sense there, anyway, since the directory modified with the test is linux-specific.

ktock · 2024-06-17T10:59:15Z

cmd/nerdctl/builder_build.go

@@ -51,6 +51,7 @@ If Dockerfile is not present and -f is not specified, it will look for Container
 	buildCommand.Flags().StringP("output", "o", "", "Output destination (format: type=local,dest=path)")
 	buildCommand.Flags().String("progress", "auto", "Set type of progress output (auto, plain, tty). Use plain to show container output")
 	buildCommand.Flags().String("provenance", "", "Shorthand for \"--attest=type=provenance\"")
+	buildCommand.Flags().Bool("pull", true, "On true, always attempt to pull latest image version from remote. Default uses buildkit's default.")


Default value should be false.

ktock · 2024-06-17T11:00:03Z

cmd/nerdctl/builder_linux_test.go

+		name string
+		pull bool
+	}{
+		{


Can you add a test case for the default behaviour? (i.e. not specifying --pull)

Signed-off-by: David Son <davbson@amazon.com>

ktock

Thanks

sondavidb force-pushed the add-pull-build branch 5 times, most recently from 348bfb7 to 0613eac Compare June 7, 2024 23:48

AkihiroSuda reviewed Jun 8, 2024

View reviewed changes

cmd/nerdctl/builder_build.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed Jun 8, 2024

View reviewed changes

Zheaoli reviewed Jun 8, 2024

View reviewed changes

cmd/nerdctl/builder_build_test.go Outdated Show resolved Hide resolved

sondavidb force-pushed the add-pull-build branch 3 times, most recently from 4d44b73 to e3e33bb Compare June 10, 2024 23:19

sondavidb force-pushed the add-pull-build branch 2 times, most recently from c5f5be4 to 98de079 Compare June 11, 2024 00:07

AkihiroSuda requested a review from ktock June 14, 2024 02:08

ktock requested changes Jun 14, 2024

View reviewed changes

sondavidb force-pushed the add-pull-build branch 2 times, most recently from 4741cec to 5744262 Compare June 14, 2024 18:26

AkihiroSuda added this to the v2.0.0 milestone Jun 16, 2024

AkihiroSuda requested a review from ktock June 17, 2024 09:34

ktock reviewed Jun 17, 2024

View reviewed changes

sondavidb force-pushed the add-pull-build branch from 5744262 to 819f183 Compare June 17, 2024 20:43

Add --pull option to build command

b63f4d6

Signed-off-by: David Son <davbson@amazon.com>

sondavidb force-pushed the add-pull-build branch from 819f183 to b63f4d6 Compare June 17, 2024 21:22

ktock approved these changes Jun 18, 2024

View reviewed changes

ktock merged commit 5a0558c into containerd:main Jun 18, 2024
22 checks passed

sondavidb deleted the add-pull-build branch June 18, 2024 20:09

sondavidb mentioned this pull request Jun 19, 2024

[release/1.7 backport] Add --pull option to build command #3121

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --pull option to build command #3074

Add --pull option to build command #3074

sondavidb commented Jun 7, 2024 •

edited

Loading

sondavidb commented Jun 7, 2024

Zheaoli commented Jun 8, 2024

AkihiroSuda Jun 8, 2024

sondavidb Jun 10, 2024 •

edited

Loading

ktock Jun 14, 2024

sondavidb Jun 14, 2024

sondavidb commented Jun 10, 2024

ktock left a comment

ktock Jun 14, 2024

sondavidb Jun 14, 2024

ktock Jun 14, 2024

sondavidb Jun 14, 2024

ktock Jun 14, 2024

ktock Jun 14, 2024

sondavidb commented Jun 14, 2024 •

edited

Loading

ktock Jun 17, 2024

ktock Jun 17, 2024

sondavidb Jun 17, 2024

ktock left a comment

Add --pull option to build command #3074

Add --pull option to build command #3074

Conversation

sondavidb commented Jun 7, 2024 • edited Loading

sondavidb commented Jun 7, 2024

Zheaoli commented Jun 8, 2024

Choose a reason for hiding this comment

sondavidb Jun 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sondavidb commented Jun 10, 2024

ktock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sondavidb commented Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktock left a comment

Choose a reason for hiding this comment

sondavidb commented Jun 7, 2024 •

edited

Loading

sondavidb Jun 10, 2024 •

edited

Loading

sondavidb commented Jun 14, 2024 •

edited

Loading