netdog: remove conditional compilation #3700

jmt-lab · 2024-01-06T00:16:03Z

Issue number:
Closes #3618

Removes conditional compilation of netdog for wicked and systemd networkd. This change moves our build system to build two variants of the netdog rpm: one for wicked and one for systemd-networkd. To facilitate this change the following was done:

Migrate #[cfg(net_backend = ] to #[cfg(feature =
Add two cargo features to sources/api/netdgo: 'wicked' and 'systemd-networkd'
Extract netdog to its own package folder (packages/netdog)
Extract netdog rpm building to netdog.spec
Create two subpackages one netdog-wicked and netdog-systemd-networkd which are selected via image-features
Migrated netdog to utilize the dogtag hostname detectors

Testing done:
Built and published aws-ecs-1 (wicked) and aws-ecs-2 (systemd-networkd) and verified network accessibility and use in a ecs cluster.

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.

packages/netdog/netdog.spec

sources/api/netdog/Cargo.toml

variants/aws-ecs-2/Cargo.toml

variants/aws-ecs-1-nvidia/Cargo.toml

cbgbt

Nice work! Please pardon any midair collisions with @sam-berning. I think we reviewed concurrently.

sources/api/netdog/Cargo.toml

packages/netdog/Cargo.toml

sources/api/netdog/Cargo.toml

packages/netdog/Cargo.lock

packages/settings-motd/Cargo.lock

packages/os/Cargo.lock

packages/netdog/netdog.spec

variants/aws-dev/Cargo.toml

packages/netdog/netdog.spec

packages/release/Cargo.lock

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

variants/aws-dev/Cargo.toml

packages/netdog/netdog.spec

packages/.gitignore

sources/api/netdog/Cargo.toml

jmt-lab · 2024-01-11T22:04:33Z

Rolled back to handling the binary split in the rpm spec. It is more than sufficient and attempts to split the binary at cargo level fell upon issues with how netdog is structured that were not minimal to address

jmt-lab · 2024-01-12T00:13:11Z

Reason for return of using rpmspec:

The reason for this was a bunch of roadblock updates i ran into with splitting the binaries out:

I first attempted a pretty straight forward split where i copied main.rs to two bin/networkd.rs and bin/wicked.rs files, this worked fine however leads to very large duplicate files since netdog currently re-uses its main.rs to set a large amount of static consts used throughout the crate
I then tried a hack by just symlinking main.rs twice to avoid cargo complaining about two targets using the same main.rs This did not propagate well into git and our ci cd
I then tried to do a lib and bin split where i took all the static const and most of the shared code to be in lib.rs and only moved the async main to the separated files, however this led to a collision with how netdog's multiple modules use private error messages with no shared top-type (meaning build error on visibility) this is fixable but would require alot ot making things public that i wasn't sure we wanted as such

As you can see above each approach i took to do the binary split at cargo time was bringing with it a bunch of more work when we had a functional solution. I decided to go back to a functional solution and file a github issue (which i need to do) for us to refactor netdog in the future to handle both this and how the cfgs are too integrated into too many source files

packages/netdog/Cargo.toml

packages/netdog/netdog.spec

packages/os/Cargo.toml

packages/netdog/netdog.spec

sources/api/netdog/src/main.rs

bcressey · 2024-01-12T00:44:36Z

sources/api/netdog/src/networkd/config/netdev.rs

@@ -142,7 +142,7 @@ impl Display for NetDevMacAddress {
 }

 impl NetDevConfig {
-    const FILE_EXT: &str = "netdev";
+    const FILE_EXT: &'static str = "netdev";


Why did this change?

This is a requirement of upgrated rust sdk. I'm surprised we aren't being riddled with more of these errors, but when the rust update landed as part of 1.18 into develop and i upgraded the PR it started flagging this as an error (previously was a warning) All module global consts need to have a static lifetime if using a type that requires lifetimes such as &str. It is no longer implicit

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

variants/aws-ecs-2/Cargo.toml

jmt-lab · 2024-01-19T19:08:43Z

fix version number and remove accidental file

variants/aws-ecs-2/Cargo.toml

packages/systemd/systemd.spec

sources/netdog/build.rs

sources/netdog/README.md

sources/netdog/src/cli/mod.rs

sources/netdog/src/networkd/config/netdev.rs

packages/netdog/Cargo.toml

packages/netdog/netdog.spec

jmt-lab · 2024-04-22T21:02:47Z

variants/aws-k8s-1.24/Cargo.toml

packages/netdog/Cargo.toml

sources/dogtag/src/lib.rs

sources/netdog/src/cli/generate_hostname.rs

packages/netdog/netdog.spec

jmt-lab · 2024-04-25T22:53:17Z

Updated dogtag to handle fallback to ip address

variants/metal-k8s-1.27/Cargo.toml

jmt-lab · 2024-04-25T23:47:32Z

Fix formatting

move shared files to common

…g to handle retry

packages/netdog/Cargo.toml

bcressey · 2024-04-29T18:12:42Z

packages/netdog/Cargo.toml

+[package.metadata.build-packages]
+source-groups = [
+    "netdog",
+    "dogtag",


nit: we could put dogtag under netdog (like how various update-related crates are under sources/updater) but that can be done later

sources/netdog/src/cli/generate_hostname.rs

packages/os/os.spec

packages/netdog/netdog.spec

jmt-lab · 2024-04-29T20:15:23Z

Rebased off develop
Bump netdog version
Adjust Requires
Add missing newline
Remove extra newline
Adjust comment

bcressey

🚢 🚢 🚢 🥅 🐶

jmt-lab marked this pull request as ready for review January 8, 2024 19:27

sam-berning reviewed Jan 9, 2024

View reviewed changes

packages/netdog/netdog.spec Show resolved Hide resolved

sources/api/netdog/Cargo.toml Outdated Show resolved Hide resolved

variants/aws-ecs-2/Cargo.toml Outdated Show resolved Hide resolved

variants/aws-ecs-1-nvidia/Cargo.toml Outdated Show resolved Hide resolved

jmt-lab force-pushed the jmt/netdog/conditional-removal branch from 3797d34 to 142ab64 Compare January 9, 2024 22:52

cbgbt reviewed Jan 9, 2024

View reviewed changes

jmt-lab force-pushed the jmt/netdog/conditional-removal branch 7 times, most recently from 3bce866 to 903e65d Compare January 10, 2024 00:43

yeazelm reviewed Jan 10, 2024

View reviewed changes

sources/api/netdog/src/cli/mod.rs Show resolved Hide resolved

variants/aws-dev/Cargo.toml Outdated Show resolved Hide resolved

packages/netdog/netdog.spec Outdated Show resolved Hide resolved

packages/.gitignore Outdated Show resolved Hide resolved

jmt-lab force-pushed the jmt/netdog/conditional-removal branch from 903e65d to d7373d6 Compare January 10, 2024 20:44

jmt-lab requested a review from cbgbt January 10, 2024 21:28

webern reviewed Jan 10, 2024

View reviewed changes

sources/api/netdog/Cargo.toml Outdated Show resolved Hide resolved

jmt-lab force-pushed the jmt/netdog/conditional-removal branch 4 times, most recently from e9ebd6f to 8d178ac Compare January 11, 2024 22:01

bcressey reviewed Jan 12, 2024

View reviewed changes

jmt-lab force-pushed the jmt/netdog/conditional-removal branch from 8d178ac to 20e59a5 Compare January 17, 2024 20:46

cbgbt mentioned this pull request Jan 17, 2024

settings: add network settings extension #3712

Draft

jmt-lab force-pushed the jmt/netdog/conditional-removal branch from 20e59a5 to a00c56f Compare January 19, 2024 19:08

bcressey mentioned this pull request Jan 25, 2024

Remove conditional compilation from early-boot-config #3637

Merged

12 tasks

webern mentioned this pull request Jan 26, 2024

Remove systemd-networkd from Manifest bottlerocket-os/twoliter#142

Open

webern reviewed Jan 26, 2024

View reviewed changes

jmt-lab force-pushed the jmt/netdog/conditional-removal branch from a00c56f to 4d95728 Compare February 1, 2024 20:43

jmt-lab force-pushed the jmt/netdog/conditional-removal branch from 66e7dc7 to 2d8d69f Compare April 22, 2024 21:00

bcressey reviewed Apr 23, 2024

View reviewed changes

jmt-lab force-pushed the jmt/netdog/conditional-removal branch 3 times, most recently from 999abe3 to 88cb74b Compare April 25, 2024 22:52

jmt-lab force-pushed the jmt/netdog/conditional-removal branch 2 times, most recently from a30c741 to 1f1c14b Compare April 25, 2024 23:09

sam-berning reviewed Apr 25, 2024

View reviewed changes

variants/metal-k8s-1.27/Cargo.toml Outdated Show resolved Hide resolved

jmt-lab force-pushed the jmt/netdog/conditional-removal branch 2 times, most recently from e93b898 to 8447e26 Compare April 25, 2024 23:47

jmt-lab force-pushed the jmt/netdog/conditional-removal branch from 8447e26 to de33087 Compare April 26, 2024 00:00

jmt-lab added 6 commits April 29, 2024 11:03

netdog: move netdog folder to sources root

f6a645c

netdog: adjust netdog to use cargo feature to switch back to wicked

1bd2237

netdog: adjust packages to utilize supplement

c56968b

move shared files to common

netdog: adjust all variant specifications

0e0f39d

netdog: add dogtag hostname for hostname resolution

19b7251

netdog: move to utilize dogtag for generate-hostname and update dogta…

48b8871

…g to handle retry

bcressey mentioned this pull request Apr 29, 2024

occasional build failures after extracting subpackages #3926

Closed

bcressey reviewed Apr 29, 2024

View reviewed changes

netdog: adjust variant files and symlink

e0b18ac

jmt-lab force-pushed the jmt/netdog/conditional-removal branch from de33087 to e0b18ac Compare April 29, 2024 20:14

bcressey approved these changes May 2, 2024

View reviewed changes

jmt-lab merged commit 8933289 into bottlerocket-os:develop May 2, 2024
33 checks passed

jmt-lab deleted the jmt/netdog/conditional-removal branch May 2, 2024 19:42

bcressey changed the title ~~Jmt/netdog/conditional removal~~ netdog: remove conditional compilation May 7, 2024

emmanuel-ferdman mentioned this pull request May 21, 2024

docs: update netdog reference #3974

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

netdog: remove conditional compilation #3700

netdog: remove conditional compilation #3700

jmt-lab commented Jan 6, 2024 •

edited by bcressey

cbgbt left a comment •

edited

jmt-lab commented Jan 11, 2024

jmt-lab commented Jan 12, 2024

bcressey Jan 12, 2024

jmt-lab Jan 12, 2024

jmt-lab commented Jan 19, 2024

jmt-lab commented Apr 22, 2024

jmt-lab commented Apr 25, 2024

jmt-lab commented Apr 25, 2024

bcressey Apr 29, 2024

jmt-lab commented Apr 29, 2024

bcressey left a comment

netdog: remove conditional compilation #3700

netdog: remove conditional compilation #3700

Conversation

jmt-lab commented Jan 6, 2024 • edited by bcressey

cbgbt left a comment • edited

Choose a reason for hiding this comment

jmt-lab commented Jan 11, 2024

jmt-lab commented Jan 12, 2024

bcressey Jan 12, 2024

Choose a reason for hiding this comment

jmt-lab Jan 12, 2024

Choose a reason for hiding this comment

jmt-lab commented Jan 19, 2024

jmt-lab commented Apr 22, 2024

jmt-lab commented Apr 25, 2024

jmt-lab commented Apr 25, 2024

bcressey Apr 29, 2024

Choose a reason for hiding this comment

jmt-lab commented Apr 29, 2024

bcressey left a comment

Choose a reason for hiding this comment

jmt-lab commented Jan 6, 2024 •

edited by bcressey

cbgbt left a comment •

edited