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 v2.2.1 #8640

Merged
merged 24 commits into from
Dec 7, 2020
Merged

Conversation

mheon
Copy link
Member

@mheon mheon commented Dec 7, 2020

These are the bugfix backports for Podman v2.2.1

mheon and others added 23 commits December 7, 2020 14:27
Podman pre-1.8 also included a field with this name, which was a
String. Podman 2.2.0 added a new field reusing the name but as a
Struct. This completely broke JSON decode for pre-1.8 containers
in Podman 2.2, resulting in completely broken behavior.

Re-name the JSON field and add a note that the old name should
not be re-used to prevent this problem from re-occurring. This
will still result in containers from 2.2.0 being broken
(specifically, containers with image volumes will have them
disappear) but this is the lesser of two evils.

Fixes containers#8613

Signed-off-by: Matthew Heon <[email protected]>
Before querying for a container's cgroup path, make sure that the
container is synced.  Also make sure to error out if the container
isn't running.

Signed-off-by: Valentin Rothberg <[email protected]>
Installing a duplicate shutdown handler fails, but if a handler
with the same name is already present, we should be set to go.
There's no reason to print a user-facing error about it.

This comes up almost nowhere because Podman never makes more than
one Libpod runtime, but there is one exception (`system reset`)
and the error messages, while harmless, were making people very
confused (we got several bug reports that `system reset` was
nonfunctional).

Signed-off-by: Matthew Heon <[email protected]>
We can't mount sysfs as rootless unless we manage the network
namespace. Problem: slirp4netns is now creating and managing a
network namespace separate from the OCI runtime, so we can't
mount sysfs in many circumstances. The `crun` OCI runtime will
automatically handle this by falling back to a bind mount, but
`runc` will not, so we didn't notice until RHEL gating tests ran
on the new branch.

Signed-off-by: Matthew Heon <[email protected]>
/containers/create compat endpoint does not set the name correctly (containers#7857)

Signed-off-by: Milivoje Legenovic <[email protected]>
When creating a container, do not clear the input-image name before
looking up image names.  Also add a regression test.

Fixes: containers#8558
Signed-off-by: Valentin Rothberg <[email protected]>
<MH: Fixed cherry-pick conflicts>
Signed-off-by: Matthew Heon <[email protected]>
Previously close rawSouce in the middle makes future use of rawSource invalid.
Move the rawSource.Close() to the end of each loop.

Signed-off-by: Qi Wang <[email protected]>
Currently asking for login password, even if not supported by
the ssh server. So wait with prompt until actually requested.

Signed-off-by: Anders F Björklund <[email protected]>
Docker defines an option of "default" which means to
use the default network.  We should support this with
the same code path as --network="".

This is important for compatibility with the Docker API.

Fixes: containers#8544

Signed-off-by: Daniel J Walsh <[email protected]>
move the conmon process to the conmon cgroup also on exec.

The previous implementation would fail to move the conmon process as
the systemd unit already exists so its creation would fail.

When the unit cannot be created, attempt to directly join the cgroup
instead.

Signed-off-by: Giuseppe Scrivano <[email protected]>
* existing code caused an unnecessary 301 redirect

Signed-off-by: Jhon Honce <[email protected]>
Previously, we always computed pause path from the Rootless
runtime directory. Problem: this does not match the behavior of
Libpod when the directory changes. Libpod will continue to use
the previous directory, cached in the database; Pause pidfiles
will swap to the new path. This is problematic when the directory
needs to exist to write the pidfile, and Libpod is what creates
the directory.

There are two potential solutions - allow the pause pidfile to
move and just make the directory when we want to write it, or use
the cached Libpod paths for a guaranteed location. This patch
does the second, because it seems safer - we will never miss a
previously-existing pidfile because the location is now
consistent.

Fixes containers#8539

Signed-off-by: Matthew Heon <[email protected]>
when formatting mount options into a string for the compat container create, the options need to be comma delimited.

Signed-off-by: baude <[email protected]>
remove mistaken use of target being used for tag

Signed-off-by: baude <[email protected]>
The `ancestor` option was missing an equal sign. Therefore
the completion did not work as expected.

Signed-off-by: Paul Holzinger <[email protected]>
Instead of being interpreted as an argument to the boolean flag,
the 'true' is being intepreted as the Podman command to be run -
so we're trying to run `podman true`, which does not exist. This
causes the cleanup command to fail when `--log-level=debug` is
set, so containers are not cleaned up or removed.

This problem is easily reproduced with any command combining the
`--rm`, `-d`, and `--log-level=debug` flags - the command will
execute and exit, but the container will not be removed.

Separate, but worth looking into later: the errors we get on
trying `podman true` with any flags are terrible - if you just
type `podman true` you get a quite sane "Unrecognized command"
error, but if you try `podman true --rm` you get an "unknown flag
--rm" error - which makes very little sense given the command
itself doesn't exist.

Signed-off-by: Matthew Heon <[email protected]>
Signed-off-by: Daniel J Walsh <[email protected]>
Vendor in the latest cobra release v1.1.1

This will hurt the completion experience but is required for
proper packaging, see: containers#8528.

The best solution is to keep the current scripts since they
work fine with cobra v1.1.1.

Signed-off-by: Paul Holzinger <[email protected]>
the volumes provided is seemingly useless representing what volumes
should be added to a container. instead, the host config bindings should
be used as they acurately describe the src/dest and options for
bindings.

Signed-off-by: baude <[email protected]>
Signed-off-by: zhangguanzhang <[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 Dec 7, 2020
@mheon
Copy link
Member Author

mheon commented Dec 7, 2020

@containers/podman-maintainers PTAL

RELEASE_NOTES.md Outdated
@@ -1,5 +1,31 @@
# Release Notes

## 2.2.1
### Changes
- Due to a conflict with a previously-removed field, we were forced to modify the way image volumes (mounting images into containers using `--mount type=image`) were handled in the database. As a result, containers created in Podman 2.2.0 with image volumes will not have them in v2.2.1, and will need to be re-created.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Due to a conflict with a previously-removed field, we were forced to modify the way image volumes (mounting images into containers using `--mount type=image`) were handled in the database. As a result, containers created in Podman 2.2.0 with image volumes will not have them in v2.2.1, and will need to be re-created.
- Due to a conflict with a previously-removed field, we were forced to modify the way image volumes (mounting images into containers using `--mount type=image`) were handled in the database. As a result, containers created in Podman 2.2.0 with image volumes will not have them in v2.2.1, and the containers will need to be re-created.

RELEASE_NOTES.md Outdated
- Fixed a bug where the Compat Create endpoint for Containers did not properly handle the `Binds` and `Mounts` parameters in `HostConfig`.
- Fixed a bug where the Compat Create endpoint for Containers ignored the `Name` query parameter.
- Fixed a bug where the Compat Create endpoint for Containers did not properly handle the "default" value for `NetworkMode` (this value is used extensively by `docker-compose`) ([#8544](https://github.com/containers/podman/issues/8544)).
- Fixed a bug where the Compat Build endpoint for Images would sometimes use the `target` query parameter as the image's tag incorrectly.
Copy link
Member

Choose a reason for hiding this comment

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

soft suggestion

Suggested change
- Fixed a bug where the Compat Build endpoint for Images would sometimes use the `target` query parameter as the image's tag incorrectly.
- Fixed a bug where the Compat Build endpoint for Images would sometimes incorrectly use the `target` query parameter as the image's tag.

@rhatdan
Copy link
Member

rhatdan commented Dec 7, 2020

LGTM
Just fix release notes.

@mheon
Copy link
Member Author

mheon commented Dec 7, 2020

Release notes are fixed, looks ready to go green.

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@baude
Copy link
Member

baude commented Dec 7, 2020

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 7, 2020
@baude
Copy link
Member

baude commented Dec 7, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2020
@mheon
Copy link
Member Author

mheon commented Dec 7, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 7, 2020
@openshift-merge-robot openshift-merge-robot merged commit 23d2dee into containers:v2.2 Dec 7, 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. 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.