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

[WIP] Final backports and release notes updates for v2.0.5 #7345

Closed
wants to merge 27 commits into from

Conversation

mheon
Copy link
Member

@mheon mheon commented Aug 17, 2020

Latest batch of backports.

This is not going to pass CI - some github.com/containers/podman/v2 paths leaked in. I can't figure out how to use build-each-commit locally to get find them, so I'm going to let upstream CI do it.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 17, 2020
@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 Aug 17, 2020
@mheon
Copy link
Member Author

mheon commented Aug 17, 2020

Alright, it's not worth fighting CI. Renamed github.com/containers/libpod/v2 to github.com/containers/podman/v2.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2020
@mheon
Copy link
Member Author

mheon commented Aug 17, 2020

[+0026s] cmd/podman/containers/stop.go:74:12: cannot use containerStopCommand (variable of type *cobra.Command) as *cobra.Command value in struct literal (typecheck)

Thank you, Golint - that's such a helpful message, that.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2020
Antonio Ojea and others added 17 commits August 17, 2020 17:19
podman containers using IPv6 were missing the default route, breaking
deployments trying to use them.

The problem is that the default route was hardcoded to IPv4, this
takes into consideration the podman subnet IP family to generate
the corresponding default route.

Signed-off-by: Antonio Ojea <[email protected]>
Commit 2b6dd3f set the killmode of the podman.service to the
systemd default which ultimately lead to the problem that systemd
will kill *all* processes inside the unit's cgroup and hence kill
all containers whenever the service is stopped.

Fix it by setting the type to sdnotify and the killmode to process.
`podman system service` will send the necessary notify messages
when the NOTIFY_SOCKET is set and unset it right after to prevent
the backend and container runtimes from jumping in between and send
messages as well.

Fixes: containers#7294
Signed-off-by: Valentin Rothberg <[email protected]>
I'm not sure if this is an OS-specific issue, but on CentOS 8, if `path`
doesn't exist, this hangs while waiting to read from this socket, even
though the socket is closed by the `reexec_in_user_namespace`.  Switching
to a pipe fixes the problem, and pipes shouldn't be an issue since this is
Linux-specific code.

Signed-off-by: Jonathan Dieter <[email protected]>
podman save uses named pipe as output path, not directly using /dev/stdout.
fix containers#7017

Signed-off-by: Qi Wang <[email protected]>

<MH: Corrected imports during cherry-pick>

Signed-off-by: Matt Heon <[email protected]>
I used the wrong propagation first time around because I forgot
that rprivate is the default propagation. Oops. Switch to
rprivate so we're using the default.

Signed-off-by: Matthew Heon <[email protected]>
upon image build completion, a new image type event is written for "build". more intricate details, like pulling an image, that might be done by build must be implemented in different vendored packages only after libpod is split from podman.

Fixes: containers#7022

Signed-off-by: Brent Baude <[email protected]>

<MH: Fixed imports during cherry-pick>

Signed-off-by: Matt Heon <[email protected]>
the deepcopy in the remote history code path was throwing an uncaught error on a type mismatch.  we now manually do the conversion and fix the type mismatch on the fly.

Fixes: containers#7122

Signed-off-by: Brent Baude <[email protected]>
Podman 1.6.2 changed systemd mode auto-detection from commands ending in
``init`` to hard-coded paths ``/sbin/init`` and ``/usr/sbin/init``. This
broke FreeIPA container. ``podman run`` and ``podman create`` now
activate systemd mode when the command is ``/usr/local/sbin/init``.

Fixes: containers#7287
Signed-off-by: Christian Heimes <[email protected]>
Signed-off-by: Christian Heimes <[email protected]>
To sync the behavior between AppArmor and seccomp it is now possible to
also specify seccomp profiles for privileged containers.

Signed-off-by: Sascha Grunert <[email protected]>
A recent crun change stopped the creation of the container's
working directory if it does not exist. This is arguably correct
for user-specified directories, to protect against typos; it is
definitely not correct for image WORKDIR, where the image author
definitely intended for the directory to be used.

This makes Podman create the working directory and chown it to
container root, if it does not already exist, and only if it was
specified by an image, not the user.

Signed-off-by: Matthew Heon <[email protected]>
This matches Docker behavior, and seems to make sense - the CMD
may have been specific to the original entrypoint and probably
does not make sense if it was changed.

While we're in here, greatly simplify the logic for populating
the SpecGen's Command. We create the full command when making the
OCI spec, so the client should not be doing any more than setting
it to the Command the user passed in, and completely ignoring
ENTRYPOINT.

Fixes containers#7115

Signed-off-by: Matthew Heon <[email protected]>
Buildah and podman build can create images without a working dir.

FROM fedora
WORKDIR /test

If you build this image with caching twice, the second time the image
will not have a working dir.

Similarly if you execute

podman run --workdir /foobar fedora

It blows up since the workingdir is not created automatically.

Finally there was duplicated code for getting the workingdir
out of an image, that this PR removes.

Signed-off-by: Daniel J Walsh <[email protected]>
Included old error + wrapped

Signed-off-by: Parker Van Roy <[email protected]>
Refactor the processing of Repository and Tag fields to default to <none>
when printing via --format flag. Previously, the default format would
print <none> but --format {{.Tag}} would not in some cases.

Fixes containers#7123

Signed-off-by: Jhon Honce <[email protected]>
The ListContainers API previously had a Pod parameter, which
determined if pod name was returned (but, notably, not Pod ID,
which was returned unconditionally). This was fairly confusing,
so we decided to deprecate/remove the parameter and return it
unconditionally.

To do this without serious performance implications, we need to
avoid expensive JSON decodes of pod configuration in the DB. The
way our Bolt tables are structured, retrieving name given ID is
actually quite cheap, but we did not expose this via the Libpod
API. Add a new GetName API to do this.

Fixes containers#7214

Signed-off-by: Matthew Heon <[email protected]>
TomSweeneyRedHat and others added 6 commits August 17, 2020 17:19
Addresses the multiple "default" userns values found
in the podman-run(1) man page:  http://docs.podman.io/en/latest/markdown/podman-run.1.html.

This in response to: https://bugzilla.redhat.com/show_bug.cgi?id=1860126
which this PR wil fix.

Signed-off-by: TomSweeneyRedHat <[email protected]>
I was *really* hoping we didn't have to do this, but CI is
utterly broken without it, so what the hell, let's do it.

Signed-off-by: Matthew Heon <[email protected]>
Signed-off-by: Matthew Heon <[email protected]>
When we rewrote Podman's pkg/spec, one of the things that was
lost was our use of a set of default environment variables, that
ensure all containers have at least $PATH and $TERM set.

While we're in the process of re-adding it, change it from a
variable to a function, so we can ensure the Join function does
not overwrite it and corrupt the defaults.

Signed-off-by: Matthew Heon <[email protected]>
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2020
@TomSweeneyRedHat
Copy link
Member

TomSweeneyRedHat commented Aug 17, 2020

Changes LGTM overall, but I'm leery about the libpod to podman change. I think it's OK, but IDK. @vrothberg any concerns in your mind with that?

@mheon
Copy link
Member Author

mheon commented Aug 17, 2020

It looks like I'm missing something necessary to get Cirrus building with the new import paths. Will investigate further tomorrow.

Also, this is still missing the system connection patches, which will need more work to get landed.

@vrothberg
Copy link
Member

Changes LGTM overall, but I'm leery about the libpod to podman change. I think it's OK, but IDK. @vrothberg any concerns in your mind with that?

We just published the bindings blog which is using libpod. If we want to do the renaming, we could do the same in the blog after the release.

@vrothberg
Copy link
Member

I want to get containers/image#1025 in as well to close a BZ against podman-search.

@mheon
Copy link
Member Author

mheon commented Aug 18, 2020 via email

@mheon
Copy link
Member Author

mheon commented Aug 18, 2020

Build each commit is going to take prohibitively long to fix. I see two options:

  • Completely disable it
  • Squash every commit in this PR down to one, so the fixes are all included
    @rhatdan @baude @vrothberg Opinions?

@mheon mheon force-pushed the 205_backports branch 2 times, most recently from b4d29af to f58c22c Compare August 18, 2020 18:04
Signed-off-by: Matthew Heon <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Aug 18, 2020

Disable check just for this PR.

@mheon
Copy link
Member Author

mheon commented Aug 18, 2020

@cevich

[+0146s] go: writing go.mod cache: mkdir /var/tmp/go/pkg: permission denied
[+0146s] go: writing go.mod cache: mkdir /var/tmp/go/pkg: permission denied

Any idea what's going on here?

@cevich
Copy link
Member

cevich commented Aug 19, 2020

ewww, hmmm, that's a new one (to me)...

@cevich
Copy link
Member

cevich commented Aug 19, 2020

...Not sure why we're just seeing this now/here (and not on master), but perhaps chown'ing $GOPATH instead might fix it (it would be owned by root otherwise).

@mheon
Copy link
Member Author

mheon commented Aug 19, 2020

Closing in favor of #7363

@mheon mheon closed this Aug 19, 2020
@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 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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.