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

Support changing host veth name #1125

Merged
merged 2 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/network/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use crate::{
use super::{
constants::{
ISOLATE_OPTION_FALSE, ISOLATE_OPTION_STRICT, ISOLATE_OPTION_TRUE,
NO_CONTAINER_INTERFACE_ERROR, OPTION_ISOLATE, OPTION_METRIC, OPTION_MTU,
OPTION_NO_DEFAULT_ROUTE, OPTION_VRF,
NO_CONTAINER_INTERFACE_ERROR, OPTION_HOST_INTERFACE_NAME, OPTION_ISOLATE, OPTION_METRIC,
OPTION_MTU, OPTION_NO_DEFAULT_ROUTE, OPTION_VRF,
},
core_utils::{self, get_ipam_addresses, join_netns, parse_option, CoreUtils},
driver::{self, DriverInfo},
Expand All @@ -38,6 +38,8 @@ const NO_BRIDGE_NAME_ERROR: &str = "no bridge interface name given";
struct InternalData {
/// interface name of the veth pair inside the container netns
container_interface_name: String,
/// interace name of the veth pair in the host netns
host_interface_name: String,
/// interface name of the bridge for on the host
bridge_interface_name: String,
/// static mac address
Expand Down Expand Up @@ -86,6 +88,11 @@ impl driver::NetworkDriver for Bridge<'_> {
let no_default_route: bool =
parse_option(&self.info.network.options, OPTION_NO_DEFAULT_ROUTE)?.unwrap_or(false);
let vrf: Option<String> = parse_option(&self.info.network.options, OPTION_VRF)?;
let host_interface_name = parse_option(
&self.info.per_network_opts.options,
OPTION_HOST_INTERFACE_NAME,
)?
.unwrap_or_else(|| "".to_string());

let static_mac = match &self.info.per_network_opts.static_mac {
Some(mac) => Some(CoreUtils::decode_address_from_hex(mac)?),
Expand All @@ -95,6 +102,7 @@ impl driver::NetworkDriver for Bridge<'_> {
self.data = Some(InternalData {
bridge_interface_name: bridge_name,
container_interface_name: self.info.per_network_opts.interface_name.clone(),
host_interface_name,
mac_address: static_mac,
ipam,
mtu,
Expand Down Expand Up @@ -647,17 +655,25 @@ fn create_veth_pair<'fd>(
let mut peer = LinkMessage::default();
netlink::parse_create_link_options(&mut peer, peer_opts);

let mut host_veth = netlink::CreateLinkOptions::new(String::from(""), InfoKind::Veth);
let mut host_veth =
netlink::CreateLinkOptions::new(data.host_interface_name.clone(), InfoKind::Veth);
host_veth.mtu = data.mtu;
host_veth.primary_index = primary_index;
host_veth.info_data = Some(InfoData::Veth(InfoVeth::Peer(peer)));

host.create_link(host_veth).map_err(|err| match err {
NetavarkError::Netlink(ref e) if -e.raw_code() == libc::EEXIST => NetavarkError::wrap(
format!(
"create veth pair: interface {} already exists on container namespace",
data.container_interface_name
),
if data.host_interface_name.is_empty() {
format!(
"create veth pair: interface {} already exists on container namespace",
data.container_interface_name
)
} else {
format!(
"create veth pair: interface {} already exists on container namespace or {} exists on host namespace",
data.container_interface_name, data.host_interface_name,
)
},
err,
),
_ => NetavarkError::wrap("create veth pair", err),
Expand Down
1 change: 1 addition & 0 deletions src/network/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub const OPTION_METRIC: &str = "metric";
pub const OPTION_NO_DEFAULT_ROUTE: &str = "no_default_route";
pub const OPTION_BCLIM: &str = "bclim";
pub const OPTION_VRF: &str = "vrf";
pub const OPTION_HOST_INTERFACE_NAME: &str = "host_interface_name";

/// 100 is the default metric for most Linux networking tools.
pub const DEFAULT_METRIC: u32 = 100;
Expand Down
4 changes: 4 additions & 0 deletions src/network/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ pub struct PerNetworkOptions {
/// MAC address for the container interface.
#[serde(rename = "static_mac")]
pub static_mac: Option<String>,

/// Driver-specific options for this container.
#[serde(rename = "options")]
pub options: Option<HashMap<String, String>>,
}

/// PortMapping is one or more ports that will be mapped into the container.
Expand Down
30 changes: 30 additions & 0 deletions test/610-bridge-vethname.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/usr/bin/env bats -*- bats -*-
#
# bridge driver tests with static veth names
#

load helpers

@test bridge - valid veth name {
run_netavark --file ${TESTSDIR}/testfiles/bridge-vethname.json setup $(get_container_netns_path)

run_in_host_netns ip -j --details link show my-veth
link_info="$output"
assert_json "$link_info" '.[].flags[] | select(.=="UP")' == "UP" "Host veth is up"
assert_json "$link_info" ".[].linkinfo.info_kind" == "veth" "The veth interface is actually a veth"
assert_json "$link_info" ".[].master" "==" "podman0" "veth is part of the correct bridge"

run_netavark --file ${TESTSDIR}/testfiles/bridge-vethname.json teardown $(get_container_netns_path)

# check if the interface gets removed
expected_rc=1 run_in_host_netns ip -j --details link show my-veth
assert "$output" "==" 'Device "my-veth" does not exist.'
}

@test bridge - existing veth name {
expected_rc=1 run_netavark --file ${TESTSDIR}/testfiles/bridge-vethname-exists.json setup $(get_container_netns_path)
Copy link
Member

Choose a reason for hiding this comment

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

please assert your exact expected error message here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I gotta say, it's pretty cool that the tests can do this. done.

assert_json ".error" "create veth pair: interface eth0 already exists on container namespace or podman0 exists on host namespace: Netlink error: File exists (os error 17)"

expected_rc=1 run_in_host_netns ip -j --details link show my-veth
assert "$output" "==" 'Device "my-veth" does not exist.'
}
32 changes: 32 additions & 0 deletions test/testfiles/bridge-vethname-exists.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"container_id": "6ce776ea58b5",
"container_name": "testcontainer",
"networks": {
"podman": {
"interface_name": "eth0",
"static_ips": [
"10.88.0.2"
],
"options": {
"host_interface_name": "podman0"
}
}
},
"network_info": {
"podman": {
"dns_enabled": false,
"driver": "bridge",
"id": "53ce4390f2adb1681eb1a90ec8b48c49c015e0a8d336c197637e7f65e365fa9e",
"internal": false,
"ipv6_enabled": false,
"name": "podman",
"network_interface": "podman0",
"subnets": [
{
"gateway": "10.88.0.1",
"subnet": "10.88.0.0/16"
}
]
}
}
}
32 changes: 32 additions & 0 deletions test/testfiles/bridge-vethname.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"container_id": "6ce776ea58b5",
"container_name": "testcontainer",
"networks": {
"podman": {
"interface_name": "eth0",
"static_ips": [
"10.88.0.2"
],
"options": {
"host_interface_name": "my-veth"
}
}
},
"network_info": {
"podman": {
"dns_enabled": false,
"driver": "bridge",
"id": "53ce4390f2adb1681eb1a90ec8b48c49c015e0a8d336c197637e7f65e365fa9e",
"internal": false,
"ipv6_enabled": false,
"name": "podman",
"network_interface": "podman0",
"subnets": [
{
"gateway": "10.88.0.1",
"subnet": "10.88.0.0/16"
}
]
}
}
}