Skip to content

Commit

Permalink
[sled-agent] revert switch zone startup flow changes (#7345)
Browse files Browse the repository at this point in the history
This commit reverts the
[changes](https://github.com/oxidecomputer/omicron/pull/7260/files#diff-b8a6f13742cae29f44d095f6b9e8c2febc712e0ff86f01f3c8ec9d4e5d2db396)
made to the switch start up flow in #7260.

This had caused a race condition as can be seen in #7337 . Moving
forward, we'd like to return to using the `zone-network-setup` service,
but there needs to be more consideration to get the implementation
details right. In the mean time, let's get rid of this bug.

Fixes #7337
  • Loading branch information
karencfv authored Jan 15, 2025
1 parent 414318d commit 1c92838
Showing 1 changed file with 43 additions and 48 deletions.
91 changes: 43 additions & 48 deletions sled-agent/src/services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ use illumos_utils::running_zone::{
};
use illumos_utils::smf_helper::SmfHelper;
use illumos_utils::zfs::ZONE_ZFS_RAMDISK_DATASET_MOUNTPOINT;
use illumos_utils::zone::AddressRequest;
use illumos_utils::zpool::{PathInPool, ZpoolName};
use illumos_utils::{execute, PFEXEC};
use internal_dns_resolver::Resolver;
Expand Down Expand Up @@ -541,18 +542,6 @@ struct SwitchZoneConfigLocal {
root: Utf8PathBuf,
}

/// Service that sets up common networking across zones
struct ZoneNetworkSetupService {}

impl illumos_utils::smf_helper::Service for ZoneNetworkSetupService {
fn service_name(&self) -> String {
"zone-network-setup".to_string()
}
fn smf_name(&self) -> String {
format!("svc:/oxide/{}", self.service_name())
}
}

/// Describes either an Omicron-managed zone or the switch zone, used for
/// functions that operate on either one or the other
enum ZoneArgs<'a> {
Expand Down Expand Up @@ -4298,52 +4287,58 @@ impl ServiceManager {
);
*request = new_request;

// Add SMF properties here and restart zone-network-setup service
let first_address = request.addresses.get(0);
let address = first_address
.map(|addr| addr.to_string())
.unwrap_or_else(|| "".to_string());

// Set new properties for the network set up service and refresh
let nw_setup_svc = ZoneNetworkSetupService {};
let nsmfh = SmfHelper::new(&zone, &nw_setup_svc);

nsmfh.delpropvalue("config/gateway", "*")?;
nsmfh.delpropvalue("config/static_addr", "*")?;

if let Some(info) = self.inner.sled_info.get() {
nsmfh.addpropvalue_type(
"config/gateway",
&info.underlay_address,
"astring",
)?;
} else {
// It should be impossible for the `sled_info` not to be set here.
// When the request addresses have changed this means the underlay is
// available as well.
error!(
for addr in &request.addresses {
if *addr == Ipv6Addr::LOCALHOST {
continue;
}
info!(
self.inner.log,
concat!(
"sled agent info is not present,",
" even though underlay address exists"
)
"Ensuring address {} exists",
addr.to_string()
);
let addr_request =
AddressRequest::new_static(IpAddr::V6(*addr), None);
zone.ensure_address(addr_request).await?;
info!(
self.inner.log,
"Ensuring address {} exists - OK",
addr.to_string()
);
}

for address in &request.addresses {
if *address != Ipv6Addr::LOCALHOST {
nsmfh.addpropvalue_type(
"config/static_addr",
&address,
"astring",
)?;
}
// When the request addresses have changed this means the underlay is
// available now as well.
if let Some(info) = self.inner.sled_info.get() {
info!(
self.inner.log,
"Ensuring there is a default route";
"gateway" => ?info.underlay_address,
);
match zone.add_default_route(info.underlay_address).map_err(
|err| Error::ZoneCommand {
intent: "Adding Route".to_string(),
err,
},
) {
Ok(_) => (),
Err(e) => {
if e.to_string().contains("entry exists") {
info!(
self.inner.log,
"Default route already exists";
"gateway" => ?info.underlay_address,
)
} else {
return Err(e);
}
}
};
}
nsmfh.refresh()?;
info!(
self.inner.log,
"refreshed zone-network-setup service with new configuration"
);

for service in &request.services {
let smfh = SmfHelper::new(&zone, service);
Expand Down

0 comments on commit 1c92838

Please sign in to comment.