Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing params for podman-remote build #9275

Merged
merged 1 commit into from
Feb 22, 2021
+152 −25
Merged

Add missing params for podman-remote build #9275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 71 additions & 21 deletions pkg/api/handlers/compat/images_build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,19 @@ import (
"os"
"path/filepath"
"strconv"
"time"

"github.com/containers/buildah"
"github.com/containers/buildah/imagebuildah"
"github.com/containers/buildah/util"
"github.com/containers/image/v5/types"
"github.com/containers/podman/v2/libpod"
"github.com/containers/podman/v2/pkg/api/handlers/utils"
"github.com/containers/podman/v2/pkg/auth"
"github.com/containers/podman/v2/pkg/channel"
"github.com/containers/storage/pkg/archive"
"github.com/gorilla/schema"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -65,6 +68,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
Annotations string `schema:"annotations"`
BuildArgs string `schema:"buildargs"`
CacheFrom string `schema:"cachefrom"`
Compression uint64 `schema:"compression"`
ConfigureNetwork int64 `schema:"networkmode"`
CpuPeriod uint64 `schema:"cpuperiod"` // nolint
CpuQuota int64 `schema:"cpuquota"` // nolint
Expand All @@ -73,17 +77,20 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
Devices string `schema:"devices"`
Dockerfile string `schema:"dockerfile"`
DropCapabilities string `schema:"dropcaps"`
Excludes string `schema:"excludes"`
ForceRm bool `schema:"forcerm"`
From string `schema:"from"`
HTTPProxy bool `schema:"httpproxy"`
Isolation int64 `schema:"isolation"`
Jobs uint64 `schema:"jobs"` // nolint
Ignore bool `schema:"ignore"`
Jobs int `schema:"jobs"` // nolint
Labels string `schema:"labels"`
Layers bool `schema:"layers"`
LogRusage bool `schema:"rusage"`
Manifest string `schema:"manifest"`
MemSwap int64 `schema:"memswap"`
Memory int64 `schema:"memory"`
NamespaceOptions string `schema:"nsoptions"`
NoCache bool `schema:"nocache"`
OutputFormat string `schema:"outputformat"`
Platform string `schema:"platform"`
Expand Down Expand Up @@ -129,6 +136,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
}
}

compression := archive.Compression(query.Compression)
// convert label formats
var dropCaps = []string{}
if _, found := r.URL.Query()["dropcaps"]; found {
Expand Down Expand Up @@ -156,7 +164,15 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
output = query.Tag[0]
}
format := buildah.Dockerv2ImageManifest
registry := query.Registry
isolation := buildah.IsolationChroot
/*
// FIXME, This is very broken. Buildah will only work with chroot
isolation := buildah.IsolationDefault
*/
if utils.IsLibpodRequest(r) {
// isolation = buildah.Isolation(query.Isolation)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we drop this commented code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets merge this. I waiting to open a PR to fix this as soon as this PR merges.

registry = ""
format = query.OutputFormat
}
var additionalTags []string
Expand All @@ -172,6 +188,14 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
}
}

var excludes = []string{}
if _, found := r.URL.Query()["excludes"]; found {
if err := json.Unmarshal([]byte(query.Excludes), &excludes); err != nil {
utils.BadRequest(w, "excludes", query.Excludes, err)
return
}
}

// convert label formats
var annotations = []string{}
if _, found := r.URL.Query()["annotations"]; found {
Expand All @@ -181,6 +205,19 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
}
}

// convert label formats
nsoptions := buildah.NamespaceOptions{}
if _, found := r.URL.Query()["nsoptions"]; found {
if err := json.Unmarshal([]byte(query.NamespaceOptions), &nsoptions); err != nil {
utils.BadRequest(w, "nsoptions", query.NamespaceOptions, err)
return
}
} else {
nsoptions = append(nsoptions, buildah.NamespaceOption{
Name: string(specs.NetworkNamespace),
Host: true,
})
}
// convert label formats
var labels = []string{}
if _, found := r.URL.Query()["labels"]; found {
Expand All @@ -189,6 +226,10 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
return
}
}
jobs := 1
if _, found := r.URL.Query()["jobs"]; found {
jobs = query.Jobs
}

pullPolicy := buildah.PullIfMissing
if _, found := r.URL.Query()["pull"]; found {
Expand Down Expand Up @@ -218,6 +259,12 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
reporter := channel.NewWriter(make(chan []byte, 1))
defer reporter.Close()

runtime := r.Context().Value("runtime").(*libpod.Runtime)
rtc, err := runtime.GetConfig()
if err != nil {
utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()"))
return
}
buildOptions := imagebuildah.BuildOptions{
AddCapabilities: addCaps,
AdditionalTags: additionalTags,
Expand All @@ -234,40 +281,43 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
MemorySwap: query.MemSwap,
ShmSize: strconv.Itoa(query.ShmSize),
},
Compression: archive.Gzip,
CNIConfigDir: rtc.Network.CNIPluginDirs[0],
CNIPluginPath: util.DefaultCNIPluginPath,
Compression: compression,
ConfigureNetwork: buildah.NetworkConfigurationPolicy(query.ConfigureNetwork),
ContextDirectory: contextDirectory,
Devices: devices,
DropCapabilities: dropCaps,
Err: auxout,
Excludes: excludes,
ForceRmIntermediateCtrs: query.ForceRm,
From: query.From,
IgnoreUnrecognizedInstructions: true,
// FIXME, This is very broken. Buildah will only work with chroot
// Isolation: buildah.Isolation(query.Isolation),
Isolation: buildah.IsolationChroot,

Labels: labels,
Layers: query.Layers,
Manifest: query.Manifest,
NoCache: query.NoCache,
Out: stdout,
Output: output,
OutputFormat: format,
PullPolicy: pullPolicy,
Quiet: query.Quiet,
Registry: query.Registry,
RemoveIntermediateCtrs: query.Rm,
ReportWriter: reporter,
Squash: query.Squash,
IgnoreUnrecognizedInstructions: query.Ignore,
Isolation: isolation,
Jobs: &jobs,
Labels: labels,
Layers: query.Layers,
Manifest: query.Manifest,
MaxPullPushRetries: 3,
NamespaceOptions: nsoptions,
NoCache: query.NoCache,
Out: stdout,
Output: output,
OutputFormat: format,
PullPolicy: pullPolicy,
PullPushRetryDelay: time.Duration(2 * time.Second),
Quiet: query.Quiet,
Registry: registry,
RemoveIntermediateCtrs: query.Rm,
ReportWriter: reporter,
Squash: query.Squash,
SystemContext: &types.SystemContext{
AuthFilePath: authfile,
DockerAuthConfig: creds,
},
Target: query.Target,
}

runtime := r.Context().Value("runtime").(*libpod.Runtime)
runCtx, cancel := context.WithCancel(context.Background())
var imageID string
go func() {
Expand Down
18 changes: 17 additions & 1 deletion pkg/bindings/images/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
}
params.Set("buildargs", bArgs)
}
if excludes := options.Excludes; len(excludes) > 0 {
bArgs, err := jsoniter.MarshalToString(excludes)
if err != nil {
return nil, err
}
params.Set("excludes", bArgs)
}
if cpuShares := options.CommonBuildOpts.CPUShares; cpuShares > 0 {
params.Set("cpushares", strconv.Itoa(int(cpuShares)))
}
Expand Down Expand Up @@ -94,7 +101,9 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
if len(options.From) > 0 {
params.Set("from", options.From)
}

if options.IgnoreUnrecognizedInstructions {
params.Set("ignore", "1")
}
params.Set("isolation", strconv.Itoa(int(options.Isolation)))
if options.CommonBuildOpts.HTTPProxy {
params.Set("httpproxy", "1")
Expand Down Expand Up @@ -159,6 +168,13 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
}
params.Set("extrahosts", h)
}
if nsoptions := options.NamespaceOptions; len(nsoptions) > 0 {
ns, err := jsoniter.MarshalToString(nsoptions)
if err != nil {
return nil, err
}
params.Set("nsoptions", ns)
}
if shmSize := options.CommonBuildOpts.ShmSize; len(shmSize) > 0 {
shmBytes, err := units.RAMInBytes(shmSize)
if err != nil {
Expand Down
27 changes: 25 additions & 2 deletions test/e2e/build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ RUN grep CapEff /proc/self/status`
To(ContainElement("0000000000000400"))
})

It("podman build --arch", func() {
It("podman build --isolation && --arch", func() {
targetPath, err := CreateTempDirInTempDir()
Expect(err).To(BeNil())

Expand All @@ -502,11 +502,34 @@ RUN grep CapEff /proc/self/status`

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

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

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

// When
session = podmanTest.Podman([]string{
"build", "--isolation", "bogus", "--arch", "arm64", targetPath,
})
session.WaitWithDefaultTimeout()
// Then
Expect(session.ExitCode()).To(Equal(125))
})
})
1 change: 0 additions & 1 deletion test/e2e/prune_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ var _ = Describe("Podman prune", func() {
It("podman image prune dangling images", func() {
podmanTest.BuildImage(pruneImage, "alpine_bash:latest", "true")
podmanTest.BuildImage(pruneImage, "alpine_bash:latest", "true")

none := podmanTest.Podman([]string{"images", "-a"})
none.WaitWithDefaultTimeout()
Expect(none.ExitCode()).To(Equal(0))
Expand Down
39 changes: 39 additions & 0 deletions test/system/070-build.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,45 @@ EOF
run_podman rmi -a --force
}

@test "build with copy-from referencing the base image" {
skip_if_rootless "cannot mount as rootless"
target=busybox-derived
target_mt=busybox-mt-derived
tmpdir=$PODMAN_TMPDIR/build-test
mkdir -p $tmpdir
containerfile1=$tmpdir/Containerfile1
cat >$containerfile1 <<EOF
FROM quay.io/libpod/busybox AS build
RUN rm -f /bin/paste
USER 1001
COPY --from=quay.io/libpod/busybox /bin/paste /test/
EOF
containerfile2=$tmpdir/Containerfile2
cat >$containerfile2 <<EOF
FROM quay.io/libpod/busybox AS test
RUN rm -f /bin/nl
FROM quay.io/libpod/alpine AS final
COPY --from=quay.io/libpod/busybox /bin/nl /test/
EOF
run_podman build -t ${target} -f ${containerfile1} ${tmpdir}
run_podman build --jobs 4 -t ${target} -f ${containerfile1} ${tmpdir}

run_podman build -t ${target} -f ${containerfile2} ${tmpdir}
run_podman build --no-cache --jobs 4 -t ${target_mt} -f ${containerfile2} ${tmpdir}

# (can only test locally; podman-remote has no image mount command)
if ! is_remote; then
run_podman image mount ${target}
root_single_job=$output

run_podman image mount ${target_mt}
root_multi_job=$output

# Check that both the version with --jobs 1 and --jobs=N have the same number of files
test $(find $root_single_job -type f | wc -l) = $(find $root_multi_job -type f | wc -l)
fi
}

Comment on lines +456 to +494
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhatdan @nalind I realize this is already merged, but I'm afraid this test is almost entirely a NOP. I've spent the last hour-plus trying to clean it up, and am forced to give up because I don't truly understand its purpose. Is it to test the --jobs option? To test COPY --from? I'm going to need to rewrite the whole thing, and I would be grateful for any hints on how to do so.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The basic idea, I believe since @nalind wrote it for Buildah, was to run the same build twice once with multiple --jobs and make sure that the output in the end was the same.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! In that case, are lines 476-477 (the first two builds) needed? Or for that matter containerfile1?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nalind WDYT?

Rereading the tests, I stole this from, I am now not sure what it is testing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test was originally to ensure that when COPY --from was given an image name rather than a name we'd given to a build stage, that it would pull content from the unmodified image rather than a stage that was based on it, which was a bug that we'd fixed.
The parts that ran builds both with and without --jobs and counted the files in containers based on the resulting images, to check that the results from both were comparable, were mixed in later rather than added as a separate test.

@test "podman build --logfile test" {
tmpdir=$PODMAN_TMPDIR/build-test
mkdir -p $tmpdir
Expand Down