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

Bump to v3.0.1 #9427

Merged
merged 38 commits into from
Feb 19, 2021
Merged

Bump to v3.0.1 #9427

merged 38 commits into from
Feb 19, 2021

Conversation

mheon
Copy link
Member

@mheon mheon commented Feb 18, 2021

Backports and release bump for Podman v3.0.1.

Changelog is going to be VERY large because I pulled in a lot of lint and bindings changes. These aren't strictly necessary but will greatly help with backports in the future by keeping this code as similar as possible to upstream.

mheon and others added 30 commits February 18, 2021 10:14
This one is rather bizarre because it triggers only on some
systems. I've included a CI test, for example, but I'm 99% sure
we use images in CI that have volumes over empty directories, and
the earlier patch to change copy-up implementation passed CI
without complaint.

I can reproduce this on a stock F33 VM, but that's the only place
I have been able to see it.

Regardless, the issue: under certain as-yet-unidentified
environmental conditions, the copier.Get method will return an
ENOENT attempting to stream a directory that is empty. Work
around this by avoiding the copy altogether in this case.

Signed-off-by: Matthew Heon <[email protected]>
Instead of using the container's mountpoint as the base of the
chroot and indexing from there by the volume directory, instead
use the full path of what we want to copy as the base of the
chroot and copy everything in it. This resolves the bug, ends up
being a bit simpler code-wise (no string concatenation, as we
already have the full path calculated for other checks), and
seems more understandable than trying to resolve things on the
destination side of the copy-up.

Fixes containers#9354

Signed-off-by: Matthew Heon <[email protected]>
Podman -s crashes when the user specifies the '{{ .Size }}` format
on the podman ps command, without specifying the --size option.

This PR will stop the crash and print out a logrus.Error stating that
the caller should add the --size option.

Fixes: containers#9408

Signed-off-by: Daniel J Walsh <[email protected]>
We recieved an issue with an image that was built with
entrypoint=[""]
This blows up on Podman, but works on Docker.

When we setup the OCI Runtime, we should drop
entrypoint if it is == [""]

containers#9377

Signed-off-by: Daniel J Walsh <[email protected]>
if the current process could not be moved to a different systemd
cgroup do not raise a warning but debug message.

[NO TESTS NEEDED]
Closes: containers#9353

Signed-off-by: Giuseppe Scrivano <[email protected]>
Make sure to not set an empty $HOME for containers and let it default to
"/".

containers/crun#599 is required to fully
address containers#9378.

Partially-Fixes: containers#9378
Signed-off-by: Valentin Rothberg <[email protected]>

<MH: Fixed cherry-pick conflicts>

Signed-off-by: Matthew Heon <[email protected]>
when creating a pod with --infra-image and using a untagged image for
the infra-image (none/none), the lookup for the image's name was
creating a panic.

Fixes: containers#9374

Signed-off-by: baude <[email protected]>
The `images/create` endpoint should always attempt to pull a newer
image.  Previously, the local images was used which is not compatible
with Docker and caused issues in the Gitlab CI.

Fixes: containers#9232
Signed-off-by: Valentin Rothberg <[email protected]>
Make sure that Podman's default OCI runtime is passed to Buildah in
`podman build`.  In theory, Podman and Buildah should use the same
defaults but the projects move at different speeds and it turns out
we caused a regression in v3.0.

Fixes: containers#9365
Signed-off-by: Valentin Rothberg <[email protected]>
The error message when failing to create an image engine unconditionally
pointed to the Podman socket which is quite confusing when running
locally.

Move the error message to the point where the first ping to the service
fails.

[NO TESTS NEEDED]

Signed-off-by: Valentin Rothberg <[email protected]>
Currently podman is always chowning the WORKDIR to root:root
This PR will return if the WORKDIR already exists.

Fixes: containers#9387

Signed-off-by: Daniel J Walsh <[email protected]>
The unit generation accidentally escaped the %t in the pod id file path.
This is a regression caused by containers#9178. This was not caught by the tests
because the test itself was wrong. It used a full path instead of the
systemd variable %t like the actual code does.

Fixes containers#9373

Signed-off-by: Paul Holzinger <[email protected]>
The timestamps of some images must have changed changing the number of
expected filtered images.  The test conditions seem fragile but for now
it's more important to get CI back.

Signed-off-by: Valentin Rothberg <[email protected]>
The logic in the e2e test for multiple network aliases is indicating the
test should wait for the containerized nginx to be ready.  As this may
take some time, the test does an exponential backoff starting at 2050ms.

Fix the logic by removing the `Expect(...)` call during the exponential
backoff.  Otherwise, the test errors immediately.

Signed-off-by: Valentin Rothberg <[email protected]>
ubuntu's dns seems a little odd and requires a fq name in its tests.

Signed-off-by: baude <[email protected]>
When the query decoding fails at the beginning of WaitContainerLibpod(),
the Error() sets the header but doesn't returns after that.

This causes the execution flow to reach the WriteResponse() at the end
of WaitContainerLibpod(), which attempts to set another header, thus
causing the following error:

http: superfluous response.WriteHeader call from github.com/containers/podman/pkg/api/handlers/utils.WriteResponse (handler.go:124)

[NO TESTS NEEDED]

Signed-off-by: Nikolay Edigaryev <[email protected]>
When docker-java calls images/create?fromImage=x, it expects two things
for a successful response: that both "error" and "errorDetail" are not
set, and that the "progress" message contains one of five hard-coded
strings ("Download complete" being one of them).

Signed-off-by: Igor Korolev <[email protected]>
Use the stylecheck linter and fix the reported problems.

[NO TESTS NEEDED]

Signed-off-by: Paul Holzinger <[email protected]>

<MH: Fix cherry-pick conflict>

Signed-off-by: Matthew Heon <[email protected]>
Use the golint linter and fix the reported problems.

[NO TESTS NEEDED]

Signed-off-by: Paul Holzinger <[email protected]>
Use the whitespace linter and fix the reported problems.

[NO TESTS NEEDED]

Signed-off-by: Paul Holzinger <[email protected]>

<MH: Fixed up cherry-pick conflicts>

Signed-off-by: Matthew Heon <[email protected]>
Cleanup the golangci.yml file and enable more linters.

`pkg/spec` and `iopodman.io` is history. The vendor directory
is excluded by default. The dependencies dir was listed twice.

Fix the reported problems in `pkg/specgen` because that was also
excluded by `pkg/spec`.

Enable the structcheck, typecheck, varcheck, deadcode and depguard
linters.

[NO TESTS NEEDED]

Signed-off-by: Paul Holzinger <[email protected]>
Extracted common functionality to util function.

Signed-off-by: Matej Vasek <[email protected]>

<MH: Fixed cherry-pick conflicts>
[NO TESTS NEEDED]

Signed-off-by: Riyad Preukschas <[email protected]>

<MH: Fixed cherry-pick conflicts>

Signed-off-by: Matthew Heon <[email protected]>
Currently if the host shares container storage with a container
running podman, the podman inside of the container resets the
storage on the host. This can cause issues on the host, as
well as causes the podman command running the container, to
fail to unmount /dev/shm.

podman run -ti --rm --privileged -v /var/lib/containers:/var/lib/containers quay.io/podman/stable podman run alpine echo hello
	* unlinkat /var/lib/containers/storage/overlay-containers/a7f3c9deb0656f8de1d107e7ddff2d3c3c279c11c1635f233a0bffb16051fb2c/userdata/shm: device or resource busy
	* unlinkat /var/lib/containers/storage/overlay-containers/a7f3c9deb0656f8de1d107e7ddff2d3c3c279c11c1635f233a0bffb16051fb2c/userdata/shm: device or resource busy

Since podman is volume mounting in the graphroot, it will add a flag to
/run/.containerenv to tell podman inside of container whether to reset storage or not.

Since the inner podman is running inside of the container, no reason to assume this is a fresh reboot, so if "container" environment variable is set then skip
reset of storage.

Also added tests to make sure /run/.containerenv is runnig correctly.

Fixes: containers#9191

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

<MH: Fixed cherry-pick conflicts>

Signed-off-by: Matthew Heon <[email protected]>
Signed-off-by: Nikolay Edigaryev <[email protected]>
Signed-off-by: Nikolay Edigaryev <[email protected]>
@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 Feb 18, 2021
@baude
Copy link
Member

baude commented Feb 18, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2021
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM
That's a big one. We weren't lazy, that's for sure.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2021
@mheon mheon mentioned this pull request Feb 18, 2021
@rhatdan
Copy link
Member

rhatdan commented Feb 18, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2021
mheon and others added 6 commits February 18, 2021 14:23
The InspectVolumeData struct was moved into Define from Libpod.

Signed-off-by: Matthew Heon <[email protected]>
Buildah bud passes a bunch more flags then podman build.

We need to implement hook up all of these flags to get full functionality.

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

<MH: Fix cherry pick conflicts>

Signed-off-by: Matthew Heon <[email protected]>
Fix handling of --iidfile to happen on the client side.

Signed-off-by: Daniel J Walsh <[email protected]>
Signed-off-by: Matthew Heon <[email protected]>
Signed-off-by: Matthew Heon <[email protected]>
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2021
@mheon
Copy link
Member Author

mheon commented Feb 18, 2021

Re-pushed again with a few more podman build fixes that should hopefully make CI happy.

@baude
Copy link
Member

baude commented Feb 18, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2021
@TomSweeneyRedHat
Copy link
Member

@mheon f32 root is failing. Not a lot of error message left behind, perhaps the GCP issue that @cevich gave us a heads up on?

@mheon
Copy link
Member Author

mheon commented Feb 18, 2021

Um. Total success doesn't seem to be running?

@mheon
Copy link
Member Author

mheon commented Feb 18, 2021

Ah, one system test is still going.

We're up to 4 hours on getting this to pass CI, though. This is getting a little ridiculous.

@mheon
Copy link
Member Author

mheon commented Feb 19, 2021

/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 Feb 19, 2021
@openshift-merge-robot openshift-merge-robot merged commit 7e286bc into containers:v3.0 Feb 19, 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.