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

Add support for bridge mode #24677

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

M1cha
Copy link
Contributor

@M1cha M1cha commented Nov 25, 2024

This simply updates common, as there is nothing to do from the podman side. I added documentation for this new option and improved the --internal documentation.

Related:

Does this PR introduce a user-facing change?

None

Signed-off-by: Michael Zimmermann <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Nov 25, 2024

@Luap99 PTAL

@@ -118,6 +126,9 @@ Additionally the `bridge` driver supports the following options:
- `com.docker.network.bridge.name`: This option assigns the given name to the created Linux Bridge
- `com.docker.network.driver.mtu`: Sets the Maximum Transmission Unit (MTU) and takes an integer value.
- `vrf`: This option assigns a VRF to the bridge interface. It accepts the name of the VRF and defaults to none. Can only be used with the Netavark network backend.
- `mode`: This option sets the specified bridge mode on the interface. Defaults to `managed`. Supported values:
- `managed`: Podman creates and deleted the bridge. It will also setup sysctls and firewall rules for it.
Copy link
Member

Choose a reason for hiding this comment

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

s/deletes/deleted if you repush

@baude
Copy link
Member

baude commented Nov 25, 2024

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 25, 2024
@mheon
Copy link
Member

mheon commented Nov 25, 2024

Docs changes LGTM

@@ -118,6 +126,9 @@ Additionally the `bridge` driver supports the following options:
- `com.docker.network.bridge.name`: This option assigns the given name to the created Linux Bridge
- `com.docker.network.driver.mtu`: Sets the Maximum Transmission Unit (MTU) and takes an integer value.
- `vrf`: This option assigns a VRF to the bridge interface. It accepts the name of the VRF and defaults to none. Can only be used with the Netavark network backend.
- `mode`: This option sets the specified bridge mode on the interface. Defaults to `managed`. Supported values:
- `managed`: Podman creates and deleted the bridge. It will also setup sysctls and firewall rules for it.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add that the firewall rules are added to MASQUERADE/NAT the traffic and for port forwarding DNAT rules are added. And then in the unmanaged case make clear no firewall rules are added which means no DNAT rules for port forwarding as well which might surprise users if they add ports but they don't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea. added.

Signed-off-by: Michael Zimmermann <[email protected]>
This goes into more detail about what this option actually does.

Signed-off-by: Michael Zimmermann <[email protected]>
@M1cha M1cha force-pushed the network-create-docs branch from 162bf15 to e608874 Compare November 26, 2024 13:24
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

Thanks

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2024
Copy link
Contributor

openshift-ci bot commented Nov 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude, Luap99, M1cha

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

Copy link

Cockpit tests failed for commit e608874. @martinpitt, @jelly, @mvollmer please check.

@openshift-merge-bot openshift-merge-bot bot merged commit 0cfe47b into containers:main Nov 26, 2024
85 of 86 checks passed
@M1cha M1cha deleted the network-create-docs branch November 26, 2024 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants