From 3bd6c2694f064b658315511d60ce143291b5b3ce Mon Sep 17 00:00:00 2001 From: Kilian Hanich Date: Mon, 11 Mar 2024 20:26:33 +0100 Subject: [PATCH 1/6] create-build: added --build option uses the given path as the build context forwarded to podman build afterwards it extracts the name of the image and sets it to satisfy other assumption in existing code the test builds a Fedora 39 toolbx and checks if the image is successfully tagged as localhost/fedora-toolbox Signed-off-by: Kilian Hanich --- src/cmd/create.go | 19 ++++++++++++++- src/cmd/enter.go | 3 ++- src/cmd/run.go | 3 ++- src/cmd/utils.go | 24 +++++++++++++++---- src/pkg/podman/podman.go | 47 +++++++++++++++++++++++++++++++++++++ src/pkg/utils/errors.go | 10 ++++++++ test/system/101-create.bats | 15 ++++++++++++ 7 files changed, 113 insertions(+), 8 deletions(-) diff --git a/src/cmd/create.go b/src/cmd/create.go index 247b809..e8ee9d3 100644 --- a/src/cmd/create.go +++ b/src/cmd/create.go @@ -54,6 +54,7 @@ var ( distro string image string release string + build string } createToolboxShMounts = []struct { @@ -104,6 +105,12 @@ func init() { "", "Create a Toolbx container for a different operating system release than the host") + flags.StringVarP(&createFlags.build, + "build", + "b", + "", + "Build a Toolbx container for use of this container") + createCmd.SetHelpFunc(createHelp) if err := createCmd.RegisterFlagCompletionFunc("distro", completionDistroNames); err != nil { @@ -150,6 +157,15 @@ func create(cmd *cobra.Command, args []string) error { return errors.New(errMsg) } + if cmd.Flag("build").Changed && (cmd.Flag("image").Changed || cmd.Flag("release").Changed || cmd.Flag("distro").Changed) { + var builder strings.Builder + fmt.Fprintf(&builder, "options --build and --release, --image or -- distro cannot be used together\n") + fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase) + + errMsg := builder.String() + return errors.New(errMsg) + } + if cmd.Flag("authfile").Changed { if !utils.PathExists(createFlags.authFile) { var builder strings.Builder @@ -177,7 +193,8 @@ func create(cmd *cobra.Command, args []string) error { containerArg, createFlags.distro, createFlags.image, - createFlags.release) + createFlags.release, + createFlags.build) if err != nil { return err diff --git a/src/cmd/enter.go b/src/cmd/enter.go index c82ca81..0a0017a 100644 --- a/src/cmd/enter.go +++ b/src/cmd/enter.go @@ -111,7 +111,8 @@ func enter(cmd *cobra.Command, args []string) error { containerArg, enterFlags.distro, "", - enterFlags.release) + enterFlags.release, + "") if err != nil { return err diff --git a/src/cmd/run.go b/src/cmd/run.go index 85d90bc..a7bcc75 100644 --- a/src/cmd/run.go +++ b/src/cmd/run.go @@ -147,7 +147,8 @@ func run(cmd *cobra.Command, args []string) error { "--container", runFlags.distro, "", - runFlags.release) + runFlags.release, + "") if err != nil { return err diff --git a/src/cmd/utils.go b/src/cmd/utils.go index c5c3523..c450759 100644 --- a/src/cmd/utils.go +++ b/src/cmd/utils.go @@ -31,6 +31,7 @@ import ( "strings" "syscall" + "github.com/containers/toolbox/pkg/podman" "github.com/containers/toolbox/pkg/utils" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" @@ -402,13 +403,26 @@ func poll(pollFn pollFunc, eventFD int32, fds ...int32) error { } } -func resolveContainerAndImageNames(container, containerArg, distroCLI, imageCLI, releaseCLI string) ( +func resolveContainerAndImageNames(container, containerArg, distroCLI, imageCLI, releaseCLI, buildCLI string) ( string, string, string, error, ) { - container, image, release, err := utils.ResolveContainerAndImageNames(container, - distroCLI, - imageCLI, - releaseCLI) + var image, release string + var err error + if buildCLI == "" { + container, image, release, err = utils.ResolveContainerAndImageNames(container, + distroCLI, + imageCLI, + releaseCLI) + } else { + image, err = podman.BuildImage(buildCLI) + if err != nil { + return "", "", "", err + } + container, image, release, err = utils.ResolveContainerAndImageNames(container, + distroCLI, + image, + releaseCLI) + } if err != nil { var errContainer *utils.ContainerError diff --git a/src/pkg/podman/podman.go b/src/pkg/podman/podman.go index 4711b8b..5747435 100644 --- a/src/pkg/podman/podman.go +++ b/src/pkg/podman/podman.go @@ -23,7 +23,9 @@ import ( "errors" "fmt" "io" + "os" "strconv" + "strings" "time" "github.com/HarryMichal/go-version" @@ -53,6 +55,12 @@ var ( LogLevel = logrus.ErrorLevel ) +var ( + ErrBuildContextDoesNotExist = errors.New("build context does not exist") + + ErrBuildContextInvalid = errors.New("build context is not a directory with a Containerfile") +) + func (image *Image) FlattenNames(fillNameWithID bool) []Image { var ret []Image @@ -129,6 +137,45 @@ func (images ImageSlice) Swap(i, j int) { images[i], images[j] = images[j], images[i] } +func BuildImage(buildContext string) (string, error) { + if !utils.PathExists(buildContext) { + return "", &utils.BuildError{BuildContext: buildContext, Err: ErrBuildContextDoesNotExist} + } + if stat, err := os.Stat(buildContext); err != nil { + return "", err + } else { + if !stat.Mode().IsDir() { + return "", &utils.BuildError{BuildContext: buildContext, Err: ErrBuildContextInvalid} + } + } + if !utils.PathExists(buildContext+"/Containerfile") && !utils.PathExists(buildContext+"/Dockerfile") { + return "", &utils.BuildError{BuildContext: buildContext, Err: ErrBuildContextInvalid} + } + logLevelString := LogLevel.String() + args := []string{"--log-level", logLevelString, "build", buildContext} + + stdout := new(bytes.Buffer) + if err := shell.Run("podman", nil, stdout, nil, args...); err != nil { + return "", err + } + output := strings.TrimRight(stdout.String(), "\n") + imageIdBegin := strings.LastIndex(output, "\n") + 1 + imageId := output[imageIdBegin:] + + info, err := InspectImage(imageId) + if err != nil { + return "", err + } + + name := "localhost/" + info["Labels"].(map[string]interface{})["name"].(string) + args = []string{"--log-level", logLevelString, "tag", imageId, name} + if err := shell.Run("podman", nil, nil, nil, args...); err != nil { + return "", err + } + + return name, nil +} + // CheckVersion compares provided version with the version of Podman. // // Takes in one string parameter that should be in the format that is used for versioning (eg. 1.0.0, 2.5.1-dev). diff --git a/src/pkg/utils/errors.go b/src/pkg/utils/errors.go index a73fdf0..921a544 100644 --- a/src/pkg/utils/errors.go +++ b/src/pkg/utils/errors.go @@ -40,6 +40,11 @@ type ParseReleaseError struct { Hint string } +type BuildError struct { + BuildContext string + Err error +} + func (err *ContainerError) Error() string { errMsg := fmt.Sprintf("%s: %s", err.Container, err.Err) return errMsg @@ -70,3 +75,8 @@ func (err *ImageError) Unwrap() error { func (err *ParseReleaseError) Error() string { return err.Hint } + +func (err *BuildError) Error() string { + errMsg := fmt.Sprintf("%s: %s", err.BuildContext, err.Err) + return errMsg +} diff --git a/test/system/101-create.bats b/test/system/101-create.bats index 00572a2..4fe48f6 100644 --- a/test/system/101-create.bats +++ b/test/system/101-create.bats @@ -841,3 +841,18 @@ teardown() { assert_line --index 1 "Enter with: toolbox enter fedora-toolbox-34" assert [ ${#lines[@]} -eq 2 ] } + +@test "create: Build an image before creating the toolbox" { + local build_context="./images/fedora/f39" + + run "$TOOLBX" create --build "$build_context" + assert_success + + assert_line --index 0 "Created container: fedora-toolbox" + assert_line --index 1 "Enter with: toolbox enter fedora-toolbox" + assert [ ${#lines[@]} -eq 2 ] + + run $PODMAN images --filter reference=localhost/fedora-toolbox + assert_success + assert [ $(#lines[@]) -eq 2 ] +} From 2fd6b30c2f1659c62f343179ec4cf56f23f95cb1 Mon Sep 17 00:00:00 2001 From: Kilian Hanich Date: Mon, 11 Mar 2024 20:29:03 +0100 Subject: [PATCH 2/6] doc: document the added --build option for the create subcommand Signed-off-by: Kilian Hanich --- doc/toolbox-create.1.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/toolbox-create.1.md b/doc/toolbox-create.1.md index b2dacb3..f4fb65a 100644 --- a/doc/toolbox-create.1.md +++ b/doc/toolbox-create.1.md @@ -8,6 +8,7 @@ toolbox\-create - Create a new Toolbx container [*--distro DISTRO* | *-d DISTRO*] [*--image NAME* | *-i NAME*] [*--release RELEASE* | *-r RELEASE*] + [*--build BUILDCONTEXT* | *-b BUILDCONTEXT*] [*CONTAINER*] ## DESCRIPTION @@ -110,6 +111,14 @@ remote registry. Create a Toolbx container for a different operating system RELEASE than the host. Cannot be used with `--image`. +**--build** BUILDCONTEXT, **-b** BUILDCONTEXT + +Build a toolbx image from the build context found at BUILDCONTEXT by passing it +to `podman build`. Afterwards it sets the tag to `localhost/` +by extracting the name from the image and then creates the container like normal. + +You cannot use `--distro`, `--release` or `--image` together with this option. + ## EXAMPLES ### Create the default Toolbx container matching the host OS From 2ca688b732233c93255a42d216e3edf44c118641 Mon Sep 17 00:00:00 2001 From: Kilian Hanich Date: Mon, 11 Mar 2024 20:34:04 +0100 Subject: [PATCH 3/6] create-build-tag: added --build-tag option tags the built image of --build by forwarding the value to --tag of podman build added a test case with and without a repository registry.fedoraproject.org was choosen as repository because podman doesn't let one chose arbitrarily Signed-off-by: Kilian Hanich --- src/cmd/create.go | 18 ++++++++++++++- src/cmd/enter.go | 3 ++- src/cmd/run.go | 2 +- src/cmd/utils.go | 4 ++-- src/pkg/podman/podman.go | 46 +++++++++++++++++++++++-------------- test/system/101-create.bats | 34 ++++++++++++++++++++++++++- 6 files changed, 84 insertions(+), 23 deletions(-) diff --git a/src/cmd/create.go b/src/cmd/create.go index e8ee9d3..4313c38 100644 --- a/src/cmd/create.go +++ b/src/cmd/create.go @@ -55,6 +55,7 @@ var ( image string release string build string + buildtag string } createToolboxShMounts = []struct { @@ -111,6 +112,12 @@ func init() { "", "Build a Toolbx container for use of this container") + flags.StringVarP(&createFlags.buildtag, + "build-tag", + "t", + "", + "Tag the image built") + createCmd.SetHelpFunc(createHelp) if err := createCmd.RegisterFlagCompletionFunc("distro", completionDistroNames); err != nil { @@ -166,6 +173,15 @@ func create(cmd *cobra.Command, args []string) error { return errors.New(errMsg) } + if cmd.Flag("build-tag").Changed && !cmd.Flag("build").Changed { + var builder strings.Builder + fmt.Fprintf(&builder, "--build-tag must be used together with --build\n") + fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase) + + errMsg := builder.String() + return errors.New(errMsg) + } + if cmd.Flag("authfile").Changed { if !utils.PathExists(createFlags.authFile) { var builder strings.Builder @@ -194,7 +210,7 @@ func create(cmd *cobra.Command, args []string) error { createFlags.distro, createFlags.image, createFlags.release, - createFlags.build) + podman.BuildOptions{Context: createFlags.build, Tag: createFlags.buildtag}) if err != nil { return err diff --git a/src/cmd/enter.go b/src/cmd/enter.go index 0a0017a..5ed8bd8 100644 --- a/src/cmd/enter.go +++ b/src/cmd/enter.go @@ -21,6 +21,7 @@ import ( "fmt" "os" + "github.com/containers/toolbox/pkg/podman" "github.com/containers/toolbox/pkg/utils" "github.com/spf13/cobra" ) @@ -112,7 +113,7 @@ func enter(cmd *cobra.Command, args []string) error { enterFlags.distro, "", enterFlags.release, - "") + podman.BuildOptions{Context: "", Tag: ""}) if err != nil { return err diff --git a/src/cmd/run.go b/src/cmd/run.go index a7bcc75..9cddb07 100644 --- a/src/cmd/run.go +++ b/src/cmd/run.go @@ -148,7 +148,7 @@ func run(cmd *cobra.Command, args []string) error { runFlags.distro, "", runFlags.release, - "") + podman.BuildOptions{Context: "", Tag: ""}) if err != nil { return err diff --git a/src/cmd/utils.go b/src/cmd/utils.go index c450759..2cb4bc1 100644 --- a/src/cmd/utils.go +++ b/src/cmd/utils.go @@ -403,12 +403,12 @@ func poll(pollFn pollFunc, eventFD int32, fds ...int32) error { } } -func resolveContainerAndImageNames(container, containerArg, distroCLI, imageCLI, releaseCLI, buildCLI string) ( +func resolveContainerAndImageNames(container, containerArg, distroCLI, imageCLI, releaseCLI string, buildCLI podman.BuildOptions) ( string, string, string, error, ) { var image, release string var err error - if buildCLI == "" { + if buildCLI.Context == "" { container, image, release, err = utils.ResolveContainerAndImageNames(container, distroCLI, imageCLI, diff --git a/src/pkg/podman/podman.go b/src/pkg/podman/podman.go index 5747435..45c0774 100644 --- a/src/pkg/podman/podman.go +++ b/src/pkg/podman/podman.go @@ -41,6 +41,11 @@ type Image struct { Names []string } +type BuildOptions struct { + Context string + Tag string +} + type ImageSlice []Image var ( @@ -137,22 +142,25 @@ func (images ImageSlice) Swap(i, j int) { images[i], images[j] = images[j], images[i] } -func BuildImage(buildContext string) (string, error) { - if !utils.PathExists(buildContext) { - return "", &utils.BuildError{BuildContext: buildContext, Err: ErrBuildContextDoesNotExist} +func BuildImage(build BuildOptions) (string, error) { + if !utils.PathExists(build.Context) { + return "", &utils.BuildError{BuildContext: build.Context, Err: ErrBuildContextDoesNotExist} } - if stat, err := os.Stat(buildContext); err != nil { + if stat, err := os.Stat(build.Context); err != nil { return "", err } else { if !stat.Mode().IsDir() { - return "", &utils.BuildError{BuildContext: buildContext, Err: ErrBuildContextInvalid} + return "", &utils.BuildError{BuildContext: build.Context, Err: ErrBuildContextInvalid} } } - if !utils.PathExists(buildContext+"/Containerfile") && !utils.PathExists(buildContext+"/Dockerfile") { - return "", &utils.BuildError{BuildContext: buildContext, Err: ErrBuildContextInvalid} + if !utils.PathExists(build.Context+"/Containerfile") && !utils.PathExists(build.Context+"/Dockerfile") { + return "", &utils.BuildError{BuildContext: build.Context, Err: ErrBuildContextInvalid} } logLevelString := LogLevel.String() - args := []string{"--log-level", logLevelString, "build", buildContext} + args := []string{"--log-level", logLevelString, "build", build.Context} + if build.Tag != "" { + args = append(args, "--tag", build.Tag) + } stdout := new(bytes.Buffer) if err := shell.Run("podman", nil, stdout, nil, args...); err != nil { @@ -162,15 +170,19 @@ func BuildImage(buildContext string) (string, error) { imageIdBegin := strings.LastIndex(output, "\n") + 1 imageId := output[imageIdBegin:] - info, err := InspectImage(imageId) - if err != nil { - return "", err - } - - name := "localhost/" + info["Labels"].(map[string]interface{})["name"].(string) - args = []string{"--log-level", logLevelString, "tag", imageId, name} - if err := shell.Run("podman", nil, nil, nil, args...); err != nil { - return "", err + var name string + if build.Tag == "" { + info, err := InspectImage(imageId) + if err != nil { + return "", err + } + name = info["Labels"].(map[string]interface{})["name"].(string) + args = []string{"--log-level", logLevelString, "tag", imageId, name} + if err := shell.Run("podman", nil, nil, nil, args...); err != nil { + return "", err + } + } else { + name = build.Tag } return name, nil diff --git a/test/system/101-create.bats b/test/system/101-create.bats index 4fe48f6..3ee0ee7 100644 --- a/test/system/101-create.bats +++ b/test/system/101-create.bats @@ -854,5 +854,37 @@ teardown() { run $PODMAN images --filter reference=localhost/fedora-toolbox assert_success - assert [ $(#lines[@]) -eq 2 ] + assert [ ${#lines[@]} -eq 2 ] +} + +@test "create: Build an image and tag it before creating the toolbox without repository" { + local build_context="./images/fedora/f39" + local build_tag="testbuild" + + run "$TOOLBX" create --build "&build_context" --build-tag "testbuild" + assert_success + + assert_line --index 0 "Created container: testbuild" + assert_line --index 1 "Enter with: toolbox enter testbuild" + assert [ ${#lines[q]} -eq 2 ] + + run $PODMAN images --filter reference=localhost/testbuild + assert_success + assert [ ${#lines[@]} -eq 2 ] +} + +@test "create: Build an image and tag it before creating the toolbox with repository" { + local build_context="./images/fedora/f39" + local build_tag="registry.fedoraproject.org/testbuild" + + run "$TOOLBX" create --build "&build_context" --build-tag "testbuild" + assert_success + + assert_line --index 0 "Created container: testbuild" + assert_line --index 1 "Enter with: toolbox enter testbuild" + assert [ ${#lines[q]} -eq 2 ] + + run $PODMAN images --filter reference="$build_tag" + assert_success + assert [ ${#lines[@]} -eq 2 ] } From b868db5e4c16d2940448e1b45a6cd17177fa104a Mon Sep 17 00:00:00 2001 From: Kilian Hanich Date: Mon, 11 Mar 2024 20:39:32 +0100 Subject: [PATCH 4/6] doc: document the added --build-tag option for the create subcommand Signed-off-by: Kilian Hanich --- doc/toolbox-create.1.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/toolbox-create.1.md b/doc/toolbox-create.1.md index f4fb65a..00be221 100644 --- a/doc/toolbox-create.1.md +++ b/doc/toolbox-create.1.md @@ -9,6 +9,7 @@ toolbox\-create - Create a new Toolbx container [*--image NAME* | *-i NAME*] [*--release RELEASE* | *-r RELEASE*] [*--build BUILDCONTEXT* | *-b BUILDCONTEXT*] + [*--build-tag TAG* | *-t TAG*] [*CONTAINER*] ## DESCRIPTION @@ -119,6 +120,14 @@ by extracting the name from the image and then creates the container like normal You cannot use `--distro`, `--release` or `--image` together with this option. +**--build-tag** TAG, **-t** TAG + +Overwrites the tagging behaviour of `--build` by tagging the image with TAG via +`podman build --tag`. If no repository if given or podman doesn't know it, +localhost is used. + +Can only be used when `--build` is also used. + ## EXAMPLES ### Create the default Toolbx container matching the host OS From 6e58a0d5255b466e70801a2edd9ed6d096fa50ec Mon Sep 17 00:00:00 2001 From: Kilian Hanich Date: Mon, 11 Mar 2024 21:55:50 +0100 Subject: [PATCH 5/6] added missing parameter Signed-off-by: Kilian Hanich --- src/cmd/rootMigrationPath.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cmd/rootMigrationPath.go b/src/cmd/rootMigrationPath.go index da11a58..2c1b42e 100644 --- a/src/cmd/rootMigrationPath.go +++ b/src/cmd/rootMigrationPath.go @@ -25,6 +25,7 @@ import ( "os" "strings" + "github.com/containers/toolbox/pkg/podman" "github.com/containers/toolbox/pkg/utils" "github.com/spf13/cobra" ) @@ -60,7 +61,7 @@ func rootRunImpl(cmd *cobra.Command, args []string) error { return nil } - container, image, release, err := resolveContainerAndImageNames("", "", "", "", "") + container, image, release, err := resolveContainerAndImageNames("", "", "", "", "", podman.BuildOptions{Context: "", Tag: ""}) if err != nil { return err } From b56a4ccf631db64cd3bf1ee6b90adb3e0f92d26f Mon Sep 17 00:00:00 2001 From: Kilian Hanich Date: Mon, 11 Mar 2024 22:52:35 +0100 Subject: [PATCH 6/6] fixing bugs around the build feature image to build in the tests is also moved to Fedora 38 as the Fedora 39 file doesn't work at the time of writing Signed-off-by: Kilian Hanich --- src/pkg/shell/shell.go | 2 +- test/system/101-create.bats | 34 +++++++++++++++++++--------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/pkg/shell/shell.go b/src/pkg/shell/shell.go index 0eb4c94..6b964e8 100644 --- a/src/pkg/shell/shell.go +++ b/src/pkg/shell/shell.go @@ -39,7 +39,7 @@ func RunContext(ctx context.Context, name string, stdin io.Reader, stdout, stder return err } if exitCode != 0 { - return fmt.Errorf("failed to invoke %s(1)", name) + return fmt.Errorf("failed to invoke %s(%d)", name, exitCode) } return nil } diff --git a/test/system/101-create.bats b/test/system/101-create.bats index 3ee0ee7..00cec6b 100644 --- a/test/system/101-create.bats +++ b/test/system/101-create.bats @@ -843,10 +843,13 @@ teardown() { } @test "create: Build an image before creating the toolbox" { - local build_context="./images/fedora/f39" + local build_context="./images/fedora/f38" run "$TOOLBX" create --build "$build_context" - assert_success + if [ "$status" -ne 0 ] + then + echo "$output" + fi assert_line --index 0 "Created container: fedora-toolbox" assert_line --index 1 "Enter with: toolbox enter fedora-toolbox" @@ -858,33 +861,34 @@ teardown() { } @test "create: Build an image and tag it before creating the toolbox without repository" { - local build_context="./images/fedora/f39" + local build_context="./images/fedora/f38" local build_tag="testbuild" - run "$TOOLBX" create --build "&build_context" --build-tag "testbuild" + run "$TOOLBX" create --build "$build_context" --build-tag "$build_tag" assert_success - assert_line --index 0 "Created container: testbuild" - assert_line --index 1 "Enter with: toolbox enter testbuild" - assert [ ${#lines[q]} -eq 2 ] + assert_line --index 0 "Created container: $build_tag" + assert_line --index 1 "Enter with: toolbox enter $build_tag" + assert [ ${#lines[@]} -eq 2 ] - run $PODMAN images --filter reference=localhost/testbuild + run $PODMAN images --filter reference="localhost/$build_tag" assert_success assert [ ${#lines[@]} -eq 2 ] } @test "create: Build an image and tag it before creating the toolbox with repository" { - local build_context="./images/fedora/f39" - local build_tag="registry.fedoraproject.org/testbuild" + local build_context="./images/fedora/f38" + local tag_repository="registry.fedoraproject.org" + local build_tag="testbuild" - run "$TOOLBX" create --build "&build_context" --build-tag "testbuild" + run "$TOOLBX" create --build "$build_context" --build-tag "$tag_repository/$build_tag" assert_success - assert_line --index 0 "Created container: testbuild" - assert_line --index 1 "Enter with: toolbox enter testbuild" - assert [ ${#lines[q]} -eq 2 ] + assert_line --index 0 "Created container: $build_tag" + assert_line --index 1 "Enter with: toolbox enter $build_tag" + assert [ ${#lines[@]} -eq 2 ] - run $PODMAN images --filter reference="$build_tag" + run $PODMAN images --filter reference="$tag_repository/$build_tag" assert_success assert [ ${#lines[@]} -eq 2 ] }