From 6e3fdb2697a19def30d3aea25a8f7f712bd3f560 Mon Sep 17 00:00:00 2001 From: karencfv Date: Mon, 11 Dec 2023 20:42:18 +1300 Subject: [PATCH 01/19] Skeleton for zone networking service --- Cargo.toml | 2 +- illumos-utils/src/ipadm.rs | 113 +++++++++++++++ illumos-utils/src/lib.rs | 2 + illumos-utils/src/route.rs | 47 +++++++ illumos-utils/src/zone.rs | 1 + sled-agent/src/bin/zone-networking-service.rs | 132 ++++++++++++++++++ 6 files changed, 296 insertions(+), 1 deletion(-) create mode 100644 illumos-utils/src/ipadm.rs create mode 100644 illumos-utils/src/route.rs create mode 100644 sled-agent/src/bin/zone-networking-service.rs diff --git a/Cargo.toml b/Cargo.toml index 5591dcebc9..c39ef55993 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -166,7 +166,7 @@ chacha20poly1305 = "0.10.1" ciborium = "0.2.1" cfg-if = "1.0" chrono = { version = "0.4", features = [ "serde" ] } -clap = { version = "4.4", features = ["derive", "env", "wrap_help"] } +clap = { version = "4.4", features = ["cargo", "derive", "env", "wrap_help"] } cookie = "0.18" criterion = { version = "0.5.1", features = [ "async_tokio" ] } crossbeam = "0.8" diff --git a/illumos-utils/src/ipadm.rs b/illumos-utils/src/ipadm.rs new file mode 100644 index 0000000000..fcc1cb41ca --- /dev/null +++ b/illumos-utils/src/ipadm.rs @@ -0,0 +1,113 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Utilities for managing IP interfaces. + +use std::net::Ipv6Addr; +// TODO: Make sure this is the correct location when running the binary +use crate::zone::IPADM; +use crate::{execute, ExecutionError, PFEXEC}; + +/// Wraps commands for interacting with interfaces. +pub struct Ipadm {} + +impl Ipadm { + // Remove current IP interface and create a new temporary one. + pub fn set_temp_interface_for_datalink( + datalink: &str, + ) -> Result<(), ExecutionError> { + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[IPADM, "delete-if", datalink]); + // First we remove IP interface if it already exists. If it doesn't + // exists and the command returns an error we continue anyway as + // the next step is to create it. + match execute(cmd) { + _ => (), + }; + + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[IPADM, "create-if", "-t", datalink]); + execute(cmd)?; + Ok(()) + } + + // Set MTU to 9000 on both IPv4 and IPv6 + pub fn set_interface_mtu(datalink: &str) -> Result<(), ExecutionError> { + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[ + IPADM, + "set-ifprop", + "-t", + "-p", + "mtu=9000", + "-m", + "ipv4", + datalink, + ]); + execute(cmd)?; + + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[ + IPADM, + "set-ifprop", + "-t", + "-p", + "mtu=9000", + "-m", + "ipv6", + datalink, + ]); + execute(cmd)?; + Ok(()) + } + + // TODO: Return a struct with the configured addresses + pub fn create_static_and_autoconfigured_addrs( + datalink: &str, + listen_addr: &Ipv6Addr, + ) -> Result<(), ExecutionError> { + // Create auto-configured address on the IP interface if it doesn't already exist + let addrobj = format!("{}/ll", datalink); + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]); + match execute(cmd) { + Err(_) => { + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[ + IPADM, + "create-addr", + "-t", + "-T", + "addrconf", + &addrobj, + ]); + execute(cmd)?; + } + Ok(_) => (), + }; + + // Create static address on the IP interface if it doesn't already exist + let addrobj = format!("{}/omicron6", datalink); + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]); + match execute(cmd) { + Err(_) => { + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[ + IPADM, + "create-addr", + "-t", + "-T", + "static", + "-a", + &listen_addr.to_string(), + &addrobj, + ]); + execute(cmd)?; + } + Ok(_) => (), + }; + Ok(()) + } +} diff --git a/illumos-utils/src/lib.rs b/illumos-utils/src/lib.rs index 1faa4c5c37..e35aa85908 100644 --- a/illumos-utils/src/lib.rs +++ b/illumos-utils/src/lib.rs @@ -16,9 +16,11 @@ pub mod dkio; pub mod dladm; pub mod dumpadm; pub mod fstyp; +pub mod ipadm; pub mod libc; pub mod link; pub mod opte; +pub mod route; pub mod running_zone; pub mod scf; pub mod svc; diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs new file mode 100644 index 0000000000..306511f913 --- /dev/null +++ b/illumos-utils/src/route.rs @@ -0,0 +1,47 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Utilities for manipulating the routing tables. + +use std::net::Ipv6Addr; +// TODO: Make sure this is the correct location when running the binary +use crate::zone::ROUTE; +use crate::{execute, ExecutionError, PFEXEC}; + +/// Wraps commands for interacting with routing tables. +pub struct Route {} + +impl Route { + pub fn ensure_default_route_with_gateway( + gateway: &Ipv6Addr, + ) -> Result<(), ExecutionError> { + // Add the desired route if it doesn't already exist + let destination = "default"; + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[ + ROUTE, + "get", + "-inet6", + destination, + "-inet6", + &gateway.to_string(), + ]); + match execute(cmd) { + Err(_) => { + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[ + ROUTE, + "add", + "-inet6", + destination, + "-inet6", + &gateway.to_string(), + ]); + execute(cmd)?; + } + Ok(_) => (), + }; + Ok(()) + } +} diff --git a/illumos-utils/src/zone.rs b/illumos-utils/src/zone.rs index a3f73b3954..3f749fc352 100644 --- a/illumos-utils/src/zone.rs +++ b/illumos-utils/src/zone.rs @@ -22,6 +22,7 @@ pub const IPADM: &str = "/usr/sbin/ipadm"; pub const SVCADM: &str = "/usr/sbin/svcadm"; pub const SVCCFG: &str = "/usr/sbin/svccfg"; pub const ZLOGIN: &str = "/usr/sbin/zlogin"; +pub const ROUTE: &str = "/usr/sbin/route"; // TODO: These could become enums pub const ZONE_PREFIX: &str = "oxz_"; diff --git a/sled-agent/src/bin/zone-networking-service.rs b/sled-agent/src/bin/zone-networking-service.rs new file mode 100644 index 0000000000..cb313079bc --- /dev/null +++ b/sled-agent/src/bin/zone-networking-service.rs @@ -0,0 +1,132 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Executable program to set up zone networking + +use anyhow::anyhow; +// use anyhow::bail; +// use anyhow::Context; +// use bytes::Buf; +// use bytes::BufMut; +// use bytes::BytesMut; +// use camino::Utf8PathBuf; +// use chrono::Local; +// use clap::Args; +// use clap::Parser; +// use clap::Subcommand; +use clap::{arg, command, Command}; +use illumos_utils::ipadm::Ipadm; +use illumos_utils::route::Route; +// use futures::stream::StreamExt; +use omicron_common::cmd::fatal; +use omicron_common::cmd::CmdError; +//use sled_agent_client::types::CleanupContextUpdate; +//use sled_agent_client::types::Duration; +//use sled_agent_client::types::PriorityDimension; +//use sled_agent_client::types::PriorityOrder; +//use sled_agent_client::Client; +//use slog::Drain; +use slog::Level; +// use slog::LevelFilter; +// use slog::Logger; +// use slog_term::FullFormat; +// use slog_term::TermDecorator; +// use std::collections::BTreeSet; +use std::net::Ipv6Addr; +//use std::time::SystemTime; +//use tar::Builder; +//use tar::Header; +//use tokio::io::AsyncWriteExt; +//use uuid::Uuid; + +// TODO: Set logger +fn _parse_log_level(s: &str) -> anyhow::Result { + s.parse().map_err(|_| anyhow!("Invalid log level")) +} + +fn parse_ipv6(s: &str) -> anyhow::Result { + s.parse().map_err(|_| anyhow!("Invalid IPv6 address")) +} + +#[tokio::main] +async fn main() { + if let Err(message) = do_run().await { + fatal(message); + } +} + +async fn do_run() -> Result<(), CmdError> { + let matches = command!() + .subcommand( + Command::new("ensure-if").about("TODO description").arg( + arg!( + -d --datalink "datalink" + ) + .required(true), + ), + ) + .subcommand( + Command::new("set-mtu").about("TODO description").arg( + arg!( + -d --datalink "datalink" + ) + .required(true), + ), + ) + .subcommand( + Command::new("set-addrs") + .about("TODO description") + .arg( + arg!( + -l --listen_addr "listen_addr" + ) + .required(true) + .value_parser(parse_ipv6), + ) + .arg( + arg!( + -d --datalink "datalink" + ) + .required(true), + ), + ) + .subcommand( + Command::new("add-route").about("TODO description").arg( + arg!( + -g --gateway "gateway" + ) + .required(true) + .value_parser(parse_ipv6), + ), + ) + .get_matches(); + + if let Some(matches) = matches.subcommand_matches("ensure-if") { + let datalink: &String = matches.get_one("datalink").unwrap(); + Ipadm::set_temp_interface_for_datalink(&datalink).unwrap(); + println!("Temporary interface '{:?}' set", datalink); + } + + if let Some(matches) = matches.subcommand_matches("set-mtu") { + let datalink: &String = matches.get_one("datalink").unwrap(); + Ipadm::set_interface_mtu(&datalink).unwrap(); + println!("MTU set to 9000 for IPv6 and IPv4 on: {:?}", datalink); + } + + if let Some(matches) = matches.subcommand_matches("set-addrs") { + let listen_addr: &Ipv6Addr = matches.get_one("listen_addr").unwrap(); + let datalink: &String = matches.get_one("datalink").unwrap(); + Ipadm::create_static_and_autoconfigured_addrs(&datalink, listen_addr) + .unwrap(); + println!("Static and auto-configured addresses set: {:?}", datalink); + } + + if let Some(matches) = matches.subcommand_matches("add-route") { + let gateway: &Ipv6Addr = matches.get_one("gateway").unwrap(); + Route::ensure_default_route_with_gateway(gateway).unwrap(); + println!("Default route with gateway '{:?}' set", gateway); + } + + Ok(()) +} From 7f1319cee12331ac6f0d17371aeebcea11ff1528 Mon Sep 17 00:00:00 2001 From: karencfv Date: Tue, 12 Dec 2023 14:31:48 +1300 Subject: [PATCH 02/19] Add binary to package manifest --- Cargo.lock | 13 +++++++ Cargo.toml | 2 + illumos-utils/src/ipadm.rs | 1 - package-manifest.toml | 12 +++++- zone-networking-cli/Cargo.toml | 14 +++++++ .../src/bin/zone-networking.rs | 37 +++---------------- 6 files changed, 45 insertions(+), 34 deletions(-) create mode 100644 zone-networking-cli/Cargo.toml rename sled-agent/src/bin/zone-networking-service.rs => zone-networking-cli/src/bin/zone-networking.rs (75%) diff --git a/Cargo.lock b/Cargo.lock index 7f966651a5..5c8bd9643f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10079,6 +10079,19 @@ dependencies = [ "zone_cfg_derive", ] +[[package]] +name = "zone-networking-cli" +version = "0.1.0" +dependencies = [ + "anyhow", + "clap 4.4.3", + "illumos-utils", + "omicron-common", + "omicron-workspace-hack", + "slog", + "tokio", +] + [[package]] name = "zone_cfg_derive" version = "0.3.0" diff --git a/Cargo.toml b/Cargo.toml index c39ef55993..33d6bef914 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,6 +68,7 @@ members = [ "wicket", "wicketd", "workspace-hack", + "zone-networking-cli", ] default-members = [ @@ -135,6 +136,7 @@ default-members = [ "wicket-dbg", "wicket", "wicketd", + "zone-networking-cli", ] resolver = "2" diff --git a/illumos-utils/src/ipadm.rs b/illumos-utils/src/ipadm.rs index fcc1cb41ca..fb0a9351dd 100644 --- a/illumos-utils/src/ipadm.rs +++ b/illumos-utils/src/ipadm.rs @@ -62,7 +62,6 @@ impl Ipadm { Ok(()) } - // TODO: Return a struct with the configured addresses pub fn create_static_and_autoconfigured_addrs( datalink: &str, listen_addr: &Ipv6Addr, diff --git a/package-manifest.toml b/package-manifest.toml index 8516a50e65..9e526d806e 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -161,7 +161,7 @@ setup_hint = "Run `./tools/ci_download_clickhouse` to download the necessary bin service_name = "cockroachdb" only_for_targets.image = "standard" source.type = "composite" -source.packages = [ "cockroachdb-service.tar.gz", "internal-dns-cli.tar.gz" ] +source.packages = [ "cockroachdb-service.tar.gz", "internal-dns-cli.tar.gz", "zone-networking-cli.tar.gz" ] output.type = "zone" [package.cockroachdb-service] @@ -601,3 +601,13 @@ source.packages = [ "sp-sim-softnpu.tar.gz" ] output.type = "zone" + +[package.zone-networking-cli] +service_name = "zone-networking-cli" +only_for_targets.image = "standard" +source.type = "local" +source.rust.binary_names = ["zone-networking"] +source.rust.release = true +source.paths = [] +output.type = "zone" +output.intermediate_only = true \ No newline at end of file diff --git a/zone-networking-cli/Cargo.toml b/zone-networking-cli/Cargo.toml new file mode 100644 index 0000000000..19ad6c009e --- /dev/null +++ b/zone-networking-cli/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "zone-networking-cli" +version = "0.1.0" +edition = "2021" +license = "MPL-2.0" + +[dependencies] +anyhow.workspace = true +clap.workspace = true +illumos-utils.workspace = true +omicron-common.workspace = true +slog.workspace = true +tokio.workspace = true +omicron-workspace-hack.workspace = true diff --git a/sled-agent/src/bin/zone-networking-service.rs b/zone-networking-cli/src/bin/zone-networking.rs similarity index 75% rename from sled-agent/src/bin/zone-networking-service.rs rename to zone-networking-cli/src/bin/zone-networking.rs index cb313079bc..649b156e50 100644 --- a/sled-agent/src/bin/zone-networking-service.rs +++ b/zone-networking-cli/src/bin/zone-networking.rs @@ -5,42 +5,15 @@ //! Executable program to set up zone networking use anyhow::anyhow; -// use anyhow::bail; -// use anyhow::Context; -// use bytes::Buf; -// use bytes::BufMut; -// use bytes::BytesMut; -// use camino::Utf8PathBuf; -// use chrono::Local; -// use clap::Args; -// use clap::Parser; -// use clap::Subcommand; use clap::{arg, command, Command}; use illumos_utils::ipadm::Ipadm; use illumos_utils::route::Route; -// use futures::stream::StreamExt; use omicron_common::cmd::fatal; use omicron_common::cmd::CmdError; -//use sled_agent_client::types::CleanupContextUpdate; -//use sled_agent_client::types::Duration; -//use sled_agent_client::types::PriorityDimension; -//use sled_agent_client::types::PriorityOrder; -//use sled_agent_client::Client; -//use slog::Drain; use slog::Level; -// use slog::LevelFilter; -// use slog::Logger; -// use slog_term::FullFormat; -// use slog_term::TermDecorator; -// use std::collections::BTreeSet; use std::net::Ipv6Addr; -//use std::time::SystemTime; -//use tar::Builder; -//use tar::Header; -//use tokio::io::AsyncWriteExt; -//use uuid::Uuid; -// TODO: Set logger +// TODO: Set logger? fn _parse_log_level(s: &str) -> anyhow::Result { s.parse().map_err(|_| anyhow!("Invalid log level")) } @@ -59,7 +32,7 @@ async fn main() { async fn do_run() -> Result<(), CmdError> { let matches = command!() .subcommand( - Command::new("ensure-if").about("TODO description").arg( + Command::new("ensure-if").about("Ensures a temporary IP interface is created with the given data link").arg( arg!( -d --datalink "datalink" ) @@ -67,7 +40,7 @@ async fn do_run() -> Result<(), CmdError> { ), ) .subcommand( - Command::new("set-mtu").about("TODO description").arg( + Command::new("set-mtu").about("Sets MTU to 9000 for IPv6 and IPv4 on the given data link").arg( arg!( -d --datalink "datalink" ) @@ -76,7 +49,7 @@ async fn do_run() -> Result<(), CmdError> { ) .subcommand( Command::new("set-addrs") - .about("TODO description") + .about("Ensures static and auto-configured addresses are set on the given data link") .arg( arg!( -l --listen_addr "listen_addr" @@ -92,7 +65,7 @@ async fn do_run() -> Result<(), CmdError> { ), ) .subcommand( - Command::new("add-route").about("TODO description").arg( + Command::new("add-route").about("Ensures there is a default route with the given gateway").arg( arg!( -g --gateway "gateway" ) From d458d50464fa95e4842d2dd6bb6db9c08adf20a9 Mon Sep 17 00:00:00 2001 From: karencfv Date: Tue, 12 Dec 2023 18:10:04 +1300 Subject: [PATCH 03/19] Try out with cockroachdb zone --- Cargo.lock | 1 + illumos-utils/src/ipadm.rs | 10 ++-- illumos-utils/src/route.rs | 4 +- smf/cockroachdb/method_script.sh | 16 +++--- zone-networking-cli/Cargo.toml | 1 + .../src/bin/zone-networking.rs | 50 +++++++++++-------- 6 files changed, 47 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5c8bd9643f..61c43f808f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10085,6 +10085,7 @@ version = "0.1.0" dependencies = [ "anyhow", "clap 4.4.3", + "dropshot", "illumos-utils", "omicron-common", "omicron-workspace-hack", diff --git a/illumos-utils/src/ipadm.rs b/illumos-utils/src/ipadm.rs index fb0a9351dd..6c4ae1255b 100644 --- a/illumos-utils/src/ipadm.rs +++ b/illumos-utils/src/ipadm.rs @@ -4,14 +4,14 @@ //! Utilities for managing IP interfaces. -use std::net::Ipv6Addr; -// TODO: Make sure this is the correct location when running the binary use crate::zone::IPADM; use crate::{execute, ExecutionError, PFEXEC}; +use std::net::Ipv6Addr; /// Wraps commands for interacting with interfaces. pub struct Ipadm {} +#[cfg_attr(any(test, feature = "testing"), mockall::automock, allow(dead_code))] impl Ipadm { // Remove current IP interface and create a new temporary one. pub fn set_temp_interface_for_datalink( @@ -20,11 +20,9 @@ impl Ipadm { let mut cmd = std::process::Command::new(PFEXEC); let cmd = cmd.args(&[IPADM, "delete-if", datalink]); // First we remove IP interface if it already exists. If it doesn't - // exists and the command returns an error we continue anyway as + // exist and the command returns an error we continue anyway as // the next step is to create it. - match execute(cmd) { - _ => (), - }; + let _ = execute(cmd); let mut cmd = std::process::Command::new(PFEXEC); let cmd = cmd.args(&[IPADM, "create-if", "-t", datalink]); diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs index 306511f913..060e2d3563 100644 --- a/illumos-utils/src/route.rs +++ b/illumos-utils/src/route.rs @@ -4,14 +4,14 @@ //! Utilities for manipulating the routing tables. -use std::net::Ipv6Addr; -// TODO: Make sure this is the correct location when running the binary use crate::zone::ROUTE; use crate::{execute, ExecutionError, PFEXEC}; +use std::net::Ipv6Addr; /// Wraps commands for interacting with routing tables. pub struct Route {} +#[cfg_attr(any(test, feature = "testing"), mockall::automock, allow(dead_code))] impl Route { pub fn ensure_default_route_with_gateway( gateway: &Ipv6Addr, diff --git a/smf/cockroachdb/method_script.sh b/smf/cockroachdb/method_script.sh index e5ab4e8eaa..261ff1c286 100755 --- a/smf/cockroachdb/method_script.sh +++ b/smf/cockroachdb/method_script.sh @@ -18,15 +18,17 @@ if [[ $DATALINK == unknown ]] || [[ $GATEWAY == unknown ]]; then fi # TODO remove when https://github.com/oxidecomputer/stlouis/issues/435 is addressed -ipadm delete-if "$DATALINK" || true -ipadm create-if -t "$DATALINK" +# Ensures a temporary IP interface is created with the given data link +/opt/oxide/zone-networking-cli/bin/zone-networking ensure-if -d $DATALINK -ipadm set-ifprop -t -p mtu=9000 -m ipv4 "$DATALINK" -ipadm set-ifprop -t -p mtu=9000 -m ipv6 "$DATALINK" +# Sets MTU to 9000 for IPv6 and IPv4 on the given data link +/opt/oxide/zone-networking-cli/bin/zone-networking set-mtu -d $DATALINK -ipadm show-addr "$DATALINK/ll" || ipadm create-addr -t -T addrconf "$DATALINK/ll" -ipadm show-addr "$DATALINK/omicron6" || ipadm create-addr -t -T static -a "$LISTEN_ADDR" "$DATALINK/omicron6" -route get -inet6 default -inet6 "$GATEWAY" || route add -inet6 default -inet6 "$GATEWAY" +# Sets static and auto-configured addresses on the given data link +/opt/oxide/zone-networking-cli/bin/zone-networking set-addrs -d $DATALINK -l $LISTEN_ADDR + +# Ensures there is a default route with the given gateway +/opt/oxide/zone-networking-cli/bin/zone-networking ensure-if -g $GATEWAY # We need to tell CockroachDB the DNS names or IP addresses of the other nodes # in the cluster. Look these up in internal DNS. Per the recommendations in diff --git a/zone-networking-cli/Cargo.toml b/zone-networking-cli/Cargo.toml index 19ad6c009e..c39f79104a 100644 --- a/zone-networking-cli/Cargo.toml +++ b/zone-networking-cli/Cargo.toml @@ -10,5 +10,6 @@ clap.workspace = true illumos-utils.workspace = true omicron-common.workspace = true slog.workspace = true +dropshot.workspace = true tokio.workspace = true omicron-workspace-hack.workspace = true diff --git a/zone-networking-cli/src/bin/zone-networking.rs b/zone-networking-cli/src/bin/zone-networking.rs index 649b156e50..f84346493a 100644 --- a/zone-networking-cli/src/bin/zone-networking.rs +++ b/zone-networking-cli/src/bin/zone-networking.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -//! Executable program to set up zone networking +//! CLI to set up zone networking use anyhow::anyhow; use clap::{arg, command, Command}; @@ -10,14 +10,9 @@ use illumos_utils::ipadm::Ipadm; use illumos_utils::route::Route; use omicron_common::cmd::fatal; use omicron_common::cmd::CmdError; -use slog::Level; +use slog::info; use std::net::Ipv6Addr; -// TODO: Set logger? -fn _parse_log_level(s: &str) -> anyhow::Result { - s.parse().map_err(|_| anyhow!("Invalid log level")) -} - fn parse_ipv6(s: &str) -> anyhow::Result { s.parse().map_err(|_| anyhow!("Invalid IPv6 address")) } @@ -30,6 +25,14 @@ async fn main() { } async fn do_run() -> Result<(), CmdError> { + let log = dropshot::ConfigLogging::File { + path: "/dev/stderr".into(), + level: dropshot::ConfigLoggingLevel::Info, + if_exists: dropshot::ConfigLoggingIfExists::Append, + } + .to_logger("zone-networking") + .map_err(|err| CmdError::Failure(anyhow!(err)))?; + let matches = command!() .subcommand( Command::new("ensure-if").about("Ensures a temporary IP interface is created with the given data link").arg( @@ -40,16 +43,17 @@ async fn do_run() -> Result<(), CmdError> { ), ) .subcommand( - Command::new("set-mtu").about("Sets MTU to 9000 for IPv6 and IPv4 on the given data link").arg( + Command::new("ensure-route").about("Ensures there is a default route with the given gateway").arg( arg!( - -d --datalink "datalink" + -g --gateway "gateway" ) - .required(true), + .required(true) + .value_parser(parse_ipv6), ), ) .subcommand( Command::new("set-addrs") - .about("Ensures static and auto-configured addresses are set on the given data link") + .about("Sets static and auto-configured addresses on the given data link") .arg( arg!( -l --listen_addr "listen_addr" @@ -65,39 +69,45 @@ async fn do_run() -> Result<(), CmdError> { ), ) .subcommand( - Command::new("add-route").about("Ensures there is a default route with the given gateway").arg( + Command::new("set-mtu").about("Sets MTU to 9000 for IPv6 and IPv4 on the given data link").arg( arg!( - -g --gateway "gateway" + -d --datalink "datalink" ) - .required(true) - .value_parser(parse_ipv6), + .required(true), ), ) .get_matches(); if let Some(matches) = matches.subcommand_matches("ensure-if") { let datalink: &String = matches.get_one("datalink").unwrap(); - Ipadm::set_temp_interface_for_datalink(&datalink).unwrap(); + info!(&log, "Ensuring a temporary IP interface is created"; "data link" => ?datalink); + Ipadm::set_temp_interface_for_datalink(&datalink) + .map_err(|err| CmdError::Failure(anyhow!(err)))?; println!("Temporary interface '{:?}' set", datalink); } if let Some(matches) = matches.subcommand_matches("set-mtu") { let datalink: &String = matches.get_one("datalink").unwrap(); - Ipadm::set_interface_mtu(&datalink).unwrap(); + Ipadm::set_interface_mtu(&datalink) + .map_err(|err| CmdError::Failure(anyhow!(err)))?; + info!(&log, "Setting MTU to 9000 for IPv6 and IPv4"; "data link" => ?datalink); println!("MTU set to 9000 for IPv6 and IPv4 on: {:?}", datalink); } if let Some(matches) = matches.subcommand_matches("set-addrs") { let listen_addr: &Ipv6Addr = matches.get_one("listen_addr").unwrap(); let datalink: &String = matches.get_one("datalink").unwrap(); + info!(&log, "Ensuring static and auto-configured addresses are set on the IP interface"; "data link" => ?datalink, "listen address" => ?listen_addr); Ipadm::create_static_and_autoconfigured_addrs(&datalink, listen_addr) - .unwrap(); + .map_err(|err| CmdError::Failure(anyhow!(err)))?; println!("Static and auto-configured addresses set: {:?}", datalink); } - if let Some(matches) = matches.subcommand_matches("add-route") { + if let Some(matches) = matches.subcommand_matches("ensure-route") { let gateway: &Ipv6Addr = matches.get_one("gateway").unwrap(); - Route::ensure_default_route_with_gateway(gateway).unwrap(); + info!(&log, "Ensuring there is a default route"; "gateway" => ?gateway); + Route::ensure_default_route_with_gateway(gateway) + .map_err(|err| CmdError::Failure(anyhow!(err)))?; println!("Default route with gateway '{:?}' set", gateway); } From 95bc7665f22b69733f7e9f824f0489de6c75f8d1 Mon Sep 17 00:00:00 2001 From: karencfv Date: Thu, 14 Dec 2023 12:55:20 +1300 Subject: [PATCH 04/19] Small fixes and validation improvements --- smf/cockroachdb/method_script.sh | 7 +----- workspace-hack/Cargo.toml | 8 +++--- .../src/bin/zone-networking.rs | 25 +++++++++++++------ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/smf/cockroachdb/method_script.sh b/smf/cockroachdb/method_script.sh index 261ff1c286..8c6cdd6162 100755 --- a/smf/cockroachdb/method_script.sh +++ b/smf/cockroachdb/method_script.sh @@ -12,11 +12,6 @@ DATASTORE="$(svcprop -c -p config/store "${SMF_FMRI}")" DATALINK="$(svcprop -c -p config/datalink "${SMF_FMRI}")" GATEWAY="$(svcprop -c -p config/gateway "${SMF_FMRI}")" -if [[ $DATALINK == unknown ]] || [[ $GATEWAY == unknown ]]; then - printf 'ERROR: missing datalink or gateway\n' >&2 - exit "$SMF_EXIT_ERR_CONFIG" -fi - # TODO remove when https://github.com/oxidecomputer/stlouis/issues/435 is addressed # Ensures a temporary IP interface is created with the given data link /opt/oxide/zone-networking-cli/bin/zone-networking ensure-if -d $DATALINK @@ -28,7 +23,7 @@ fi /opt/oxide/zone-networking-cli/bin/zone-networking set-addrs -d $DATALINK -l $LISTEN_ADDR # Ensures there is a default route with the given gateway -/opt/oxide/zone-networking-cli/bin/zone-networking ensure-if -g $GATEWAY +/opt/oxide/zone-networking-cli/bin/zone-networking ensure-route -g $GATEWAY # We need to tell CockroachDB the DNS names or IP addresses of the other nodes # in the cluster. Look these up in internal DNS. Per the recommendations in diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 3aff947fd3..40e2a84958 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -26,8 +26,8 @@ byteorder = { version = "1.5.0" } bytes = { version = "1.5.0", features = ["serde"] } chrono = { version = "0.4.31", features = ["alloc", "serde"] } cipher = { version = "0.4.4", default-features = false, features = ["block-padding", "zeroize"] } -clap = { version = "4.4.3", features = ["derive", "env", "wrap_help"] } -clap_builder = { version = "4.4.2", default-features = false, features = ["color", "env", "std", "suggestions", "usage", "wrap_help"] } +clap = { version = "4.4.3", features = ["cargo", "derive", "env", "wrap_help"] } +clap_builder = { version = "4.4.2", default-features = false, features = ["cargo", "color", "env", "std", "suggestions", "usage", "wrap_help"] } console = { version = "0.15.7" } const-oid = { version = "0.9.5", default-features = false, features = ["db", "std"] } crossbeam-epoch = { version = "0.9.15" } @@ -127,8 +127,8 @@ byteorder = { version = "1.5.0" } bytes = { version = "1.5.0", features = ["serde"] } chrono = { version = "0.4.31", features = ["alloc", "serde"] } cipher = { version = "0.4.4", default-features = false, features = ["block-padding", "zeroize"] } -clap = { version = "4.4.3", features = ["derive", "env", "wrap_help"] } -clap_builder = { version = "4.4.2", default-features = false, features = ["color", "env", "std", "suggestions", "usage", "wrap_help"] } +clap = { version = "4.4.3", features = ["cargo", "derive", "env", "wrap_help"] } +clap_builder = { version = "4.4.2", default-features = false, features = ["cargo", "color", "env", "std", "suggestions", "usage", "wrap_help"] } console = { version = "0.15.7" } const-oid = { version = "0.9.5", default-features = false, features = ["db", "std"] } crossbeam-epoch = { version = "0.9.15" } diff --git a/zone-networking-cli/src/bin/zone-networking.rs b/zone-networking-cli/src/bin/zone-networking.rs index f84346493a..bfe72ea19a 100644 --- a/zone-networking-cli/src/bin/zone-networking.rs +++ b/zone-networking-cli/src/bin/zone-networking.rs @@ -13,8 +13,18 @@ use omicron_common::cmd::CmdError; use slog::info; use std::net::Ipv6Addr; -fn parse_ipv6(s: &str) -> anyhow::Result { - s.parse().map_err(|_| anyhow!("Invalid IPv6 address")) +fn validate_ipv6(s: &str) -> anyhow::Result { + if s == "unknown" { + return Err(anyhow!("ERROR: Missing input value")); + }; + s.parse().map_err(|_| anyhow!("ERROR: Invalid IPv6 address")) +} + +fn validate_datalink(s: &str) -> anyhow::Result<()> { + if s == "unknown" { + return Err(anyhow!("ERROR: Missing data link")); + }; + Ok(()) } #[tokio::main] @@ -39,7 +49,8 @@ async fn do_run() -> Result<(), CmdError> { arg!( -d --datalink "datalink" ) - .required(true), + .required(true) + .value_parser(validate_datalink), ), ) .subcommand( @@ -48,7 +59,7 @@ async fn do_run() -> Result<(), CmdError> { -g --gateway "gateway" ) .required(true) - .value_parser(parse_ipv6), + .value_parser(validate_ipv6), ), ) .subcommand( @@ -59,13 +70,13 @@ async fn do_run() -> Result<(), CmdError> { -l --listen_addr "listen_addr" ) .required(true) - .value_parser(parse_ipv6), + .value_parser(validate_ipv6), ) .arg( arg!( -d --datalink "datalink" ) - .required(true), + .required(true).value_parser(validate_datalink), ), ) .subcommand( @@ -73,7 +84,7 @@ async fn do_run() -> Result<(), CmdError> { arg!( -d --datalink "datalink" ) - .required(true), + .required(true).value_parser(validate_datalink), ), ) .get_matches(); From 2b634acee39ee4e56003dcb8e85011813e7180b7 Mon Sep 17 00:00:00 2001 From: karencfv Date: Thu, 14 Dec 2023 14:44:09 +1300 Subject: [PATCH 05/19] I know this isn't the approach we're taking I just want the tests to pass --- zone-networking-cli/src/bin/zone-networking.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/zone-networking-cli/src/bin/zone-networking.rs b/zone-networking-cli/src/bin/zone-networking.rs index bfe72ea19a..57f83f3f39 100644 --- a/zone-networking-cli/src/bin/zone-networking.rs +++ b/zone-networking-cli/src/bin/zone-networking.rs @@ -13,18 +13,18 @@ use omicron_common::cmd::CmdError; use slog::info; use std::net::Ipv6Addr; -fn validate_ipv6(s: &str) -> anyhow::Result { +fn parse_ipv6(s: &str) -> anyhow::Result { if s == "unknown" { return Err(anyhow!("ERROR: Missing input value")); }; s.parse().map_err(|_| anyhow!("ERROR: Invalid IPv6 address")) } -fn validate_datalink(s: &str) -> anyhow::Result<()> { +fn parse_datalink(s: &str) -> anyhow::Result { if s == "unknown" { return Err(anyhow!("ERROR: Missing data link")); }; - Ok(()) + s.parse().map_err(|_| anyhow!("ERROR: Data link")) } #[tokio::main] @@ -50,7 +50,7 @@ async fn do_run() -> Result<(), CmdError> { -d --datalink "datalink" ) .required(true) - .value_parser(validate_datalink), + .value_parser(parse_datalink), ), ) .subcommand( @@ -59,7 +59,7 @@ async fn do_run() -> Result<(), CmdError> { -g --gateway "gateway" ) .required(true) - .value_parser(validate_ipv6), + .value_parser(parse_ipv6), ), ) .subcommand( @@ -70,13 +70,13 @@ async fn do_run() -> Result<(), CmdError> { -l --listen_addr "listen_addr" ) .required(true) - .value_parser(validate_ipv6), + .value_parser(parse_ipv6), ) .arg( arg!( -d --datalink "datalink" ) - .required(true).value_parser(validate_datalink), + .required(true).value_parser(parse_datalink), ), ) .subcommand( @@ -84,7 +84,7 @@ async fn do_run() -> Result<(), CmdError> { arg!( -d --datalink "datalink" ) - .required(true).value_parser(validate_datalink), + .required(true).value_parser(parse_datalink), ), ) .get_matches(); From c8e6ed054934179940d55c1c72377f8d375f469e Mon Sep 17 00:00:00 2001 From: karencfv Date: Fri, 15 Dec 2023 15:46:20 +1300 Subject: [PATCH 06/19] Initial sketch of manifest.xml file --- sled-agent/src/rack_setup/service.rs | 2 ++ sled-agent/src/services.rs | 2 ++ sled-agent/src/smf_helper.rs | 3 ++ smf/zone_network_setup/manifest.xml | 49 ++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+) create mode 100644 smf/zone_network_setup/manifest.xml diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index 8038658fb1..ddc9fb568c 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -1003,6 +1003,8 @@ impl ServiceInner { let version4_cockroachdb = version3_dns_and_ntp.next(); let version5_everything = version4_cockroachdb.next(); + // TODO: Probably want to set up zone networking service around here? + // Set up internal DNS services first and write the initial // DNS configuration to the internal DNS servers. let v1generator = OmicronZonesConfigGenerator::initial_version( diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 837c2a05df..ee6a548823 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -365,6 +365,8 @@ pub struct OmicronZoneConfigLocal { pub root: Utf8PathBuf, } +// TODO: I'll need something similar to this for the nw service + /// Describes how we want a switch zone to be configured /// /// This is analogous to `OmicronZoneConfig`, but for the switch zone (which is diff --git a/sled-agent/src/smf_helper.rs b/sled-agent/src/smf_helper.rs index d9ec4f02d6..922d897f36 100644 --- a/sled-agent/src/smf_helper.rs +++ b/sled-agent/src/smf_helper.rs @@ -2,6 +2,9 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. + +// TODO: I'll need these helpers for the nw service + use illumos_utils::running_zone::RunningZone; #[derive(thiserror::Error, Debug)] diff --git a/smf/zone_network_setup/manifest.xml b/smf/zone_network_setup/manifest.xml new file mode 100644 index 0000000000..d11764f56f --- /dev/null +++ b/smf/zone_network_setup/manifest.xml @@ -0,0 +1,49 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From c442fbd5297a3b221819b68080bdb052f73c713d Mon Sep 17 00:00:00 2001 From: karencfv Date: Wed, 20 Dec 2023 19:00:55 +1300 Subject: [PATCH 07/19] Add separate SMF service for zone network setup service --- package-manifest.toml | 18 ++- sled-agent/src/smf_helper.rs | 1 - smf/cockroachdb/method_script.sh | 14 +-- smf/zone_network_setup/method_script.sh | 13 +++ .../src/bin/zone-networking.rs | 108 ++++++------------ 5 files changed, 69 insertions(+), 85 deletions(-) create mode 100755 smf/zone_network_setup/method_script.sh diff --git a/package-manifest.toml b/package-manifest.toml index 9e526d806e..65c6f79835 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -161,6 +161,8 @@ setup_hint = "Run `./tools/ci_download_clickhouse` to download the necessary bin service_name = "cockroachdb" only_for_targets.image = "standard" source.type = "composite" +# TODO: Removeme once the zone networking setup service is functional -> "zone-networking-cli.tar.gz" +# TODO: Once service is ready add "zone_network_setup.tar.gz", source.packages = [ "cockroachdb-service.tar.gz", "internal-dns-cli.tar.gz", "zone-networking-cli.tar.gz" ] output.type = "zone" @@ -602,6 +604,7 @@ source.packages = [ ] output.type = "zone" +# TODO: Removeme once the zone networking setup service is functional [package.zone-networking-cli] service_name = "zone-networking-cli" only_for_targets.image = "standard" @@ -610,4 +613,17 @@ source.rust.binary_names = ["zone-networking"] source.rust.release = true source.paths = [] output.type = "zone" -output.intermediate_only = true \ No newline at end of file +output.intermediate_only = true + +[package.zone_network_setup] +service_name = "zone_network_setup" +only_for_targets.image = "standard" +source.type = "local" +source.rust.binary_names = ["zone-networking"] +source.rust.release = true +source.paths = [ + { from = "smf/zone_network_setup/manifest.xml", to = "/var/svc/manifest/site/zone_network_setup/manifest.xml" }, + { from = "smf/zone_network_setup/method_script.sh", to = "/opt/oxide/lib/svc/manifest/zone_network_setup.sh" }, +] +output.type = "zone" +output.intermediate_only = true diff --git a/sled-agent/src/smf_helper.rs b/sled-agent/src/smf_helper.rs index 922d897f36..a4ad6cf347 100644 --- a/sled-agent/src/smf_helper.rs +++ b/sled-agent/src/smf_helper.rs @@ -2,7 +2,6 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. - // TODO: I'll need these helpers for the nw service use illumos_utils::running_zone::RunningZone; diff --git a/smf/cockroachdb/method_script.sh b/smf/cockroachdb/method_script.sh index 8c6cdd6162..0566a6d5f7 100755 --- a/smf/cockroachdb/method_script.sh +++ b/smf/cockroachdb/method_script.sh @@ -12,18 +12,8 @@ DATASTORE="$(svcprop -c -p config/store "${SMF_FMRI}")" DATALINK="$(svcprop -c -p config/datalink "${SMF_FMRI}")" GATEWAY="$(svcprop -c -p config/gateway "${SMF_FMRI}")" -# TODO remove when https://github.com/oxidecomputer/stlouis/issues/435 is addressed -# Ensures a temporary IP interface is created with the given data link -/opt/oxide/zone-networking-cli/bin/zone-networking ensure-if -d $DATALINK - -# Sets MTU to 9000 for IPv6 and IPv4 on the given data link -/opt/oxide/zone-networking-cli/bin/zone-networking set-mtu -d $DATALINK - -# Sets static and auto-configured addresses on the given data link -/opt/oxide/zone-networking-cli/bin/zone-networking set-addrs -d $DATALINK -l $LISTEN_ADDR - -# Ensures there is a default route with the given gateway -/opt/oxide/zone-networking-cli/bin/zone-networking ensure-route -g $GATEWAY +# TODO: Removeme once the zone networking setup service is functional +/opt/oxide/zone-networking-cli/bin/zone-networking -d $DATALINK -l $LISTEN_ADDR -g $GATEWAY # We need to tell CockroachDB the DNS names or IP addresses of the other nodes # in the cluster. Look these up in internal DNS. Per the recommendations in diff --git a/smf/zone_network_setup/method_script.sh b/smf/zone_network_setup/method_script.sh new file mode 100755 index 0000000000..d872f0a227 --- /dev/null +++ b/smf/zone_network_setup/method_script.sh @@ -0,0 +1,13 @@ +#!/bin/bash + +set -x +set -o errexit +set -o pipefail + +. /lib/svc/share/smf_include.sh + +LISTEN_ADDR="$(svcprop -c -p config/listen_addr "${SMF_FMRI}")" +DATALINK="$(svcprop -c -p config/datalink "${SMF_FMRI}")" +GATEWAY="$(svcprop -c -p config/gateway "${SMF_FMRI}")" + +/opt/oxide/zone-networking-cli/bin/zone-networking -d $DATALINK -l $LISTEN_ADDR -g $GATEWAY diff --git a/zone-networking-cli/src/bin/zone-networking.rs b/zone-networking-cli/src/bin/zone-networking.rs index 57f83f3f39..3f965b2811 100644 --- a/zone-networking-cli/src/bin/zone-networking.rs +++ b/zone-networking-cli/src/bin/zone-networking.rs @@ -5,7 +5,7 @@ //! CLI to set up zone networking use anyhow::anyhow; -use clap::{arg, command, Command}; +use clap::{arg, command}; use illumos_utils::ipadm::Ipadm; use illumos_utils::route::Route; use omicron_common::cmd::fatal; @@ -24,7 +24,7 @@ fn parse_datalink(s: &str) -> anyhow::Result { if s == "unknown" { return Err(anyhow!("ERROR: Missing data link")); }; - s.parse().map_err(|_| anyhow!("ERROR: Data link")) + s.parse().map_err(|_| anyhow!("ERROR: Invalid data link")) } #[tokio::main] @@ -44,83 +44,49 @@ async fn do_run() -> Result<(), CmdError> { .map_err(|err| CmdError::Failure(anyhow!(err)))?; let matches = command!() - .subcommand( - Command::new("ensure-if").about("Ensures a temporary IP interface is created with the given data link").arg( - arg!( - -d --datalink "datalink" - ) - .required(true) - .value_parser(parse_datalink), - ), + .arg( + arg!( + -d --datalink "datalink" + ) + .required(true) + .value_parser(parse_datalink), ) - .subcommand( - Command::new("ensure-route").about("Ensures there is a default route with the given gateway").arg( - arg!( - -g --gateway "gateway" - ) - .required(true) - .value_parser(parse_ipv6), - ), + .arg( + arg!( + -g --gateway "gateway" + ) + .required(true) + .value_parser(parse_ipv6), ) - .subcommand( - Command::new("set-addrs") - .about("Sets static and auto-configured addresses on the given data link") - .arg( - arg!( - -l --listen_addr "listen_addr" - ) - .required(true) - .value_parser(parse_ipv6), - ) - .arg( - arg!( - -d --datalink "datalink" - ) - .required(true).value_parser(parse_datalink), - ), - ) - .subcommand( - Command::new("set-mtu").about("Sets MTU to 9000 for IPv6 and IPv4 on the given data link").arg( - arg!( - -d --datalink "datalink" - ) - .required(true).value_parser(parse_datalink), - ), + .arg( + arg!( + -l --listen_addr "listen_addr" + ) + .required(true) + .value_parser(parse_ipv6), ) .get_matches(); - if let Some(matches) = matches.subcommand_matches("ensure-if") { - let datalink: &String = matches.get_one("datalink").unwrap(); - info!(&log, "Ensuring a temporary IP interface is created"; "data link" => ?datalink); - Ipadm::set_temp_interface_for_datalink(&datalink) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; - println!("Temporary interface '{:?}' set", datalink); - } + let datalink: &String = matches.get_one("datalink").unwrap(); + let listen_addr: &Ipv6Addr = matches.get_one("listen_addr").unwrap(); + let gateway: &Ipv6Addr = matches.get_one("gateway").unwrap(); - if let Some(matches) = matches.subcommand_matches("set-mtu") { - let datalink: &String = matches.get_one("datalink").unwrap(); - Ipadm::set_interface_mtu(&datalink) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; - info!(&log, "Setting MTU to 9000 for IPv6 and IPv4"; "data link" => ?datalink); - println!("MTU set to 9000 for IPv6 and IPv4 on: {:?}", datalink); - } + // TODO: remove when https://github.com/oxidecomputer/stlouis/issues/435 is addressed + info!(&log, "Ensuring a temporary IP interface is created"; "data link" => ?datalink); + Ipadm::set_temp_interface_for_datalink(&datalink) + .map_err(|err| CmdError::Failure(anyhow!(err)))?; - if let Some(matches) = matches.subcommand_matches("set-addrs") { - let listen_addr: &Ipv6Addr = matches.get_one("listen_addr").unwrap(); - let datalink: &String = matches.get_one("datalink").unwrap(); - info!(&log, "Ensuring static and auto-configured addresses are set on the IP interface"; "data link" => ?datalink, "listen address" => ?listen_addr); - Ipadm::create_static_and_autoconfigured_addrs(&datalink, listen_addr) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; - println!("Static and auto-configured addresses set: {:?}", datalink); - } + info!(&log, "Setting MTU to 9000 for IPv6 and IPv4"; "data link" => ?datalink); + Ipadm::set_interface_mtu(&datalink) + .map_err(|err| CmdError::Failure(anyhow!(err)))?; - if let Some(matches) = matches.subcommand_matches("ensure-route") { - let gateway: &Ipv6Addr = matches.get_one("gateway").unwrap(); - info!(&log, "Ensuring there is a default route"; "gateway" => ?gateway); - Route::ensure_default_route_with_gateway(gateway) - .map_err(|err| CmdError::Failure(anyhow!(err)))?; - println!("Default route with gateway '{:?}' set", gateway); - } + info!(&log, "Ensuring static and auto-configured addresses are set on the IP interface"; "data link" => ?datalink, "listen address" => ?listen_addr); + Ipadm::create_static_and_autoconfigured_addrs(&datalink, listen_addr) + .map_err(|err| CmdError::Failure(anyhow!(err)))?; + + info!(&log, "Ensuring there is a default route"; "gateway" => ?gateway); + Route::ensure_default_route_with_gateway(gateway) + .map_err(|err| CmdError::Failure(anyhow!(err)))?; Ok(()) } From 2628105ff48f4da3519d8a847d148a9c8cb13885 Mon Sep 17 00:00:00 2001 From: karencfv Date: Wed, 20 Dec 2023 20:24:43 +1300 Subject: [PATCH 08/19] Network setup service install --- sled-agent/src/rack_setup/service.rs | 2 - sled-agent/src/services.rs | 52 +++++++++++++++++++++++++ smf/zone_network_setup/manifest.xml | 2 +- smf/zone_network_setup/method_script.sh | 2 + 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index ddc9fb568c..8038658fb1 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -1003,8 +1003,6 @@ impl ServiceInner { let version4_cockroachdb = version3_dns_and_ntp.next(); let version5_everything = version4_cockroachdb.next(); - // TODO: Probably want to set up zone networking service around here? - // Set up internal DNS services first and write the initial // DNS configuration to the internal DNS servers. let v1generator = OmicronZonesConfigGenerator::initial_version( diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index ee6a548823..1b0ee3ca35 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -1471,6 +1471,56 @@ impl ServiceManager { .install() .await?; + // Only for self assembling zones for now. Once all zones are self assembling + // There will be no need to apply selectively. + // + // TODO: Extract this into it's own function and pass only if + // it matches each of the self assembling zones + // NB: Only adding cockroach for now as it's just to test it out + #[allow(clippy::match_single_binding)] + match &request { + // TODO: Uncomment once I have removed all previous zone networking from the Cockroach db zone + // ZoneArgs::Omicron(OmicronZoneConfigLocal { + // zone: + // OmicronZoneConfig { + // zone_type: OmicronZoneType::CockroachDb { .. }, + // underlay_address, + // .. + // }, + // .. + // }) => { + // let Some(info) = self.inner.sled_info.get() else { + // return Err(Error::SledAgentNotReady); + // }; + // + // let datalink = installed_zone.get_control_vnic_name(); + // let gateway = &info.underlay_address.to_string(); + // let listen_addr = &underlay_address.to_string(); + // + // let mut config_builder = PropertyGroupBuilder::new("config"); + // config_builder = config_builder + // .add_property("datalink", "astring", datalink) + // .add_property("gateway", "astring", gateway) + // .add_property("listen_addr", "astring", listen_addr); + // let nw_setup = ServiceBuilder::new("oxide/zone_network_setup") + // .add_property_group(config_builder) + // .add_instance(ServiceInstanceBuilder::new("default")); + // // TODO: I'm unsure about the name here, should it be something other than "omicron"? + // let profile = + // ProfileBuilder::new("omicron").add_service(nw_setup); + // profile + // .add_to_zone(&self.inner.log, &installed_zone) + // .await + // .map_err(|err| { + // Error::io( + // "Failed to setup zone_network_setup profile", + // err, + // ) + // })?; + // } + _ => {} + } + // TODO(https://github.com/oxidecomputer/omicron/issues/1898): // // These zones are self-assembling -- after they boot, there should @@ -1497,6 +1547,8 @@ impl ServiceManager { let listen_port = &CLICKHOUSE_PORT.to_string(); let config = PropertyGroupBuilder::new("config") + // TODO: Once the nw setup service is running, we don't need to pass datalink or gateway + // to the individual zones .add_property("datalink", "astring", datalink) .add_property("gateway", "astring", gateway) .add_property("listen_addr", "astring", listen_addr) diff --git a/smf/zone_network_setup/manifest.xml b/smf/zone_network_setup/manifest.xml index d11764f56f..849688980e 100644 --- a/smf/zone_network_setup/manifest.xml +++ b/smf/zone_network_setup/manifest.xml @@ -16,7 +16,7 @@ - - + - + + + + + - - - - From aa394b0b3f970b92917efc2f7e319a1538b28342 Mon Sep 17 00:00:00 2001 From: karencfv Date: Tue, 9 Jan 2024 18:51:38 +1300 Subject: [PATCH 14/19] Verify route output status code before running second command --- illumos-utils/src/lib.rs | 2 +- illumos-utils/src/route.rs | 38 +++++++++++++++++++++++--------------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/illumos-utils/src/lib.rs b/illumos-utils/src/lib.rs index e35aa85908..550170b0f2 100644 --- a/illumos-utils/src/lib.rs +++ b/illumos-utils/src/lib.rs @@ -72,7 +72,7 @@ pub enum ExecutionError { mod inner { use super::*; - fn to_string(command: &mut std::process::Command) -> String { + pub fn to_string(command: &mut std::process::Command) -> String { command .get_args() .map(|s| s.to_string_lossy().into()) diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs index 6ebd75d3ee..0415a72efc 100644 --- a/illumos-utils/src/route.rs +++ b/illumos-utils/src/route.rs @@ -5,7 +5,7 @@ //! Utilities for manipulating the routing tables. use crate::zone::ROUTE; -use crate::{execute, ExecutionError, PFEXEC}; +use crate::{inner, execute, ExecutionError, PFEXEC, output_to_exec_error}; use std::net::Ipv6Addr; /// Wraps commands for interacting with routing tables. @@ -21,26 +21,34 @@ impl Route { let mut cmd = std::process::Command::new(PFEXEC); let cmd = cmd.args(&[ ROUTE, + "-n", "get", "-inet6", destination, "-inet6", &gateway.to_string(), ]); - match execute(cmd) { - Err(_) => { - let mut cmd = std::process::Command::new(PFEXEC); - let cmd = cmd.args(&[ - ROUTE, - "add", - "-inet6", - destination, - "-inet6", - &gateway.to_string(), - ]); - execute(cmd)?; - } - Ok(_) => (), + + let out = cmd.output().map_err(|err| { + ExecutionError::ExecutionStart { command: inner::to_string(cmd), err } + })?; + match out.status.code() { + Some(0) => (), + // If the entry is not found in the table, the exit status of the command is be 3 (ESRCH) + // When that is the case, we'll add the route. + Some(3) => { + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[ + ROUTE, + "add", + "-inet6", + destination, + "-inet6", + &gateway.to_string(), + ]); + execute(cmd)?; + } + Some(_) | None => return Err(output_to_exec_error(cmd, &out)), }; Ok(()) } From d18d4f3a01ce25416d0d79e0d76082bcbf45a2a0 Mon Sep 17 00:00:00 2001 From: karencfv Date: Tue, 9 Jan 2024 19:14:38 +1300 Subject: [PATCH 15/19] address review comments --- illumos-utils/src/route.rs | 32 ++++++++++--------- sled-agent/src/services.rs | 15 ++++----- smf/zone-network-setup/manifest.xml | 4 +-- smf/zone-network-setup/method_script.sh | 15 --------- zone-network-setup/src/bin/zone-networking.rs | 8 ++--- 5 files changed, 29 insertions(+), 45 deletions(-) delete mode 100755 smf/zone-network-setup/method_script.sh diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs index 0415a72efc..804b715018 100644 --- a/illumos-utils/src/route.rs +++ b/illumos-utils/src/route.rs @@ -5,7 +5,7 @@ //! Utilities for manipulating the routing tables. use crate::zone::ROUTE; -use crate::{inner, execute, ExecutionError, PFEXEC, output_to_exec_error}; +use crate::{execute, inner, output_to_exec_error, ExecutionError, PFEXEC}; use std::net::Ipv6Addr; /// Wraps commands for interacting with routing tables. @@ -29,25 +29,27 @@ impl Route { &gateway.to_string(), ]); - let out = cmd.output().map_err(|err| { - ExecutionError::ExecutionStart { command: inner::to_string(cmd), err } - })?; + let out = + cmd.output().map_err(|err| ExecutionError::ExecutionStart { + command: inner::to_string(cmd), + err, + })?; match out.status.code() { Some(0) => (), // If the entry is not found in the table, the exit status of the command is be 3 (ESRCH) // When that is the case, we'll add the route. Some(3) => { - let mut cmd = std::process::Command::new(PFEXEC); - let cmd = cmd.args(&[ - ROUTE, - "add", - "-inet6", - destination, - "-inet6", - &gateway.to_string(), - ]); - execute(cmd)?; - } + let mut cmd = std::process::Command::new(PFEXEC); + let cmd = cmd.args(&[ + ROUTE, + "add", + "-inet6", + destination, + "-inet6", + &gateway.to_string(), + ]); + execute(cmd)?; + } Some(_) | None => return Err(output_to_exec_error(cmd, &out)), }; Ok(()) diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 67a5b90825..50f5e97f1c 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -1378,10 +1378,10 @@ impl ServiceManager { .add_instance(ServiceInstanceBuilder::new("default"))) } - async fn zone_network_setup_install( + fn zone_network_setup_install( info: &SledAgentInfo, zone: &InstalledZone, - listen_addr: &String, + static_addr: &String, ) -> Result { let datalink = zone.get_control_vnic_name(); let gateway = &info.underlay_address.to_string(); @@ -1390,7 +1390,7 @@ impl ServiceManager { config_builder = config_builder .add_property("datalink", "astring", datalink) .add_property("gateway", "astring", gateway) - .add_property("listen_addr", "astring", listen_addr); + .add_property("static_addr", "astring", static_addr); Ok(ServiceBuilder::new("oxide/zone-network-setup") .add_property_group(config_builder) @@ -1517,8 +1517,7 @@ impl ServiceManager { info, &installed_zone, listen_addr, - ) - .await?; + )?; let dns_service = Self::dns_install(info).await?; @@ -1566,8 +1565,7 @@ impl ServiceManager { info, &installed_zone, listen_addr, - ) - .await?; + )?; let dns_service = Self::dns_install(info).await?; @@ -1622,8 +1620,7 @@ impl ServiceManager { info, &installed_zone, listen_addr, - ) - .await?; + )?; let dns_service = Self::dns_install(info).await?; diff --git a/smf/zone-network-setup/manifest.xml b/smf/zone-network-setup/manifest.xml index 4d1b7105b0..0776329749 100644 --- a/smf/zone-network-setup/manifest.xml +++ b/smf/zone-network-setup/manifest.xml @@ -18,7 +18,7 @@ @@ -28,7 +28,7 @@ - + diff --git a/smf/zone-network-setup/method_script.sh b/smf/zone-network-setup/method_script.sh deleted file mode 100755 index be83827ea1..0000000000 --- a/smf/zone-network-setup/method_script.sh +++ /dev/null @@ -1,15 +0,0 @@ -#!/bin/bash - -set -x -set -o errexit -set -o pipefail - -. /lib/svc/share/smf_include.sh - -LISTEN_ADDR="$(svcprop -c -p config/listen_addr "${SMF_FMRI}")" -DATALINK="$(svcprop -c -p config/datalink "${SMF_FMRI}")" -GATEWAY="$(svcprop -c -p config/gateway "${SMF_FMRI}")" - -/opt/oxide/zone-network-setup/bin/zone-networking -d $DATALINK -l $LISTEN_ADDR -g $GATEWAY - -exit $SMF_EXIT_OK \ No newline at end of file diff --git a/zone-network-setup/src/bin/zone-networking.rs b/zone-network-setup/src/bin/zone-networking.rs index 3f965b2811..b955ca856a 100644 --- a/zone-network-setup/src/bin/zone-networking.rs +++ b/zone-network-setup/src/bin/zone-networking.rs @@ -60,7 +60,7 @@ async fn do_run() -> Result<(), CmdError> { ) .arg( arg!( - -l --listen_addr "listen_addr" + -s --static_addr "static_addr" ) .required(true) .value_parser(parse_ipv6), @@ -68,7 +68,7 @@ async fn do_run() -> Result<(), CmdError> { .get_matches(); let datalink: &String = matches.get_one("datalink").unwrap(); - let listen_addr: &Ipv6Addr = matches.get_one("listen_addr").unwrap(); + let static_addr: &Ipv6Addr = matches.get_one("static_addr").unwrap(); let gateway: &Ipv6Addr = matches.get_one("gateway").unwrap(); // TODO: remove when https://github.com/oxidecomputer/stlouis/issues/435 is addressed @@ -80,8 +80,8 @@ async fn do_run() -> Result<(), CmdError> { Ipadm::set_interface_mtu(&datalink) .map_err(|err| CmdError::Failure(anyhow!(err)))?; - info!(&log, "Ensuring static and auto-configured addresses are set on the IP interface"; "data link" => ?datalink, "listen address" => ?listen_addr); - Ipadm::create_static_and_autoconfigured_addrs(&datalink, listen_addr) + info!(&log, "Ensuring static and auto-configured addresses are set on the IP interface"; "data link" => ?datalink, "static address" => ?static_addr); + Ipadm::create_static_and_autoconfigured_addrs(&datalink, static_addr) .map_err(|err| CmdError::Failure(anyhow!(err)))?; info!(&log, "Ensuring there is a default route"; "gateway" => ?gateway); From 8fc578ceae2f5a5db4388ca078e4ad9d611e6c82 Mon Sep 17 00:00:00 2001 From: karencfv Date: Wed, 10 Jan 2024 09:56:03 +1300 Subject: [PATCH 16/19] address comment --- package-manifest.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/package-manifest.toml b/package-manifest.toml index 1a588259d5..dd2890bc2d 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -622,7 +622,6 @@ source.rust.binary_names = ["zone-networking"] source.rust.release = true source.paths = [ { from = "smf/zone-network-setup/manifest.xml", to = "/var/svc/manifest/site/zone-network-setup/manifest.xml" }, - { from = "smf/zone-network-setup/method_script.sh", to = "/opt/oxide/lib/svc/manifest/zone-network-setup.sh" }, ] output.type = "zone" output.intermediate_only = true From 8b829b022f84494dbb73a52a8b4bf512dc588786 Mon Sep 17 00:00:00 2001 From: karencfv Date: Wed, 10 Jan 2024 11:46:19 +1300 Subject: [PATCH 17/19] fix bad grammar --- illumos-utils/src/route.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs index 804b715018..68116be32a 100644 --- a/illumos-utils/src/route.rs +++ b/illumos-utils/src/route.rs @@ -36,7 +36,8 @@ impl Route { })?; match out.status.code() { Some(0) => (), - // If the entry is not found in the table, the exit status of the command is be 3 (ESRCH) + // If the entry is not found in the table, + // the exit status of the command will be 3 (ESRCH). // When that is the case, we'll add the route. Some(3) => { let mut cmd = std::process::Command::new(PFEXEC); From b9545be98a66dc797f5d24972f5cc928a0090e8e Mon Sep 17 00:00:00 2001 From: karencfv Date: Wed, 10 Jan 2024 11:47:31 +1300 Subject: [PATCH 18/19] fmt --- illumos-utils/src/route.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs index 68116be32a..8d969d967d 100644 --- a/illumos-utils/src/route.rs +++ b/illumos-utils/src/route.rs @@ -36,7 +36,7 @@ impl Route { })?; match out.status.code() { Some(0) => (), - // If the entry is not found in the table, + // If the entry is not found in the table, // the exit status of the command will be 3 (ESRCH). // When that is the case, we'll add the route. Some(3) => { From c565a244e2ec067eca4f65ecdcb227fda556ed9d Mon Sep 17 00:00:00 2001 From: karencfv Date: Tue, 16 Jan 2024 10:24:17 +1300 Subject: [PATCH 19/19] Use libc's ESRCH constant for ensure_default_route_with_gateway() --- illumos-utils/src/route.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/illumos-utils/src/route.rs b/illumos-utils/src/route.rs index 8d969d967d..2b6af9a9fd 100644 --- a/illumos-utils/src/route.rs +++ b/illumos-utils/src/route.rs @@ -6,6 +6,7 @@ use crate::zone::ROUTE; use crate::{execute, inner, output_to_exec_error, ExecutionError, PFEXEC}; +use libc::ESRCH; use std::net::Ipv6Addr; /// Wraps commands for interacting with routing tables. @@ -39,7 +40,7 @@ impl Route { // If the entry is not found in the table, // the exit status of the command will be 3 (ESRCH). // When that is the case, we'll add the route. - Some(3) => { + Some(ESRCH) => { let mut cmd = std::process::Command::new(PFEXEC); let cmd = cmd.args(&[ ROUTE,