Skip to content

Commit

Permalink
Fix podman build --pull-never
Browse files Browse the repository at this point in the history
Currently pull policy is set incorrectly when users set --pull-never.

Also pull-policy is not being translated correctly when using
podman-remote.

Fixes: containers#9573

Signed-off-by: Daniel J Walsh <[email protected]>

<MH: Fixed cherry-pick conflict>

Signed-off-by: Matthew Heon <[email protected]>
  • Loading branch information
rhatdan authored and mheon committed Mar 29, 2021
1 parent be02c85 commit 1089f83
Show file tree
Hide file tree
Showing 24 changed files with 143 additions and 86 deletions.
17 changes: 16 additions & 1 deletion cmd/podman/images/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,21 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
return nil, err
}

pullFlagsCount := 0
if c.Flag("pull").Changed {
pullFlagsCount++
}
if c.Flag("pull-always").Changed {
pullFlagsCount++
}
if c.Flag("pull-never").Changed {
pullFlagsCount++
}

if pullFlagsCount > 1 {
return nil, errors.Errorf("can only set one of 'pull' or 'pull-always' or 'pull-never'")
}

pullPolicy := define.PullIfMissing
if c.Flags().Changed("pull") && flags.Pull {
pullPolicy = define.PullAlways
Expand All @@ -313,7 +328,7 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
}

if flags.PullNever {
pullPolicy = define.PullIfMissing
pullPolicy = define.PullNever
}

args := make(map[string]string)
Expand Down
3 changes: 3 additions & 0 deletions contrib/cirrus/setup_environment.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ do
fi
done

# Make sure cni network plugins directory exists
mkdir -p /etc/cni/net.d

# Ensure that all lower-level contexts and child-processes have
# ready access to higher level orchestration (e.g Cirrus-CI)
# variables.
Expand Down
14 changes: 10 additions & 4 deletions pkg/api/handlers/compat/images_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"time"

"github.com/containers/buildah"
"github.com/containers/buildah/define"
"github.com/containers/buildah/imagebuildah"
"github.com/containers/buildah/util"
"github.com/containers/image/v5/types"
Expand Down Expand Up @@ -98,6 +99,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
OutputFormat string `schema:"outputformat"`
Platform string `schema:"platform"`
Pull bool `schema:"pull"`
PullPolicy string `schema:"pullpolicy"`
Quiet bool `schema:"q"`
Registry string `schema:"registry"`
Rm bool `schema:"rm"`
Expand Down Expand Up @@ -273,10 +275,14 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
jobs = query.Jobs
}

pullPolicy := buildah.PullIfMissing
if _, found := r.URL.Query()["pull"]; found {
if query.Pull {
pullPolicy = buildah.PullAlways
pullPolicy := define.PullIfMissing
if utils.IsLibpodRequest(r) {
pullPolicy = define.PolicyMap[query.PullPolicy]
} else {
if _, found := r.URL.Query()["pull"]; found {
if query.Pull {
pullPolicy = define.PullAlways
}
}
}

Expand Down
7 changes: 3 additions & 4 deletions pkg/bindings/images/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"strconv"
"strings"

"github.com/containers/buildah"
"github.com/containers/podman/v3/pkg/auth"
"github.com/containers/podman/v3/pkg/bindings"
"github.com/containers/podman/v3/pkg/domain/entities"
Expand Down Expand Up @@ -175,9 +174,9 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
if len(platform) > 0 {
params.Set("platform", platform)
}
if options.PullPolicy == buildah.PullAlways {
params.Set("pull", "1")
}

params.Set("pullpolicy", options.PullPolicy.String())

if options.Quiet {
params.Set("q", "1")
}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/build/basicalpine/Containerfile
Original file line number Diff line number Diff line change
@@ -1 +1 @@
FROM alpine
FROM quay.io/libpod/alpine:latest
2 changes: 1 addition & 1 deletion test/e2e/build/basicalpine/Containerfile.path
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
FROM alpine
FROM quay.io/libpod/alpine:latest
ENV PATH=/tmp:/bin:/usr/bin:/usr/sbin
2 changes: 1 addition & 1 deletion test/e2e/build/basicalpine/Containerfile.volume
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
FROM alpine
FROM quay.io/libpod/alpine:latest
VOLUME "/volume0"
2 changes: 1 addition & 1 deletion test/e2e/build/squash/Dockerfile.squash-a
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
FROM busybox:latest
FROM quay.io/libpod/busybox:latest
ADD alpinetest.tgz /data
2 changes: 1 addition & 1 deletion test/e2e/build/squash/Dockerfile.squash-c
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
FROM busybox:latest
FROM quay.io/libpod/busybox:latest
ADD alpinetest.tgz /data
RUN rm -rf /data
35 changes: 18 additions & 17 deletions test/e2e/build_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package integration

import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -150,7 +151,7 @@ var _ = Describe("Podman build", func() {
}

fakeFile := filepath.Join(os.TempDir(), "Containerfile")
Expect(ioutil.WriteFile(fakeFile, []byte("FROM alpine"), 0755)).To(BeNil())
Expect(ioutil.WriteFile(fakeFile, []byte(fmt.Sprintf("FROM %s", ALPINE)), 0755)).To(BeNil())

targetFile := filepath.Join(targetPath, "Containerfile")
Expect(ioutil.WriteFile(targetFile, []byte("FROM scratch"), 0755)).To(BeNil())
Expand Down Expand Up @@ -219,8 +220,8 @@ var _ = Describe("Podman build", func() {
podmanTest.StartRemoteService()
}
podmanTest.AddImageToRWStore(ALPINE)
dockerfile := `FROM quay.io/libpod/alpine:latest
RUN printenv http_proxy`
dockerfile := fmt.Sprintf(`FROM %s
RUN printenv http_proxy`, ALPINE)

dockerfilePath := filepath.Join(podmanTest.TempDir, "Dockerfile")
err := ioutil.WriteFile(dockerfilePath, []byte(dockerfile), 0755)
Expand Down Expand Up @@ -263,9 +264,9 @@ RUN printenv http_proxy`
err = ioutil.WriteFile(dummyFile, []byte("dummy"), 0644)
Expect(err).To(BeNil())

containerfile := `FROM quay.io/libpod/alpine:latest
containerfile := fmt.Sprintf(`FROM %s
ADD . /test
RUN find /test`
RUN find /test`, ALPINE)

containerfilePath := filepath.Join(targetPath, "Containerfile")
err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644)
Expand Down Expand Up @@ -307,7 +308,7 @@ RUN find /test`
err = os.Mkdir(targetSubPath, 0755)
Expect(err).To(BeNil())

containerfile := `FROM quay.io/libpod/alpine:latest`
containerfile := fmt.Sprintf("FROM %s", ALPINE)

containerfilePath := filepath.Join(targetSubPath, "Containerfile")
err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644)
Expand Down Expand Up @@ -344,9 +345,9 @@ RUN find /test`
targetPath, err := CreateTempDirInTempDir()
Expect(err).To(BeNil())

containerfile := `FROM quay.io/libpod/alpine:latest
containerfile := fmt.Sprintf(`FROM %s
ADD . /testfilter/
RUN find /testfilter/`
RUN find /testfilter/`, ALPINE)

containerfilePath := filepath.Join(targetPath, "Containerfile")
err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644)
Expand Down Expand Up @@ -428,10 +429,10 @@ subdir**`
Expect(os.Chdir(targetSubPath)).To(BeNil())
Expect(os.Symlink("dummy", "dummy-symlink")).To(BeNil())

containerfile := `FROM quay.io/libpod/alpine:latest
containerfile := fmt.Sprintf(`FROM %s
ADD . /test
RUN find /test
RUN [[ -L /test/dummy-symlink ]] && echo SYMLNKOK || echo SYMLNKERR`
RUN [[ -L /test/dummy-symlink ]] && echo SYMLNKOK || echo SYMLNKERR`, ALPINE)

containerfilePath := filepath.Join(targetSubPath, "Containerfile")
err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644)
Expand Down Expand Up @@ -475,14 +476,14 @@ RUN grep CapEff /proc/self/status`

// When
session := podmanTest.Podman([]string{
"build", "--pull-never", "--cap-drop=all", "--cap-add=net_bind_service", "--add-host", "testhost:1.2.3.4", "--from", "alpine", targetPath,
"build", "--pull-never", "--cap-drop=all", "--cap-add=net_bind_service", "--add-host", "testhost:1.2.3.4", "--from", ALPINE, targetPath,
})
session.WaitWithDefaultTimeout()

// Then
Expect(session.ExitCode()).To(Equal(0))
Expect(strings.Fields(session.OutputToString())).
To(ContainElement("alpine"))
To(ContainElement(ALPINE))
Expect(strings.Fields(session.OutputToString())).
To(ContainElement("testhost"))
Expect(strings.Fields(session.OutputToString())).
Expand All @@ -494,23 +495,23 @@ RUN grep CapEff /proc/self/status`
Expect(err).To(BeNil())

containerFile := filepath.Join(targetPath, "Containerfile")
Expect(ioutil.WriteFile(containerFile, []byte("FROM alpine"), 0755)).To(BeNil())
Expect(ioutil.WriteFile(containerFile, []byte(fmt.Sprintf("FROM %s", ALPINE)), 0755)).To(BeNil())

defer func() {
Expect(os.RemoveAll(containerFile)).To(BeNil())
}()

// When
session := podmanTest.Podman([]string{
"build", "--pull-never", "--isolation", "oci", "--arch", "arm64", targetPath,
"build", "--isolation", "oci", "--arch", "arm64", targetPath,
})
session.WaitWithDefaultTimeout()
// Then
Expect(session.ExitCode()).To(Equal(0))

// When
session = podmanTest.Podman([]string{
"build", "--pull-never", "--isolation", "chroot", "--arch", "arm64", targetPath,
"build", "--isolation", "chroot", "--arch", "arm64", targetPath,
})
session.WaitWithDefaultTimeout()
// Then
Expand All @@ -534,8 +535,8 @@ RUN grep CapEff /proc/self/status`
})

It("podman build --timestamp flag", func() {
containerfile := `FROM quay.io/libpod/alpine:latest
RUN echo hello`
containerfile := fmt.Sprintf(`FROM %s
RUN echo hello`, ALPINE)

containerfilePath := filepath.Join(podmanTest.TempDir, "Containerfile")
err := ioutil.WriteFile(containerfilePath, []byte(containerfile), 0755)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/containers_conf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ var _ = Describe("Podman run", func() {
if IsRemote() {
podmanTest.RestartRemoteService()
}
session := podmanTest.Podman([]string{"run", "busybox", "grep", "CapEff", "/proc/self/status"})
session := podmanTest.Podman([]string{"run", BB, "grep", "CapEff", "/proc/self/status"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(session.OutputToString()).ToNot(Equal(cap.OutputToString()))
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,10 +475,10 @@ var _ = Describe("Podman exec", func() {
})

It("podman exec preserves container groups with --user and --group-add", func() {
dockerfile := `FROM registry.fedoraproject.org/fedora-minimal
dockerfile := fmt.Sprintf(`FROM %s
RUN groupadd -g 4000 first
RUN groupadd -g 4001 second
RUN useradd -u 1000 auser`
RUN useradd -u 1000 auser`, fedoraMinimal)
imgName := "testimg"
podmanTest.BuildImage(dockerfile, imgName, "false")

Expand Down
7 changes: 4 additions & 3 deletions test/e2e/prune_test.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
package integration

import (
"fmt"
"os"

. "github.com/containers/podman/v3/test/utils"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

var pruneImage = `
FROM alpine:latest
var pruneImage = fmt.Sprintf(`
FROM %s
LABEL RUN podman --version
RUN apk update
RUN apk add bash`
RUN apk add bash`, ALPINE)

var _ = Describe("Podman prune", func() {
var (
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/ps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ var _ = Describe("Podman ps", func() {
})

It("podman --format by size", func() {
session := podmanTest.Podman([]string{"create", "busybox", "ls"})
session := podmanTest.Podman([]string{"create", BB, "ls"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))

Expand All @@ -366,7 +366,7 @@ var _ = Describe("Podman ps", func() {
})

It("podman --sort by size", func() {
session := podmanTest.Podman([]string{"create", "busybox", "ls"})
session := podmanTest.Podman([]string{"create", BB, "ls"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))

Expand Down
19 changes: 11 additions & 8 deletions test/e2e/rmi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,19 +184,20 @@ var _ = Describe("Podman rmi", func() {

It("podman rmi with cached images", func() {
podmanTest.AddImageToRWStore(cirros)
dockerfile := `FROM quay.io/libpod/cirros:latest
dockerfile := fmt.Sprintf(`FROM %s
RUN mkdir hello
RUN touch test.txt
ENV foo=bar
`
`, cirros)
podmanTest.BuildImage(dockerfile, "test", "true")

dockerfile = `FROM quay.io/libpod/cirros:latest
dockerfile = fmt.Sprintf(`FROM %s
RUN mkdir hello
RUN touch test.txt
RUN mkdir blah
ENV foo=bar
`
`, cirros)

podmanTest.BuildImage(dockerfile, "test2", "true")

session := podmanTest.Podman([]string{"images", "-q", "-a"})
Expand Down Expand Up @@ -249,14 +250,15 @@ var _ = Describe("Podman rmi", func() {
})

It("podman rmi -a with parent|child images", func() {
dockerfile := `FROM quay.io/libpod/cirros:latest AS base
podmanTest.AddImageToRWStore(cirros)
dockerfile := fmt.Sprintf(`FROM %s AS base
RUN touch /1
ENV LOCAL=/1
RUN find $LOCAL
FROM base
RUN find $LOCAL
`
`, cirros)
podmanTest.BuildImage(dockerfile, "test", "true")
session := podmanTest.Podman([]string{"rmi", "-a"})
session.WaitWithDefaultTimeout()
Expand Down Expand Up @@ -284,14 +286,15 @@ RUN find $LOCAL
// a race, we may not hit the condition a 100 percent of times
// but ocal reproducers hit it all the time.

podmanTest.AddImageToRWStore(cirros)
var wg sync.WaitGroup

buildAndRemove := func(i int) {
defer GinkgoRecover()
defer wg.Done()
imageName := fmt.Sprintf("rmtest:%d", i)
containerfile := `FROM quay.io/libpod/cirros:latest
RUN ` + fmt.Sprintf("touch %s", imageName)
containerfile := fmt.Sprintf(`FROM %s
RUN touch %s`, cirros, imageName)

podmanTest.BuildImage(containerfile, imageName, "false")
session := podmanTest.Podman([]string{"rmi", "-f", imageName})
Expand Down
Loading

0 comments on commit 1089f83

Please sign in to comment.