From 1b5853e64794403d80c4d339c97b61dd63dd093a Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 24 Sep 2020 18:38:52 -0400 Subject: Properly handle podman run --pull command Currently the --pull missing|always|never is ignored This PR implements this for local API. For remote we need to default to pullpolicy specified in the containers.conf file. Also fixed an issue when images were matching other images names based on prefix, causing images to always be pulled. I had named an image myfedora and when ever I pulled fedora, the system thought that it there were two images named fedora since it was checking for the name fedora as well as the prefix fedora. I changed it to check for fedora and the prefix /fedora, to prefent failures like I had. Signed-off-by: Daniel J Walsh --- cmd/podman/common/create.go | 2 +- cmd/podman/containers/create.go | 1 + libpod/image/utils.go | 2 +- pkg/api/handlers/compat/images.go | 14 +++++++++++-- pkg/domain/entities/images.go | 7 +++++-- pkg/domain/infra/abi/images.go | 2 +- test/e2e/run_apparmor_test.go | 2 +- test/e2e/run_test.go | 42 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 64 insertions(+), 8 deletions(-) diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index 7e3dc7fb4..bb4726817 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -363,7 +363,7 @@ func GetCreateFlags(cf *ContainerCLIOpts) *pflag.FlagSet { ) createFlags.StringVar( &cf.Pull, - "pull", "missing", + "pull", containerConfig.Engine.PullPolicy, `Pull image before creating ("always"|"missing"|"never")`, ) createFlags.BoolVarP( diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index 96d94dc00..d75352848 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -261,6 +261,7 @@ func pullImage(imageName string) (string, error) { OverrideOS: cliVals.OverrideOS, OverrideVariant: cliVals.OverrideVariant, SignaturePolicy: cliVals.SignaturePolicy, + PullPolicy: pullPolicy, }) if pullErr != nil { return "", pullErr diff --git a/libpod/image/utils.go b/libpod/image/utils.go index 918314476..2538f429b 100644 --- a/libpod/image/utils.go +++ b/libpod/image/utils.go @@ -33,7 +33,7 @@ func findImageInRepotags(search imageParts, images []*Image) (*storage.Image, er continue } // account for registry:/somedir/image - if strings.HasSuffix(dName, searchName) && dSuspiciousTagValueForSearch == searchSuspiciousTagValueForSearch { + if strings.HasSuffix(dName, "/"+searchName) && dSuspiciousTagValueForSearch == searchSuspiciousTagValueForSearch { results = append(results, image.image) continue } diff --git a/pkg/api/handlers/compat/images.go b/pkg/api/handlers/compat/images.go index c1ba9ca66..0900d1793 100644 --- a/pkg/api/handlers/compat/images.go +++ b/pkg/api/handlers/compat/images.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/containers/buildah" + "github.com/containers/common/pkg/config" "github.com/containers/image/v5/manifest" "github.com/containers/podman/v2/libpod" image2 "github.com/containers/podman/v2/libpod/image" @@ -17,7 +18,6 @@ import ( "github.com/containers/podman/v2/pkg/api/handlers/utils" "github.com/containers/podman/v2/pkg/auth" "github.com/containers/podman/v2/pkg/domain/entities" - "github.com/containers/podman/v2/pkg/util" "github.com/docker/docker/api/types" "github.com/gorilla/schema" "github.com/pkg/errors" @@ -268,6 +268,16 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) { if sys := runtime.SystemContext(); sys != nil { registryOpts.DockerCertPath = sys.DockerCertPath } + rtc, err := runtime.GetConfig() + if err != nil { + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()")) + return + } + pullPolicy, err := config.ValidatePullPolicy(rtc.Engine.PullPolicy) + if err != nil { + utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()")) + return + } img, err := runtime.ImageRuntime().New(r.Context(), fromImage, "", // signature policy @@ -276,7 +286,7 @@ func CreateImageFromImage(w http.ResponseWriter, r *http.Request) { ®istryOpts, image2.SigningOptions{}, nil, // label - util.PullImageMissing, + pullPolicy, ) if err != nil { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err) diff --git a/pkg/domain/entities/images.go b/pkg/domain/entities/images.go index cad6693fa..ac81c282d 100644 --- a/pkg/domain/entities/images.go +++ b/pkg/domain/entities/images.go @@ -3,6 +3,7 @@ package entities import ( "time" + "github.com/containers/common/pkg/config" "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/types" "github.com/containers/podman/v2/pkg/inspect" @@ -119,8 +120,8 @@ type ImageHistoryReport struct { // ImagePullOptions are the arguments for pulling images. type ImagePullOptions struct { - // AllTags can be specified to pull all tags of the spiecifed image. Note - // that this only works if the specified image does not include a tag. + // AllTags can be specified to pull all tags of an image. Note + // that this only works if the image does not include a tag. AllTags bool // Authfile is the path to the authentication file. Ignored for remote // calls. @@ -146,6 +147,8 @@ type ImagePullOptions struct { SignaturePolicy string // SkipTLSVerify to skip HTTPS and certificate verification. SkipTLSVerify types.OptionalBool + // PullPolicy whether to pull new image + PullPolicy config.PullPolicy } // ImagePullReport is the response from pulling one or more images. diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 25c0c184f..d56dc7d94 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -255,7 +255,7 @@ func pull(ctx context.Context, runtime *image.Runtime, rawImage string, options } if !options.AllTags { - newImage, err := runtime.New(ctx, rawImage, options.SignaturePolicy, options.Authfile, writer, &dockerRegistryOptions, image.SigningOptions{}, label, util.PullImageAlways) + newImage, err := runtime.New(ctx, rawImage, options.SignaturePolicy, options.Authfile, writer, &dockerRegistryOptions, image.SigningOptions{}, label, options.PullPolicy) if err != nil { return nil, err } diff --git a/test/e2e/run_apparmor_test.go b/test/e2e/run_apparmor_test.go index 7d522a752..0faf0b496 100644 --- a/test/e2e/run_apparmor_test.go +++ b/test/e2e/run_apparmor_test.go @@ -106,7 +106,7 @@ profile aa-test-profile flags=(attach_disconnected,mediate_deleted) { parse := SystemExec("apparmor_parser", []string{"-Kr", aaFile}) Expect(parse.ExitCode()).To(Equal(0)) - session := podmanTest.Podman([]string{"create", "--security-opt", "apparmor=aa-test-profile", "ls"}) + session := podmanTest.Podman([]string{"create", "--security-opt", "apparmor=aa-test-profile", ALPINE, "ls"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 0bb3fe772..5c28f18f2 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -1273,4 +1273,46 @@ WORKDIR /madethis` session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) }) + + It("podman run a container with --pull never should fail if no local store", func() { + // Make sure ALPINE image does not exist. Ignore errors + session := podmanTest.PodmanNoCache([]string{"rmi", "--force", "never", ALPINE}) + session.WaitWithDefaultTimeout() + + session = podmanTest.PodmanNoCache([]string{"run", "--pull", "never", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(125)) + }) + + It("podman run container with --pull missing and only pull once", func() { + // Make sure ALPINE image does not exist. Ignore errors + session := podmanTest.PodmanNoCache([]string{"rmi", "--force", "never", ALPINE}) + session.WaitWithDefaultTimeout() + + session = podmanTest.PodmanNoCache([]string{"run", "--pull", "missing", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.ErrorToString()).To(ContainSubstring("Trying to pull")) + + session = podmanTest.PodmanNoCache([]string{"run", "--pull", "missing", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.ErrorToString()).ToNot(ContainSubstring("Trying to pull")) + }) + + It("podman run container with --pull missing should pull image multiple times", func() { + // Make sure ALPINE image does not exist. Ignore errors + session := podmanTest.PodmanNoCache([]string{"rmi", "--force", "never", ALPINE}) + session.WaitWithDefaultTimeout() + + session = podmanTest.PodmanNoCache([]string{"run", "--pull", "always", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.ErrorToString()).To(ContainSubstring("Trying to pull")) + + session = podmanTest.PodmanNoCache([]string{"run", "--pull", "always", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.ErrorToString()).To(ContainSubstring("Trying to pull")) + }) }) -- cgit v1.2.3-54-g00ecf