Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@karencfv
karencfv committed Jul 28, 2024
1 parent 6e2c54d commit c849460
Showing 1 changed file with 38 additions and 38 deletions.
76 changes: 38 additions & 38 deletions sled-agent/src/services.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ pub enum Error {
#[error("No U.2 devices found with a {ZONE_DATASET} mountpoint")]
U2NotFound,

#[error("Sled-local switch zone error: {0}")]
SledLocalSwitchZone(anyhow::Error),
#[error("Switch zone error: {0}")]
SwitchZone(anyhow::Error),

#[error("Failed to issue SMF command: {0}")]
SmfCommand(#[from] crate::smf_helper::Error),
Expand Down Expand Up @@ -209,8 +209,8 @@ pub enum Error {
#[error("Error contacting dpd: {0}")]
DpdError(#[from] DpdError<DpdTypes::Error>),

#[error("Failed to create Vnic in sled-local zone: {0}")]
SledLocalVnicCreation(illumos_utils::dladm::CreateVnicError),
#[error("Failed to create Vnic in the switch zone: {0}")]
SwitchZoneVnicCreation(illumos_utils::dladm::CreateVnicError),

#[error("Failed to add GZ addresses: {message}: {err}")]
GzAddress {
Expand Down Expand Up @@ -547,9 +547,9 @@ impl<'a> ZoneArgs<'a> {
}
}

/// If this is a sled-local (switch) zone, iterate over the services it's
/// If this is a switch zone, iterate over the services it's
/// supposed to be running
pub fn sled_local_services(
pub fn switch_zone_services(
&self,
) -> Box<dyn Iterator<Item = &'a SwitchService> + 'a> {
match self {
Expand Down Expand Up @@ -589,8 +589,8 @@ impl Task {
}
}

/// Describes the state of a sled-local switch zone.
enum SledLocalSwitchZoneState {
/// Describes the state of a switch zone.
enum SwitchZoneState {
// The zone is not currently running.
Disabled,
// The zone is still initializing - it may be awaiting the initialization
Expand Down Expand Up @@ -648,7 +648,7 @@ type ZoneMap = BTreeMap<String, OmicronZone>;
pub struct ServiceManagerInner {
log: Logger,
global_zone_bootstrap_link_local_address: Ipv6Addr,
switch_zone: Mutex<SledLocalSwitchZoneState>,
switch_zone: Mutex<SwitchZoneState>,
sled_mode: SledMode,
time_sync_config: TimeSyncConfig,
time_synced: AtomicBool,
Expand Down Expand Up @@ -730,7 +730,7 @@ impl ServiceManager {
.global_zone_bootstrap_link_local_ip,
// TODO(https://github.com/oxidecomputer/omicron/issues/725):
// Load the switch zone if it already exists?
switch_zone: Mutex::new(SledLocalSwitchZoneState::Disabled),
switch_zone: Mutex::new(SwitchZoneState::Disabled),
sled_mode,
time_sync_config,
time_synced: AtomicBool::new(false),
Expand Down Expand Up @@ -946,7 +946,7 @@ impl ServiceManager {
// physical devices need to be mapped into the zone when it is created.
fn devices_needed(zone_args: &ZoneArgs<'_>) -> Result<Vec<String>, Error> {
let mut devices = vec![];
for svc_details in zone_args.sled_local_services() {
for svc_details in zone_args.switch_zone_services() {
match svc_details {
SwitchService::Dendrite {
asic: DendriteAsic::TofinoAsic,
Expand Down Expand Up @@ -994,7 +994,7 @@ impl ServiceManager {
.inner
.bootstrap_vnic_allocator
.new_bootstrap()
.map_err(Error::SledLocalVnicCreation)?;
.map_err(Error::SwitchZoneVnicCreation)?;
Ok(Some((link, self.inner.switch_zone_bootstrap_address)))
} else {
Ok(None)
Expand Down Expand Up @@ -1030,7 +1030,7 @@ impl ServiceManager {
Error::Underlay(underlay::Error::SystemDetection(e))
})?;

for svc_details in zone_args.sled_local_services() {
for svc_details in zone_args.switch_zone_services() {
match &svc_details {
SwitchService::Tfport { pkt_source, asic: _ } => {
// The tfport service requires a MAC device to/from which
Expand Down Expand Up @@ -1254,7 +1254,7 @@ impl ServiceManager {
"dtrace_user".to_string(),
"dtrace_proc".to_string(),
];
for svc_details in zone_args.sled_local_services() {
for svc_details in zone_args.switch_zone_services() {
match svc_details {
SwitchService::Tfport { .. } => {
needed.push("sys_dl_config".to_string());
Expand Down Expand Up @@ -2534,7 +2534,7 @@ impl ServiceManager {
);
} else {
return Err(
Error::SledLocalSwitchZone(
Error::SwitchZone(
anyhow::anyhow!(
"{dev_cnt} devices needed \
for tofino asic"
Expand Down Expand Up @@ -3082,7 +3082,7 @@ impl ServiceManager {
name: &str,
) -> Result<ZoneBundleMetadata, BundleError> {
// Search for the named zone.
if let SledLocalSwitchZoneState::Running { zone, .. } =
if let SwitchZoneState::Running { zone, .. } =
&*self.inner.switch_zone.lock().await
{
if zone.name() == name {
Expand Down Expand Up @@ -3675,7 +3675,7 @@ impl ServiceManager {
// A pure gimlet sled should not be trying to activate a switch
// zone.
SledMode::Gimlet => {
return Err(Error::SledLocalSwitchZone(anyhow::anyhow!(
return Err(Error::SwitchZone(anyhow::anyhow!(
"attempted to activate switch zone on non-scrimlet sled"
)))
}
Expand Down Expand Up @@ -3811,15 +3811,15 @@ impl ServiceManager {
let mut switch_zone = self.inner.switch_zone.lock().await;

let zone = match &mut *switch_zone {
SledLocalSwitchZoneState::Running { zone, .. } => zone,
SledLocalSwitchZoneState::Disabled => {
return Err(Error::SledLocalSwitchZone(anyhow!(
SwitchZoneState::Running { zone, .. } => zone,
SwitchZoneState::Disabled => {
return Err(Error::SwitchZone(anyhow!(
"Cannot configure switch zone uplinks: \
switch zone disabled"
)));
}
SledLocalSwitchZoneState::Initializing { .. } => {
return Err(Error::SledLocalSwitchZone(anyhow!(
SwitchZoneState::Initializing { .. } => {
return Err(Error::SwitchZone(anyhow!(
"Cannot configure switch zone uplinks: \
switch zone still initializing"
)));
Expand Down Expand Up @@ -3868,13 +3868,13 @@ impl ServiceManager {
// This is a helper function for "ensure_switch_zone".
fn start_switch_zone(
self,
zone: &mut SledLocalSwitchZoneState,
zone: &mut SwitchZoneState,
request: SwitchZoneConfig,
filesystems: Vec<zone::Fs>,
data_links: Vec<String>,
) {
let (exit_tx, exit_rx) = oneshot::channel();
*zone = SledLocalSwitchZoneState::Initializing {
*zone = SwitchZoneState::Initializing {
request,
filesystems,
data_links,
Expand All @@ -3900,7 +3900,7 @@ impl ServiceManager {
let zone_typestr = "switch";

match (&mut *sled_zone, request) {
(SledLocalSwitchZoneState::Disabled, Some(request)) => {
(SwitchZoneState::Disabled, Some(request)) => {
info!(log, "Enabling {zone_typestr} zone (new)");
self.clone().start_switch_zone(
&mut sled_zone,
Expand All @@ -3910,7 +3910,7 @@ impl ServiceManager {
);
}
(
SledLocalSwitchZoneState::Initializing { request, .. },
SwitchZoneState::Initializing { request, .. },
Some(new_request),
) => {
info!(log, "Enabling {zone_typestr} zone (already underway)");
Expand All @@ -3919,7 +3919,7 @@ impl ServiceManager {
*request = new_request;
}
(
SledLocalSwitchZoneState::Running { request, zone },
SwitchZoneState::Running { request, zone },
Some(new_request),
) if request.addresses != new_request.addresses => {
// If the switch zone is running but we have new addresses, it
Expand Down Expand Up @@ -4231,31 +4231,31 @@ impl ServiceManager {
}
}
}
(SledLocalSwitchZoneState::Running { .. }, Some(_)) => {
(SwitchZoneState::Running { .. }, Some(_)) => {
info!(log, "Enabling {zone_typestr} zone (already complete)");
}
(SledLocalSwitchZoneState::Disabled, None) => {
(SwitchZoneState::Disabled, None) => {
info!(log, "Disabling {zone_typestr} zone (already complete)");
}
(SledLocalSwitchZoneState::Initializing { worker, .. }, None) => {
(SwitchZoneState::Initializing { worker, .. }, None) => {
info!(log, "Disabling {zone_typestr} zone (was initializing)");
worker.take().unwrap().stop().await;
*sled_zone = SledLocalSwitchZoneState::Disabled;
*sled_zone = SwitchZoneState::Disabled;
}
(SledLocalSwitchZoneState::Running { zone, .. }, None) => {
(SwitchZoneState::Running { zone, .. }, None) => {
info!(log, "Disabling {zone_typestr} zone (was running)");
let _ = zone.stop().await;
*sled_zone = SledLocalSwitchZoneState::Disabled;
*sled_zone = SwitchZoneState::Disabled;
}
}
Ok(())
}

async fn try_initialize_sled_local_switch_zone(
async fn try_initialize_switch_zone(
&self,
sled_zone: &mut SledLocalSwitchZoneState,
sled_zone: &mut SwitchZoneState,
) -> Result<(), Error> {
let SledLocalSwitchZoneState::Initializing {
let SwitchZoneState::Initializing {
request,
filesystems,
data_links,
Expand All @@ -4279,7 +4279,7 @@ impl ServiceManager {
let zone = self
.initialize_zone(zone_args, filesystems, data_links, None)
.await?;
*sled_zone = SledLocalSwitchZoneState::Running {
*sled_zone = SwitchZoneState::Running {
request: request.clone(),
zone,
};
Expand All @@ -4296,7 +4296,7 @@ impl ServiceManager {
{
let mut sled_zone = self.inner.switch_zone.lock().await;
match self
.try_initialize_sled_local_switch_zone(&mut sled_zone)
.try_initialize_switch_zone(&mut sled_zone)
.await
{
Ok(()) => return,
Expand Down

0 comments on commit c849460

Please sign in to comment.