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 v4.3.0 #16221

Merged
merged 55 commits into from
Oct 19, 2022
Merged

Bump to v4.3.0 #16221

merged 55 commits into from
Oct 19, 2022

Conversation

mheon
Copy link
Member

@mheon mheon commented Oct 18, 2022

Release notes and backports for v4.3.0 release, plus the actual release bump.

NONE

mheon and others added 30 commits October 17, 2022 16:46
Only in container/pod stop/rm/restart man pages; the others
(volume-rm, network-rm, system-service) are too different to refactor.

Mostly an easy one, no manual reconciliation needed apart from
the pod-vs-container difference.

Signed-off-by: Ed Santiago <[email protected]>
We have a test to verify that init containers in pods are
deleted when the `--init-ctr=once` option is specified. The test
creates two containers, one of them an init container, starts the
pod, stops the pod, and restarts the pod, checking for the
presence of a file created by the init container during the
second start. We're seeing a race where the file still exists,
which I'm fairly certain comes down to the SHM mount not being
cleaned up after the pod is stopped.

Fortunately, we already have code to do this - just flip the bool
that controls cleanup from false to true.

[NO NEW TESTS NEEDED] Fixes a difficult to reproduce race
condition.

Fixes containers#16046

Signed-off-by: Matthew Heon <[email protected]>
When shutting down the image engine we always wait for the image
even goroutine to finish writing any outstanding events. However,
the loop for that always waits 100msec every iteration. This means
that (depending on the phase) shutdown is always delayed up to 100msec.

This is delaying "podman run" extra much because podman is run twice
(once for the run and once as cleanup via a conmon callback).

Changing the image loop to exit immediately when a libimageEventsShutdown
(but first checking for any outstanding events to write) improves podman
run times by about 100msec on average.

Note: We can't just block on the event loop reading the shutdown event
anymore, we need to wait until it read and processed any outstanding
events, so we now send the shutdown event and then block waiting for the
channel to be closed by the event loop.

[NO NEW TESTS NEEDED]

Signed-off-by: Alexander Larsson <[email protected]>
Make sure that the on-failure actions only kick in once the health check
has passed its retries.  Also fix race conditions on reading/writing the
log.

Signed-off-by: Valentin Rothberg <[email protected]>
This moves the cgroup code to pod_internal_linux.go and adds a no-op
stub for FreeBSD.

[NO NEW TESTS NEEDED]

Signed-off-by: Doug Rabson <[email protected]>
The value of p.PodSpecGen.InfraContainerSpec.ResourceLimits can be nil
on FreeBSD.

[NO NEW TESTS NEEDED]

Signed-off-by: Doug Rabson <[email protected]>
Most of the code can be shared with other unix-like platforms.

[NO NEW TESTS NEEDED]

Signed-off-by: Doug Rabson <[email protected]>
This moves the code to runtime_pod_linux.go since cgroups are
platform-specific.

[NO NEW TESTS NEEDED]

Signed-off-by: Doug Rabson <[email protected]>
[NO NEW TESTS NEEDED]

Signed-off-by: Doug Rabson <[email protected]>
Fix the "stop" on-failure action by not removing the transient systemd
timer and service during container stop.  Removing the service will
in turn cause systemd to terminate the Podman process attempting to
stop the container and hence leave it in the "stopping" state.

Instead move the removal into the restart sequence.

Signed-off-by: Valentin Rothberg <[email protected]>
When you run "podman run foo" we attach to the container, which essentially
blocks until the container process exits. When that happens podman immediately
calls Container.WaitForExit(), but at this point the exit value has not
yet been written to the db by conmon. This means that we almost always
hit the "check for exit state; sleep 250msec" loop in WaitForExit(),
delaying the exit of podman run by 250 msec.

More recent kernels (>= 5.3) supports the pidfd_open() syscall, that
lets you open a fd representing a pid and then poll on it to wait
until the process exits. We can use this to have the first sleep
be exactly as long as is needed for conmon to exit (if we know its pid).
If for whatever reason there is still issues we use the old sleep loop
on later iterations.

This makes "time podman run fedora true" about 200msec faster.

[NO NEW TESTS NEEDED]

Signed-off-by: Alexander Larsson <[email protected]>
This just gets ctr.config.Spec.Process.Terminal with some null checks,
allowing several places that open-coded this to use the helper.

In particular, this helps the code in
pkg/domain/infra/abi/terminal.StartAttachCtr(), that used to do:
`ctr.Spec().Process.Terminal`, which looks fine, but actually causes
a deep json copy in the `ctr.Spec()` call that takes over 3 msec.

[NO NEW TESTS NEEDED] Just minor performance effects

Signed-off-by: Alexander Larsson <[email protected]>
When the `XDG_CONFIG_HOME` environment variable is changed, for example,
to switch development contexts, the behavior of the podman-machine can
be confusing. The documentation had not mentioned this, and this commit
adds these mentions.

Closes: containers#15577

Reviewed-by: Daniel J Walsh <[email protected]>
Signed-off-by: Naoaki Ueda <[email protected]>
Basically, in the timeout loop where we checked for new CID
on the restarted container, we were running 'podman inspect'
(not 'inspect --format ID'), and comparing full hundred-line
output against single-line CID string.

While I'm in here, add 'c_' prefix to container to make it
easier for my old eyes to recognize "oh, that's a container name"
vs "is that a name? a SHA? a woozle?"

Signed-off-by: Ed Santiago <[email protected]>
This reverts commit 9c3d8bb.

Signed-off-by: Daniel J Walsh <[email protected]>
This reverts commit bbe1063.

Signed-off-by: Daniel J Walsh <[email protected]>
Bumps [github.com/onsi/gomega](https://github.com/onsi/gomega) from 1.20.2 to 1.22.1.
- [Release notes](https://github.com/onsi/gomega/releases)
- [Changelog](https://github.com/onsi/gomega/blob/master/CHANGELOG.md)
- [Commits](onsi/gomega@v1.20.2...v1.22.1)

---
updated-dependencies:
- dependency-name: github.com/onsi/gomega
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Only between the two podman-manifest-* commands. podman-build
is too different.

Easy one, text was already identical

Signed-off-by: Ed Santiago <[email protected]>
Bumps [golang.org/x/text](https://github.com/golang/text) from 0.3.7 to 0.3.8.
- [Release notes](https://github.com/golang/text/releases)
- [Commits](golang/text@v0.3.7...v0.3.8)

---
updated-dependencies:
- dependency-name: golang.org/x/text
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Starting listening for the READY messages on the sdnotify proxies before
starting the Pod.  Otherwise, we may be missing messages.

[NO NEW TESTS NEEDED] as it's hard to test this very narrow race.

Related to but may not be fixing containers#16076.

Signed-off-by: Valentin Rothberg <[email protected]>
Bumps [github.com/spf13/cobra](https://github.com/spf13/cobra) from 1.5.0 to 1.6.0.
- [Release notes](https://github.com/spf13/cobra/releases)
- [Commits](spf13/cobra@v1.5.0...v1.6.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/cobra
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

PH: manually update the completion scripts and fix deprecated function
call.

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Paul Holzinger <[email protected]>
The new cobra update fixed a bug which caused some options to not be
included in --help when there was already a option with the same name
on a parent command.

Signed-off-by: Paul Holzinger <[email protected]>
Bumps [github.com/containers/ocicrypt](https://github.com/containers/ocicrypt) from 1.1.5 to 1.1.6.
- [Release notes](https://github.com/containers/ocicrypt/releases)
- [Commits](containers/ocicrypt@v1.1.5...v1.1.6)

---
updated-dependencies:
- dependency-name: github.com/containers/ocicrypt
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
To improve the error message reported in containers#16142 where the container is
reported to be in the wrong state but we do not know which.  This is not
a fix for containers#16142 but will hopefully aid in better understanding what's
going on if it flakes again.

[NO NEW TESTS NEEDED] as hitting the condition is inherently racy.

Signed-off-by: Valentin Rothberg <[email protected]>
[Note: I already refactored --annotation for container-related
 commands; this one is for manifest-related commands]

This one needed reconciling: one man page said "newly added image",
the other said "specified image", I just reduced that to "image".
If that's not cool, any suggestions on how to make it better? Or,
just reject this PR, we can live with this duplication.

Signed-off-by: Ed Santiago <[email protected]>
jnohlgard and others added 8 commits October 18, 2022 13:53
Avoids the error "Error: error preparing container xyz... for attach:
crun: open /proc/sys/net/ipv4/ping_group_range: Read-only file system:
OCI runtime error" when using `podman run --net bridge` inside rootful
Podman running without --security-opt unmask=ALL (or 'unmask=/proc/*')

Signed-off-by: Joakim Nohlgård <[email protected]>
containers-common now has a new `-extra` subpackage which handles
dependencies common to podman and buildah and also depends on
the main package `containers-common` itself.

The podman-next copr rebuilds containers-common from the rawhide branch
of dist-git so it will always have the latest version and will also
supersede the official containers-common packages (except on rawhide
where it will be equal).

Fixes: containers#16137

Signed-off-by: Lokesh Mandvekar <[email protected]>
[NO NEW TESTS NEEDED]

Signed-off-by: Doug Rabson <[email protected]>
This simply runs ps(1) on the host and filters for processes inside the
container.

[NO NEW TESTS NEEDED]

Signed-off-by: Doug Rabson <[email protected]>
Signed-off-by: Matthew Heon <[email protected]>
Signed-off-by: Matthew Heon <[email protected]>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 18, 2022

[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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 18, 2022
@github-actions github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Oct 18, 2022
@mheon
Copy link
Member Author

mheon commented Oct 18, 2022

@TomSweeneyRedHat and anyone else who might want to double-check the release notes... PTAL

@rhatdan
Copy link
Member

rhatdan commented Oct 18, 2022

LGTM

Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

Whew. No major issues, but enough to (IMO) merit a re-push. Nice work.

RELEASE_NOTES.md Outdated
- A new command, `podman update`, has been added,which makes changes to the resource limits of existing containers. Please note that these changes do not persist if the container is restarted ([#15067](https://github.com/containers/podman/issues/15067)).
- A new command, `podman kube down`, has been added, which removes pods and containers created by the given Kubernetes YAML (functionality is identical to `podman kube play --down`, but it now has its own command).
- The `podman kube play` command now supports Kubernetes secrets using Podman's secrets backend.
- Systemd-managed pods created by the `podman kube play` command now integrates with sd-notify, using the `io.containers.sdnotify` annotation (or `io.containers.sdnotify/$name` for specific containers).
Copy link
Member

Choose a reason for hiding this comment

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

integrate, not integrates

RELEASE_NOTES.md Outdated
- The remote Podman client now supports proxying signals for attach sessions when the `--sig-proxy` option is set ([#14707](https://github.com/containers/podman/issues/14707)).

### Changes
- Duplicate volume mounts are now allowed, so long as source, destination, and options all match ([#4217](https://github.com/containers/podman/issues/4217)).
Copy link
Member

Choose a reason for hiding this comment

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

How about are now allowed **on the command line**? The wording as it is suggests that the mounts themselves are duplicated.

- A number of Podman commands (`podman init`, `podman container checkpoint`, `podman container restore`, `podman container cleanup`) now print the user-inputted name of the container, instead of its full ID, on success.
- When an unsupported option (e.g. resource limit) is specified for a rootless container on a cgroups v1 system, a warning message is now printed that the limit will not be honored.
- The installer for the Windows Podman client has been improved.
- The `--cpu-rt-period` and `--cpu-rt-runtime` options to `podman run` and `podman create` now print a warning and are ignored on cgroups v2 systems (cgroups v2 having dropped support for these controllers) ([#15666](https://github.com/containers/podman/issues/15666)).
Copy link
Member

Choose a reason for hiding this comment

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

Could this be consolidated with the entry on line 46 (two lines above) about unsupported resource limits? (Maybe not, maybe the rootless is an important difference)

Copy link
Member Author

Choose a reason for hiding this comment

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

Rootless is an important difference, so they cannot

RELEASE_NOTES.md Outdated
- When an unsupported option (e.g. resource limit) is specified for a rootless container on a cgroups v1 system, a warning message is now printed that the limit will not be honored.
- The installer for the Windows Podman client has been improved.
- The `--cpu-rt-period` and `--cpu-rt-runtime` options to `podman run` and `podman create` now print a warning and are ignored on cgroups v2 systems (cgroups v2 having dropped support for these controllers) ([#15666](https://github.com/containers/podman/issues/15666)).
- Privileged containers running systemd will no longer mount `/dev/tty*` into the container ([#15878](https://github.com/containers/podman/issues/15878)).
Copy link
Member

Choose a reason for hiding this comment

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

Ugh. This is actually a bug in the PR title itself, it irked me but I didn't fix it. Shame on me.

This should actually be /dev/ttyN, or /dev/ttyX, or "tty devices other than /dev/tty". The use of *, in a context where one will read it as a glob, suggests that /dev/tty itself will not be mounted.

RELEASE_NOTES.md Outdated
- Pods created by `podman play kube` are now, by default, placed into a network named `podman-kube`. If the `podman-kube` network does not exist, it will be created. This ensures pods can connect to each other by their names, as the network has DNS enabled.

### Bugfixes
- Fixed a bug where the `podman network prune` and `podman container prune` commands did not properly support the `--filer label!=` option ([#14182](https://github.com/containers/podman/issues/14182)).
Copy link
Member

Choose a reason for hiding this comment

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

typo, filter not filer

RELEASE_NOTES.md Outdated
- Fixed a bug where API forwarding with `podman machine` VMs on windows could sometimes fail because the pipe was not created in time ([#14811](https://github.com/containers/podman/issues/14811)).
- Fixed a bug where the `podman pod rm` command could error if removal of a container in the pod was interrupted by a reboot.
- Fixed a bug where the `exited` and `exec died` events for containers did not include the container's labels ([#15617](https://github.com/containers/podman/issues/15617)).
- Fixed a bug where running Systemd containers on a systemd not using Systemd as PID 1 could fail ([#15647](https://github.com/containers/podman/issues/15647)).
Copy link
Member

Choose a reason for hiding this comment

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

...on a system not using Systemd (not "on a systemd")

RELEASE_NOTES.md Outdated
- Fixed a bug where the `podman events` command could function improperly when no events were present ([#15688](https://github.com/containers/podman/issues/15688)).
- Fixed a bug where the `--format` flag to various Podman commands did not properly handle template strings including a newline (`\n`) ([#13446](https://github.com/containers/podman/issues/13446)).
- Fixed a bug where Systemd-managed pods would kill every container in a pod when a single container exited ([#14546](https://github.com/containers/podman/issues/14546)).
- Fixed a bug where the `podman generate systemd` command would generate incorrect yAML for pods created without the `--name` option.
Copy link
Member

Choose a reason for hiding this comment

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

YAML (capitalization glitch)

RELEASE_NOTES.md Outdated
- Fixed a bug where the `podman machine list --format json` command did not properly show machine starting status.
- Fixed a bug where automatic updates would not error when attempting to update a container with a non-fully qualified image name ([#15879](https://github.com/containers/podman/issues/15879)).
- Fixed a bug where the `podman pod logs --latest` command could panic ([#15556](https://github.com/containers/podman/issues/15556)).
- Fixed a bug where Podman could leave lingering network namespace mounts on the system if cleaning up the network failed for an unrelated reason.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure what the "for an unrelated reason" adds here; Unrelated to what? I'd suggest nuking it.

RELEASE_NOTES.md Outdated

### API
- Fixed a bug where the Compat DF endpoint reported incorrect reference counts for volumes ([#15720](https://github.com/containers/podman/issues/15720)).
- Fixed a bug where the Compat Inspect endpoint for Networks where an incorrect network option was displayed, causing issues with `docker-compose` ([#15580](https://github.com/containers/podman/issues/15580)).
Copy link
Member

Choose a reason for hiding this comment

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

Sentence does not parse. Maybe "a bug in the Compat..."? Or maybe something else needed?

RELEASE_NOTES.md Outdated
- Fixed a bug where the Compat Inspect endpoint for Networks where an incorrect network option was displayed, causing issues with `docker-compose` ([#15580](https://github.com/containers/podman/issues/15580)).
- The Libpod Restore endpoint for Containers now features a new query parameter, `pod`, to set the pod that the container will be restored into ([#15018](https://github.com/containers/podman/issues/15018)).
- Fixed a bug where the REST API could panic while retrieving images.
- Fixed a bug where a cancelled connection to several endpoints could, potentially, induce a memory leak.
Copy link
Member

Choose a reason for hiding this comment

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

", potentially," does not add anything; nuke it

Signed-off-by: Matthew Heon <[email protected]>
@mheon
Copy link
Member Author

mheon commented Oct 18, 2022

Re-pushed with fixes

@mheon
Copy link
Member Author

mheon commented Oct 18, 2022

Restarted one test. Otherwise should be ready. @Luap99 @baude PTAL.

@edsantiago
Copy link
Member

I restarted five, earlier. All of them search-related. We're going to be seeing that one a lot, I suspect.

@mheon
Copy link
Member Author

mheon commented Oct 19, 2022

The optional release test is failing, but we should be good to merge without it.

@rhatdan
Copy link
Member

rhatdan commented Oct 19, 2022

Does podman info show -dev?

@Luap99
Copy link
Member

Luap99 commented Oct 19, 2022

Does podman info show -dev?

Yes because the release PR always contains at least the bump to vX.Y.Z-dev commit after the real release/tagged commit. The optional release test make no sense unless it starts actually checking the bump to vX.Y.Z commit. And even then it might still go wrong when @mheon or anybody else who tags a release tag the wrong commit by accident!

@rhatdan
Copy link
Member

rhatdan commented Oct 19, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2022
@openshift-merge-robot openshift-merge-robot merged commit bd2ad61 into containers:v4.3 Oct 19, 2022
@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 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 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. kind/api-change Change to remote API; merits scrutiny 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. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.