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

[v5.0] Bump Buildah to v1.35.3 #22219

Conversation

TomSweeneyRedHat
Copy link
Member

As the title says, bumping to Buildah v1.35.3 in preparation of Podman v5.0.1

[NO NEW TESTS NEEDED]

Does this PR introduce a user-facing change?

None

As the title says, bumping to Buildah v1.35.3 in preparation
of Podman v5.0.1

[NO NEW TESTS NEEDED]

Signed-off-by: tomsweeneyredhat <[email protected]>
Copy link
Contributor

openshift-ci bot commented Mar 30, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TomSweeneyRedHat

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 Mar 30, 2024
@TomSweeneyRedHat
Copy link
Member Author

@mheon @ashley-cui PTAL

@TomSweeneyRedHat
Copy link
Member Author

@mheon the failures here look real, network related. Are there Podman changes that need to be backported too?

@Luap99
Copy link
Member

Luap99 commented Mar 30, 2024

see my mail, you need to grab my pasta fixes from https://github.com/containers/podman/pull/22079/commits

Luap99 added 2 commits March 30, 2024 09:46
By default we just ignored any localhost reolvers, this is problematic
for anyone with more complicated dns setups, i.e. split dns with
systemd-reolved. To address this we now make use of the build in dns
proxy in pasta. As such we need to set the default nameserver ip now.

A second change is the option to exclude certain ips when generating the
host.containers.internal ip. With that we no longer set it to the same
ip as is used in the netns. The fix is not perfect as it could mean on a
system with a single ip we no longer add the entry, however given the
previous entry was incorrect anyway this seems like the better behavior.

Fixes containers#22044

[NO NEW TESTS NEEDED]

Signed-off-by: Paul Holzinger <[email protected]>
Always teardown the network, trying to reuse the netns has caused
a significant amount of bugs in this code here. It also never worked
for containers with user namespaces. So once and for all simplify this
by never reusing the netns. Originally this was done to have a faster
restart of containers but with netavark now we are much faster so it
shouldn't be that noticeable in practice. It also makes more sense to
reconfigure the netns as it is likely that the container exited due
some broken network state in which case reusing would just cause more
harm than good.

The main motivation for this change was the pasta change to use
--dns-forward by default. As the restarted contianer had no idea what
nameserver to use as pasta just kept running.

[NO NEW TESTS NEEDED]

Signed-off-by: Paul Holzinger <[email protected]>
Signed-off-by: tomsweeneyredhat <[email protected]>
@TomSweeneyRedHat
Copy link
Member Author

@Luap99 Thanks! I've just cherry-picked those two, fingers crossed on the tests.

@TomSweeneyRedHat
Copy link
Member Author

Different errors after the merge. Now it looks to be ssh proxy issues on the machine.

@Luap99
Copy link
Member

Luap99 commented Mar 30, 2024

machine tests are still flaky I think, I restarted them

@TomSweeneyRedHat
Copy link
Member Author

Restart got it, Woot! Thanks @Luap99
Happy Green Test Buttons, good for L-G-T-Ming and merge.

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

@rhatdan
Copy link
Member

rhatdan commented Mar 31, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 31, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 1ecd7d0 into containers:v5.0 Mar 31, 2024
92 of 93 checks passed
@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Jun 30, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Jun 30, 2024
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. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants