Skip to content

Commit

Permalink
zone-setup cleanup/refactoring (#6175)
Browse files Browse the repository at this point in the history
(This is a rebased and expanded version of #6015 now that the
self-assembling switch zone has landed.)

I need to make some changes to zone-setup to support Reconfigurator
rebalancing boundary NTP zones, and found it a little trickier to work
with than expected. I think there were two reasons for this:

* The use of clap's builder interface instead of derive (addressed in
3708774),
which requires a surprising amount of boilerplate and custom parsing
functions, most of which are now gone
* Using Result<(), CmdError::Failure> throughout, when really only main
cares about that and everywhere else could use anyhow::Result<()>
(mostly addressed in
cac26dd);
I also tried to make sure we always attach some .context() to errors
(the old code did this in some cases but not all)

I also made a cleanup pass through the new switch zone user setup code;
the biggest win there was reusing `illumos_utils::execute()` to check
for success and build detailed error messages on failure. There are some
other minor changes like long flags with different names (AFAICT we only
ever use the short flags in SMF manifests, so this shouldn't require
other changes) and potential bugfixes (e.g., the switch zone setup code
was only correct for 0 or 1 profile, and should now be correct for 2+ if
we ever need more than one).

While all these _should_ be correct, I want to run this through madrid
before merging. I think it's ready for review, though; will update here
once I do the madrid test.
  • Loading branch information
jgallagher authored Jul 31, 2024
1 parent 6f0340d commit 35a5041
Show file tree
Hide file tree
Showing 5 changed files with 543 additions and 754 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions illumos-utils/src/route.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
/// Wraps commands for interacting with routing tables.
pub struct Route {}

#[derive(Debug, Clone, Copy)]
pub enum Gateway {
Ipv4(Ipv4Addr),
Ipv6(Ipv6Addr),
Expand Down
4 changes: 3 additions & 1 deletion zone-setup/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,17 @@ workspace = true

[dependencies]
anyhow.workspace = true
camino.workspace = true
clap.workspace = true
dropshot.workspace = true
illumos-utils.workspace = true
omicron-common.workspace = true
omicron-workspace-hack.workspace = true
oxnet.workspace = true
serde_json.workspace = true
sled-hardware-types.workspace = true
slog.workspace = true
tokio.workspace = true
uzers.workspace = true
zone.workspace = true
omicron-sled-agent.workspace = true
omicron-sled-agent.workspace = true
Loading

0 comments on commit 35a5041

Please sign in to comment.