Introduce netavark create command by ashley-cui · Pull Request #1394 · containers/netavark

ashley-cui · 2026-01-27T13:39:27Z

The netavark create command is a new command that takes a partially filled out network config, and some options, via a json on stdin. The command will validate the network and fill out incompleted fields in the config. It then will return a complete network config json. The behavior is to match and replace https://github.com/containers/container-libs/blob/1e46b0756b39f76f287c64aa46c41107ad1110bb/common/libnetwork/netavark/config.go#L118

ashley-cui · 2026-01-27T13:43:54Z

Container libs part lives here as a draft, but the tests won't pass until this PR merges, unless there's another way to throw it at tests? The config tests are updated there and pass locally, but if there's good ideas for testing, I'm all ears.

TomSweeneyRedHat · 2026-01-28T16:17:03Z

@ashley-cui you have some test unhappiness

docs/netavark-create-input.md

docs/netavark.1.md

packit-as-a-service · 2026-01-28T18:49:26Z

Ephemeral COPR build failed. @containers/packit-build please check.

ashley-cui · 2026-01-28T19:14:07Z

@containers/netavark-maintainers PTAL

TomSweeneyRedHat · 2026-01-28T20:01:26Z

LGTM
but I'd like at least one person with some serious rust chops to look at this.

Luap99

not a full review but some comments

I did not carefully check all the validation logic matches common but I guess we can run the common libnetwork unit tests with your new code to ensure we have proper coverage if you haven't done already

src/network/create_config/mod.rs

src/network/create_config/driver.rs

Luap99 · 2026-01-29T14:58:49Z

src/network/create_config/driver.rs

+    let mut child = Command::new(&plugin_path)
+        .arg("create")
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .stderr(Stdio::inherit())
+        .spawn()?;
+
+    let stdin = child.stdin.take().unwrap();
+    serde_json::to_writer(&stdin, &input)?;
+    // Close stdin here to avoid that the plugin waits forever for an EOF.
+    // And then we would wait for the child to exit which would cause a hang.
+    drop(stdin);
+
+    // Note: We need to buffer the output and then deserialize into the correct type after
+    // the plugin exits, since the plugin can return two different json types depending on
+    // the exit code.
+    let mut buffer: Vec<u8> = Vec::new();
+
+    let mut stdout = child.stdout.take().unwrap();
+    // Do not handle error here, we have to wait for the child first.
+    let result = stdout.read_to_end(&mut buffer);
+
+    let exit_status = wrap!(child.wait(), "wait for plugin to exit")?;
+    if let Some(rc) = exit_status.code() {
+        // make sure the buffer is correct
+        wrap!(result, "read into buffer")?;
+        if rc == 0 {
+            // read the modified network config
+            let updated_network: Network = serde_json::from_slice(&buffer)?;
+            // Update the network with the plugin's response
+            *network = updated_network;
+            Ok(())
+        } else {
+            // exit code not 0 => error
+            let err: JsonError = serde_json::from_slice(&buffer)?;
+            Err(NetavarkError::msg(format!(
+                "plugin {:?} failed with exit code {}, message: {}",
+                plugin_path.file_name().unwrap_or_default(),
+                rc,
+                err.error
+            )))
+        }
+    } else {
+        // If we could not get the exit code then the process was killed by a signal.
+        // I don't think it is necessary to read and return the signal so we just return a generic error.
+        Err(NetavarkError::msg(format!(
+            "plugin {:?} killed by signal",
+            plugin_path.file_name().unwrap_or_default()
+        )))
+    }
+}


this feels like copy pasted from the other location so this must be shared as well

There are some differences in how the code is used - the other driver plugin code requires fields such as container_name and container_id which we don't have since we're only filling out the config, I've re-pushed to consolidate some of the code using helper functions, but that was heavily LLM-aided. Let me know if the changes are theoretically okay, or if i should leave it as two separate functions.

src/network/create_config/driver.rs

Cargo.toml

src/network/create_config/driver.rs

ashley-cui · 2026-02-09T13:34:03Z

re-pushed with some fixes, and split out some of the commits for easier review, I'll squash them later.

ashley-cui · 2026-02-12T15:16:32Z

@containers/netavark-maintainers PTAL

mheon

LGTM

docs/netavark-create-input.md

TomSweeneyRedHat · 2026-02-24T23:46:05Z

LGTM

src/network/plugin.rs

src/network/create_config/config.rs

Luap99 · 2026-03-11T15:49:04Z

src/network/create_config/config.rs

+        validate_routes(&mut network)?;
+    }
+
+    network.created = Some(Utc::now());


Is UTC correct, looking at existing code we get the local time in the config not UTC?
I think this need to be the local time Local::now() unless I am missing something

I think UTC is correct because Go marshall/unmarshalls into utc?

Luap99 · 2026-03-11T15:50:53Z

src/network/create_config/driver.rs

+    };
+
+    // validate the given options
+    if let Some(value) = network_opts.remove("com.docker.network.driver.mtu") {


this is fine for now, long term I rather no have docker specific option in this code base so I need to think about how to do that.

Luap99 · 2026-03-11T16:00:40Z

src/network/create_config/driver.rs

+
+    let bridge_opts = parse_bridge_opts(&network.options, true)?;
+    if bridge_opts.vlan.is_some()
+        || (bridge_opts.mode.is_some() && bridge_opts.mode.unwrap() != "managed")


using is_some() followed by unwrap() is a bit of an anti pattern I would say.

generally speaking in this function both bools always act the same so one bool is enough. Then there is no need to declare them as mut since we could write this as early return code,

if bridge_opts.vlan.is_some() {return false} if let Some(mode) = bridge_opts.mode { if mode != "manged" {return false} } return true

It also seems like the function never returns an error, so remove the result and just return the single bool

38 has a ? that can result in an error, so I left the result

Luap99 · 2026-03-11T16:05:08Z

src/network/create_config/driver.rs

+                ipam_opts.insert("driver".to_string(), constants::IPAM_HOSTLOCAL.to_string());
+            }
+        }
+        d if d == constants::IPAM_HOSTLOCAL => {


that looks odd, is the syntax not just constants::IPAM_HOSTLOCAL => { which should match just fine?

src/network/create_config/driver.rs

Cargo.lock

Signed-off-by: Ashley Cui <acui@redhat.com>

The netavark create command is a new command that takes a partially filled out network config, and some options, via a json on stdin. The command will validate the network and fill out incompleted fields in the config. It then will return a complete network config json. The behavior is to match and replace https://github.com/containers/container-libs/blob/1e46b0756b39f76f287c64aa46c41107ad1110bb/common/libnetwork/netavark/config.go#L118 Signed-off-by: Ashley Cui <acui@redhat.com>

Signed-off-by: Ashley Cui <acui@redhat.com>

ashley-cui mentioned this pull request Jan 27, 2026

Use netavark create to fill out network config containers/container-libs#610

Draft

ashley-cui force-pushed the create branch from 05694c0 to 542f7ee Compare January 27, 2026 14:41

TomSweeneyRedHat reviewed Jan 28, 2026

View reviewed changes

docs/netavark-create-input.md Show resolved Hide resolved

TomSweeneyRedHat reviewed Jan 28, 2026

View reviewed changes

docs/netavark-create-input.md Outdated Show resolved Hide resolved

TomSweeneyRedHat reviewed Jan 28, 2026

View reviewed changes

docs/netavark.1.md Outdated Show resolved Hide resolved

TomSweeneyRedHat reviewed Jan 28, 2026

View reviewed changes

docs/netavark.1.md Outdated Show resolved Hide resolved

ashley-cui force-pushed the create branch 2 times, most recently from ea6d595 to 5047c27 Compare January 28, 2026 18:48

Luap99 reviewed Jan 29, 2026

View reviewed changes

ashley-cui force-pushed the create branch from 5047c27 to ac2ff71 Compare February 9, 2026 13:28

mheon approved these changes Feb 24, 2026

View reviewed changes

TomSweeneyRedHat reviewed Feb 24, 2026

View reviewed changes

docs/netavark-create-input.md Outdated Show resolved Hide resolved

ashley-cui force-pushed the create branch 4 times, most recently from 8629a75 to 4279e31 Compare March 3, 2026 14:56

Luap99 reviewed Mar 11, 2026

View reviewed changes

Luap99 mentioned this pull request Mar 12, 2026

routes: Add blackhole route support #1417

Open

ashley-cui added 3 commits March 14, 2026 21:56

Break out exec plugin logic

69c98a8

Signed-off-by: Ashley Cui <acui@redhat.com>

Re-use parse functions for drivers

9b95af8

Signed-off-by: Ashley Cui <acui@redhat.com>

ashley-cui force-pushed the create branch from 4279e31 to 9b95af8 Compare March 16, 2026 13:44

Conversation

ashley-cui commented Jan 27, 2026

Uh oh!

ashley-cui commented Jan 27, 2026

Uh oh!

TomSweeneyRedHat commented Jan 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

packit-as-a-service bot commented Jan 28, 2026

Uh oh!

ashley-cui commented Jan 28, 2026

Uh oh!

TomSweeneyRedHat commented Jan 28, 2026

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ashley-cui commented Feb 9, 2026

Uh oh!

ashley-cui commented Feb 12, 2026

Uh oh!

mheon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TomSweeneyRedHat commented Feb 24, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants