Support static addressing via `net.toml` #2445

zmrow · 2022-09-20T20:38:21Z

Issue number:
Closes #2204

Description of changes:
It's probably best to review this PR by commit to follow the chain of changes!

Adds support for setting ipv4/6 static addresses and simple routes in net.toml. Using the DNS API settings added in #2353 , a user can configure a complete and working simple static addressing setup.

In order to add this support, a new version of network config has been created that adds a few additional settings: static4, static6, and route. These settings are deserialized in net_config/v2 into the structures defined in net_config/static_addressing. They are then converted to the associated structures defined in wicked/static_addressing. The Wicked* structures are then serialized to file.

An additional test macro has been defined in net_config containing unit tests for the static addressing validation and deserialization. The e2e tests defined in wicked have also been updated to include various configurations. The wicked e2e test now iterates through the network configuration versions, validating both of them.

The wicked helper netdog install has been updated to handle static addressses, reading from the correct lease file and ensuring that the current IP is always written. In the case multiple IPs exist, the first IP in the addresses list is uses as the primary. If ipv4 and ipv6 addresses are set, the first ipv4 addresses is used as the primary.

An example valid net config from my home setup:

version = 2

[eno1.static4]
addresses = ["192.168.86.200/24"]

[[eno1.route]]
to = "default"
via "192.168.86.1"

along with the corresponding DNS API settings (user-data.toml):

[settings.dns]
name-servers = ["192.168.86.1"]

Testing done:

All new and old unit tests pass.
Manually validate (using aws-dev VM) that all of the config setups defined in test_data/wicked/net_config.toml are properly parsed and addresses are properly set. I did this by writing the resulting XML config directly to /etc/wicked/ifconfig/eth0 and restarting the network. Since my network is simple, the kernel chokes on trying to reach some of the defined routes and therefore doesn't set them, but at least we know the configuration is properly parsed.
Boot aws-k8s-1.21 and ensure the simple network defined in the kernel cmdline continues to work.
Boot metal-dev with a few different static IP configurations, DNS settings, and a default route that would work on my network. Successfully reaches the internet and run containers.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

This commit begins the process of modularizing the `wicked` code to prepare for future additions. The structure will largely follow the structure in `net_config` to make the code easy to correlate. Currently only the DHCP-related wicked code is split out; more to come.

zmrow · 2022-09-20T20:49:50Z

^ Add an additional comment regarding the link scope when a route's gateway (via) is omitted.

zmrow · 2022-09-23T21:19:06Z

sources/api/netdog/src/wicked/dhcp.rs

@@ -0,0 +1,119 @@
+use crate::net_config::{Dhcp4ConfigV1, Dhcp4OptionsV1, Dhcp6ConfigV1, Dhcp6OptionsV1};


Nothing in this file is new code, all of it was moved out of wicked.rs

bcressey

The code changes look great to me. I'd like to try this out under QEMU just to see what it looks like on a running system, but otherwise I don't see any blockers. Nice work.

sources/api/netdog/src/net_config/static_address.rs

sources/api/netdog/src/net_config/v1.rs

bcressey · 2022-09-27T00:40:37Z

sources/api/netdog/src/net_config/v2.rs

+            primary_count <= 1,
+            error::InvalidNetConfigSnafu {
+                reason: "multiple primary interfaces defined, expected 1"
+            }


Do we expect 1 or 0 or 1?

We expect 0 or 1. In the case a user does not tell us which interface is primary, we document that we choose the first interface in the file.

sources/api/netdog/src/lease.rs

bcressey · 2022-09-27T00:47:04Z

sources/api/netdog/src/cli/install.rs

@@ -1,12 +1,12 @@
 use super::{error, InterfaceFamily, InterfaceType, Result};
 use crate::dns::DnsSettings;
-use crate::lease::{lease_path, LeaseInfo};
+use crate::lease::{dhcp_lease_path, static_lease_path, LeaseInfo};


nit: commit says "we use similar log" where I think "logic" is meant

sources/api/netdog/src/cli/install.rs

PROVISIONING-METAL.md

Adds the corresponding static address modules to both `net_config` and `wicked`. The `net_config` module contains the structs that `net.toml` deserializes into. The `wicked` module contains the structs that are serialized into interface configuration files, as well as the code to convert the `net_config` structs into `wicked*` structs.

This commit adds a `new()` constructor for `WickedInterface`. This constructor helps to hide defaults for `WickedControl`, and provides a base on which to build out the structure (in lieu of a full on builder). As we add additional struct members, we can add them to this method rather than to all the callers/users of `WickedInterface`.

This change adds v2 of network config, which supports setting static addresses and routes. An additional test macro and associated test files have also been added to validate the functionality.

This change adds an additional public function to the `lease` module that allows the user to fetch the path for a given interface's static lease if it exists. It renames the original `lease_path` function to `dhcp_lease_path` to be a bit more descriptive, and moves the shared functionality to a private function.

This change ensures that when `netdog install` is called when setting up a static address, the proper lease file is used to write the current IP. When writing the `resolv.conf` file at this time, we use similar logic as before, getting settings from DNS config or from the DHCP lease, since static addresses don't contain DNS info. This change also unifies the logic of gathering the lease, writing the resolv.conf and current IP into shared functions.

This commit updates the wicked e2e testing. It templates the `net_config.toml` and adds additional config to said file to ensure that the static address configuration renders properly. The wicked testing function has been updated to loop through existing net config versions.

This adds the docs for v2 of network configuration, which supports adding static addresses and simple routes to an interface.

zmrow · 2022-09-27T17:05:41Z

^ Addresses all of @bcressey 's comments! Thanks!

foersleo · 2022-09-29T09:31:11Z

PROVISIONING-METAL.md

+  * `to` (`"default"` or IP address with prefix, required): Destination address.
+  * `from` (IP address): Source IP address.
+  * `via` (IP address): Gateway IP address.  If no gateway is provided, a scope of `link` is assumed.
+  * `route-metric` (integer): Relative route priority.


nit: I do not know if we need to be that specific but this should probably be unsigned integer.

Hm. I don't think we need to get that specific, but we'll definitely change this if we get reports of folks having issues with it!

zmrow requested review from etungsten and bcressey September 20, 2022 20:38

zmrow force-pushed the static-addressing branch from 1a19d04 to 2a02a46 Compare September 20, 2022 20:49

zmrow changed the title ~~Network config: Support static addressing via net.toml~~ Support static addressing via net.toml Sep 20, 2022

zmrow commented Sep 23, 2022

View reviewed changes

bcressey approved these changes Sep 27, 2022

View reviewed changes

zmrow force-pushed the static-addressing branch from 2a02a46 to b86b6dc Compare September 27, 2022 17:02

zmrow added 6 commits September 27, 2022 17:04

netdog: Add net config v2 and static address tests

c9db785

This change adds v2 of network config, which supports setting static addresses and routes. An additional test macro and associated test files have also been added to validate the functionality.

PROVISIONING-METAL: Add net config v2 documentation

f50bbbd

This adds the docs for v2 of network configuration, which supports adding static addresses and simple routes to an interface.

zmrow force-pushed the static-addressing branch from b86b6dc to f50bbbd Compare September 27, 2022 17:05

foersleo approved these changes Sep 29, 2022

View reviewed changes

zmrow merged commit d752435 into bottlerocket-os:develop Sep 30, 2022

zmrow deleted the static-addressing branch September 30, 2022 15:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support static addressing via `net.toml` #2445

Support static addressing via `net.toml` #2445

zmrow commented Sep 20, 2022 •

edited

zmrow commented Sep 20, 2022

zmrow Sep 23, 2022

bcressey left a comment

bcressey Sep 27, 2022

zmrow Sep 27, 2022 •

edited

bcressey Sep 27, 2022

zmrow commented Sep 27, 2022

foersleo Sep 29, 2022

zmrow Sep 30, 2022

		@@ -0,0 +1,119 @@
		use crate::net_config::{Dhcp4ConfigV1, Dhcp4OptionsV1, Dhcp6ConfigV1, Dhcp6OptionsV1};

Support static addressing via net.toml #2445

Support static addressing via net.toml #2445

Conversation

zmrow commented Sep 20, 2022 • edited

zmrow commented Sep 20, 2022

zmrow Sep 23, 2022

Choose a reason for hiding this comment

bcressey left a comment

Choose a reason for hiding this comment

bcressey Sep 27, 2022

Choose a reason for hiding this comment

zmrow Sep 27, 2022 • edited

Choose a reason for hiding this comment

bcressey Sep 27, 2022

Choose a reason for hiding this comment

zmrow commented Sep 27, 2022

foersleo Sep 29, 2022

Choose a reason for hiding this comment

zmrow Sep 30, 2022

Choose a reason for hiding this comment

Support static addressing via `net.toml` #2445

Support static addressing via `net.toml` #2445

zmrow commented Sep 20, 2022 •

edited

zmrow Sep 27, 2022 •

edited