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.

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

Backports for v3.0 RC2 #9163

Merged
merged 15 commits into from
Jan 29, 2021
Merged

Conversation

mheon
Copy link
Member

@mheon mheon commented Jan 29, 2021

Backports for v3.0.0-rc2

vrothberg and others added 13 commits January 29, 2021 15:22
Make sure to write error from conmon on the hijacked http connection.
This fixes issues where errors were not reported on the client side,
for instance, when specified command was not found on the container.

To future generations: I am sorry.  The code is complex, and there are
many interdependencies among the concurrent goroutines.  I added more
complexity on top but I don't have a good idea of how to reduce
complexity in the available time.

Fixes: containers#8281
Signed-off-by: Valentin Rothberg <[email protected]>
Docker has, for unclear reasons, three separate fields in their
Create Container struct in which volumes can be placed. Right now
we support two of those - Binds and Mounts, which (roughly)
correspond to `-v` and `--mount` respectively. Unfortunately, we
did not support the third, `Volumes`, which is used for anonymous
named volumes created by `-v` (e.g. `-v /test`). It seems that
volumes listed here are *not* included in the remaining two from
my investigation, so it should be safe to just append them into
our handling of the `Binds` (`-v`) field.

Fixes containers#8649

Signed-off-by: Matthew Heon <[email protected]>
when inspecting a container that is only connected to the default
network, we should populate the default network in the container inspect
information.

Fixes: containers#6618

Signed-off-by: baude <[email protected]>

MH: Small fixes, added another test

Signed-off-by: Matthew Heon <[email protected]>
Add an API to libpod to resolve a path on the container.  We can
refactor the code that was originally written for copy.  Other
functions are requiring a proper path resolution, so libpod seems
like a reasonable home for sharing that code.

Signed-off-by: Valentin Rothberg <[email protected]>
A container's workdir can be specified via the CLI via `--workdir` and
via an image config with the CLI having precedence.

Since images have a tendency to specify workdirs without necessarily
shipping the paths with the root FS, make sure that Podman creates the
workdir.  When specified via the CLI, do not create the path, but check
for its existence and return a human-friendly error.

NOTE: `crun` is performing a similar check that would yield exit code
127.  With this change, however, Podman performs the check and yields
exit code 126.  Since this is specific to `crun`, I do not consider it
to be a breaking change of Podman.

Fixes: containers#9040
Signed-off-by: Valentin Rothberg <[email protected]>
There was a potential race where two handlers could be added at
the same time. Go Maps are not thread-safe, so that could do
unpleasant things. Add a mutex to keep things safe.

Also, swap the order or Register and Start for the handlers in
Libpod runtime created. As written, there was a small gap between
Start and Register where SIGTERM/SIGINT would be completely
ignored, instead of stopping Podman. Swapping the two closes this
gap.

Signed-off-by: Matthew Heon <[email protected]>
If the container create command contains an argument with double
curly braces the golang template parsing can fail since it tries
to interpret the value as variable. To fix this change the default
delimiter for the internal template to `{{{{`.

Fixes containers#9034

Signed-off-by: Paul Holzinger <[email protected]>
Set `-mod=vendor` when generating the bindings.  We expect all
dependencies to be vendored already.  This should slightly speed
up the bindings generation and prevent redundant network accesses.

Signed-off-by: Valentin Rothberg <[email protected]>
Run `go generate ./pkg/bindings/...` once for all bindings instead of
generating them separately.  This should speed up bindings generation
as a given package is visited only once, and it fixes containers#8989 by dropping
the use of pushd and popd.

Fixes: containers#8989
Signed-off-by: Valentin Rothberg <[email protected]>
The Go gods did not shine upon us trying to understand what's going on
in containers#9000.  The symptom is that `go generate` did not add required
imports to a generated file, ultimately breaking subsequent compilation.

While it still remains unclear *why* Go is behaving like that, the
symptom disappears when `go generate` runs in module mode; that is
without `-mod=vendor` and without `GO111MODULE=off`.  This was
reproducible on two separate machines (Ubuntu and Fedora).

Also, when facing an unset GOPATH, set it to Go's default (i.e.,
$HOME/go) and make sure that GOBIN is in PATH since `goimports`
is required by `go generate`.

Fixes: containers#9000
Signed-off-by: Valentin Rothberg <[email protected]>
Instead of implicitly generating the bindings, make it explicit, similar
to `make vendor`.  This should prevent redundant and possibly error
prone generations.  A following commit will shield CI.

Signed-off-by: Valentin Rothberg <[email protected]>
Make sure that bindings are in sync with the code.  The check is similar
to what's already being done with `make vendor`, so integrate the two.

[NO TESTS NEEDED]

Signed-off-by: Valentin Rothberg <[email protected]>
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 29, 2021
mheon added 2 commits January 29, 2021 15:37
Signed-off-by: Matthew Heon <[email protected]>
Signed-off-by: Matthew Heon <[email protected]>
@mheon
Copy link
Member Author

mheon commented Jan 29, 2021

RC commit now included

@baude
Copy link
Member

baude commented Jan 29, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 29, 2021
@openshift-merge-robot openshift-merge-robot merged commit 745fa4a into containers:v3.0 Jan 29, 2021
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants