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

pkg/specgen: parse default network mode on server #14436

Merged
merged 1 commit into from
Jun 2, 2022

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented May 31, 2022

When podman-remote is used we should not resolve the default network
mode on the client. Defaults should be set on the server. In this case
this is important because we have different defaults for root/rootless.
So when the client is rootless and the server is root we must pick the
root default.

Note that this already worked when --network was set since we did not
parsed the flag in this case. To reproduce you need --network=default.

Also removed a unused function.

[NO NEW TESTS NEEDED] I tested it manually but I am not sure how I can
hook a test like this up in CI. The client would need to run as rootless
and the server as root or the other way around.

Fixes #14368

Does this PR introduce a user-facing change?

The default network mode is now assigned by the server and not the client.

@openshift-ci openshift-ci bot added release-note approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 31, 2022
@rhatdan
Copy link
Member

rhatdan commented May 31, 2022

LGTM

@mheon
Copy link
Member

mheon commented May 31, 2022

Tests red

@Luap99 Luap99 force-pushed the net-remote-default branch 2 times, most recently from af2e8ab to d5afa82 Compare June 1, 2022 13:14
When podman-remote is used we should not resolve the default network
mode on the client. Defaults should be set on the server. In this case
this is important because we have different defaults for root/rootless.
So when the client is rootless and the server is root we must pick the
root default.

Note that this already worked when --network was set since we did not
parsed the flag in this case. To reproduce you need --network=default.

Also removed a unused function.

[NO NEW TESTS NEEDED] I tested it manually but I am not sure how I can
hook a test like this up in CI. The client would need to run as rootless
and the server as root or the other way around.

Fixes containers#14368

Signed-off-by: Paul Holzinger <[email protected]>
@Luap99 Luap99 force-pushed the net-remote-default branch from d5afa82 to 2805c73 Compare June 1, 2022 15:14
@Luap99
Copy link
Member Author

Luap99 commented Jun 2, 2022

Tests are green

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

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 2, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 2, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, vrothberg

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-merge-robot openshift-merge-robot merged commit 8b972ff into containers:main Jun 2, 2022
@Luap99 Luap99 deleted the net-remote-default branch June 2, 2022 12:50
@edsantiago edsantiago added the kind/bug Categorizes issue or PR as related to a bug. label Jun 2, 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 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 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/bug Categorizes issue or PR as related to a bug. 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman-remote defaults to client host's default network mode (slirp4netns when not running as root)
6 participants