Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api: clean up Instance and InstanceProperties types #803

Merged
merged 4 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions bin/mock-server/src/lib/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,6 @@ async fn instance_get(
let instance_info = api::Instance {
properties: instance.properties.clone(),
state: instance.state,
disks: vec![],
nics: vec![],
};
Ok(HttpResponseOk(api::InstanceGetResponse { instance: instance_info }))
}
Expand Down
19 changes: 9 additions & 10 deletions bin/propolis-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use anyhow::{anyhow, Context};
use clap::{Parser, Subcommand};
use futures::{future, SinkExt};
use newtype_uuid::{GenericUuid, TypedUuid, TypedUuidKind, TypedUuidTag};
use propolis_client::types::InstanceMetadata;
use propolis_client::types::{InstanceMetadata, VersionedInstanceSpec};
use slog::{o, Drain, Level, Logger};
use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio_tungstenite::tungstenite::{
Expand Down Expand Up @@ -253,16 +253,12 @@ async fn new_instance(
name,
description: "propolis-cli generated instance".to_string(),
metadata,
// TODO: Use real UUID
image_id: Uuid::default(),
// TODO: Use real UUID
bootrom_id: Uuid::default(),
memory,
vcpus,
};

let request = InstanceEnsureRequest {
properties,
vcpus,
memory,
// TODO: Allow specifying NICs
nics: vec![],
disks,
Expand Down Expand Up @@ -504,17 +500,20 @@ async fn migrate_instance(
) -> anyhow::Result<()> {
// Grab the instance details
let src_instance =
src_client.instance_get().send().await.with_context(|| {
src_client.instance_spec_get().send().await.with_context(|| {
anyhow!("failed to get src instance properties")
})?;
let src_uuid = src_instance.instance.properties.id;
let src_uuid = src_instance.properties.id;
let VersionedInstanceSpec::V0(spec) = &src_instance.spec;

let request = InstanceEnsureRequest {
properties: InstanceProperties {
// Use a new ID for the destination instance we're creating
id: dst_uuid,
..src_instance.instance.properties.clone()
..src_instance.properties.clone()
},
vcpus: spec.board.cpus,
memory: spec.board.memory_mb,
// TODO: Handle migrating NICs
nics: vec![],
disks,
Expand Down
13 changes: 7 additions & 6 deletions bin/propolis-server/src/lib/initializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ pub struct MachineInitializer<'a> {
pub(crate) producer_registry: Option<ProducerRegistry>,
pub(crate) state: MachineInitializerState,
pub(crate) kstat_sampler: Option<KstatSampler>,
pub(crate) stats_vm: crate::stats::VirtualMachine,
}

impl<'a> MachineInitializer<'a> {
Expand Down Expand Up @@ -744,7 +745,7 @@ impl<'a> MachineInitializer<'a> {
track_network_interface_kstats(
&self.log,
sampler,
self.properties,
&self.stats_vm,
interface_ids.unwrap(),
)
.await
Expand Down Expand Up @@ -1031,15 +1032,15 @@ impl<'a> MachineInitializer<'a> {
// unknown
proc_upgrade: 0x2,
// make core and thread counts equal for now
core_count: self.properties.vcpus,
core_enabled: self.properties.vcpus,
thread_count: self.properties.vcpus,
core_count: self.spec.board.cpus,
core_enabled: self.spec.board.cpus,
thread_count: self.spec.board.cpus,
proc_characteristics: type4::Characteristics::IS_64_BIT
| type4::Characteristics::MULTI_CORE,
..Default::default()
};

let memsize_bytes = (self.properties.memory as usize) * MB;
let memsize_bytes = (self.spec.board.memory_mb as usize) * MB;
let mut smb_type16 = smbios::table::Type16 {
location: type16::Location::SystemBoard,
array_use: type16::ArrayUse::System,
Expand Down Expand Up @@ -1228,7 +1229,7 @@ impl<'a> MachineInitializer<'a> {
self.devices.insert(format!("vcpu-{}", vcpu.id), vcpu.clone());
}
if let Some(sampler) = self.kstat_sampler.as_ref() {
track_vcpu_kstats(&self.log, sampler, self.properties).await;
track_vcpu_kstats(&self.log, sampler, &self.stats_vm).await;
}
Ok(())
}
Expand Down
4 changes: 1 addition & 3 deletions bin/propolis-server/src/lib/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ fn instance_spec_from_request(
request: &api::InstanceEnsureRequest,
toml_config: &VmTomlConfig,
) -> Result<Spec, SpecBuilderError> {
let mut spec_builder = SpecBuilder::new(&request.properties);
let mut spec_builder = SpecBuilder::new(request.vcpus, request.memory);

spec_builder.add_devices_from_config(toml_config)?;

Expand Down Expand Up @@ -376,8 +376,6 @@ async fn instance_get(
instance: api::Instance {
properties: full.properties,
state: full.state,
disks: vec![],
nics: vec![],
},
})
})
Expand Down
32 changes: 6 additions & 26 deletions bin/propolis-server/src/lib/spec/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use propolis_api_types::{
},
PciPath,
},
DiskRequest, InstanceProperties, NetworkInterfaceRequest,
DiskRequest, NetworkInterfaceRequest,
};
use thiserror::Error;

Expand Down Expand Up @@ -78,10 +78,10 @@ pub(crate) struct SpecBuilder {
}

impl SpecBuilder {
pub fn new(properties: &InstanceProperties) -> Self {
pub fn new(cpus: u8, memory_mb: u64) -> Self {
let board = Board {
cpus: properties.vcpus,
memory_mb: properties.memory,
cpus,
memory_mb,
chipset: Chipset::I440Fx(I440Fx { enable_pcie: false }),
};

Expand Down Expand Up @@ -455,36 +455,16 @@ mod test {
backends::{BlobStorageBackend, VirtioNetworkBackend},
devices::{VirtioDisk, VirtioNic},
},
InstanceMetadata, Slot, VolumeConstructionRequest,
Slot, VolumeConstructionRequest,
};
use uuid::Uuid;

use crate::spec::{StorageBackend, StorageDevice};

use super::*;

fn test_metadata() -> InstanceMetadata {
InstanceMetadata {
silo_id: uuid::uuid!("556a67f8-8b14-4659-bd9f-d8f85ecd36bf"),
project_id: uuid::uuid!("75f60038-daeb-4a1d-916a-5fa5b7237299"),
sled_id: uuid::uuid!("43a789ac-a0dd-4e1e-ac33-acdada142faa"),
sled_serial: "some-gimlet".into(),
sled_revision: 1,
sled_model: "abcd".into(),
}
}

fn test_builder() -> SpecBuilder {
SpecBuilder::new(&InstanceProperties {
id: Default::default(),
name: Default::default(),
description: Default::default(),
metadata: test_metadata(),
image_id: Default::default(),
bootrom_id: Default::default(),
memory: 512,
vcpus: 4,
})
SpecBuilder::new(4, 512)
}

#[test]
Expand Down
17 changes: 7 additions & 10 deletions bin/propolis-server/src/lib/stats/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use oximeter::{
};
use oximeter_instruments::kstat::KstatSampler;
use oximeter_producer::{Config, Error, Server};
use propolis_api_types::InstanceProperties;
use slog::Logger;
use uuid::Uuid;

Expand Down Expand Up @@ -188,11 +187,10 @@ pub fn start_oximeter_server(
/// Create an object that can be used to sample kstat-based metrics.
pub(crate) fn create_kstat_sampler(
log: &Logger,
properties: &InstanceProperties,
spec: &Spec,
) -> Option<KstatSampler> {
let kstat_limit = usize::try_from(
(u32::from(properties.vcpus) * KSTAT_LIMIT_PER_VCPU)
(u32::from(spec.board.cpus) * KSTAT_LIMIT_PER_VCPU)
+ (spec.nics.len() as u32 * SAMPLE_BUFFER),
)
.unwrap();
Expand All @@ -216,7 +214,7 @@ pub(crate) fn create_kstat_sampler(
pub(crate) async fn track_vcpu_kstats(
log: &Logger,
_: &KstatSampler,
_: &InstanceProperties,
_: &VirtualMachine,
) {
slog::error!(log, "vCPU stats are not supported on this platform");
}
Expand All @@ -226,13 +224,12 @@ pub(crate) async fn track_vcpu_kstats(
pub(crate) async fn track_vcpu_kstats(
log: &Logger,
sampler: &KstatSampler,
properties: &InstanceProperties,
virtual_machine: &VirtualMachine,
) {
let virtual_machine = VirtualMachine::from(properties);
let details = oximeter_instruments::kstat::CollectionDetails::never(
VCPU_KSTAT_INTERVAL,
);
if let Err(e) = sampler.add_target(virtual_machine, details).await {
if let Err(e) = sampler.add_target(virtual_machine.clone(), details).await {
slog::error!(
log,
"failed to add VirtualMachine target, \
Expand All @@ -247,7 +244,7 @@ pub(crate) async fn track_vcpu_kstats(
pub(crate) async fn track_network_interface_kstats(
log: &Logger,
_: &KstatSampler,
_: &InstanceProperties,
_: &VirtualMachine,
_: NetworkInterfaceIds,
) {
slog::error!(
Expand All @@ -261,10 +258,10 @@ pub(crate) async fn track_network_interface_kstats(
pub(crate) async fn track_network_interface_kstats(
log: &Logger,
sampler: &KstatSampler,
properties: &InstanceProperties,
virtual_machine: &VirtualMachine,
interface_ids: NetworkInterfaceIds,
) {
let nics = InstanceNetworkInterfaces::new(properties, &interface_ids);
let nics = InstanceNetworkInterfaces::new(virtual_machine, &interface_ids);
let details = oximeter_instruments::kstat::CollectionDetails::never(
NETWORK_INTERFACE_SAMPLE_INTERVAL,
);
Expand Down
8 changes: 4 additions & 4 deletions bin/propolis-server/src/lib/stats/network_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl InstanceNetworkInterfaces {
/// metrics from.
#[cfg(all(not(test), target_os = "illumos"))]
pub(crate) fn new(
properties: &propolis_api_types::InstanceProperties,
virtual_machine: &super::VirtualMachine,
interface_ids: &NetworkInterfaceIds,
) -> Self {
Self {
Expand All @@ -160,9 +160,9 @@ impl InstanceNetworkInterfaces {
// multiple targets in the `to_samples` method and override
// this.
interface_id: uuid::Uuid::nil(),
instance_id: properties.id,
project_id: properties.metadata.project_id,
silo_id: properties.metadata.silo_id,
instance_id: virtual_machine.target.instance_id,
project_id: virtual_machine.target.project_id,
silo_id: virtual_machine.target.silo_id,
},
interface_ids: interface_ids.to_vec(),
}
Expand Down
9 changes: 6 additions & 3 deletions bin/propolis-server/src/lib/stats/virtual_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,11 @@ pub struct VirtualMachine {
vm_name: String,
}

impl From<&propolis_api_types::InstanceProperties> for VirtualMachine {
fn from(properties: &propolis_api_types::InstanceProperties) -> Self {
impl VirtualMachine {
pub fn new(
n_vcpus: u8,
properties: &propolis_api_types::InstanceProperties,
) -> Self {
Self {
target: VirtualMachineTarget {
silo_id: properties.metadata.silo_id,
Expand All @@ -73,7 +76,7 @@ impl From<&propolis_api_types::InstanceProperties> for VirtualMachine {
sled_revision: properties.metadata.sled_revision,
sled_serial: properties.metadata.sled_serial.clone().into(),
},
n_vcpus: properties.vcpus.into(),
n_vcpus: u32::from(n_vcpus),
vm_name: properties.vm_name(),
}
}
Expand Down
12 changes: 7 additions & 5 deletions bin/propolis-server/src/lib/vm/ensure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use crate::{
build_instance, MachineInitializer, MachineInitializerState,
},
spec::Spec,
stats::create_kstat_sampler,
stats::{create_kstat_sampler, VirtualMachine},
vm::request_queue::InstanceAutoStart,
};

Expand Down Expand Up @@ -187,10 +187,10 @@ impl<'a> VmEnsureNotStarted<'a> {
state: MachineInitializerState::default(),
kstat_sampler: initialize_kstat_sampler(
self.log,
properties,
self.instance_spec(),
options.oximeter_registry.clone(),
),
stats_vm: VirtualMachine::new(spec.board.cpus, properties),
};

init.initialize_rom(options.toml_config.bootrom.as_path())?;
Expand All @@ -205,7 +205,10 @@ impl<'a> VmEnsureNotStarted<'a> {
let com1 = Arc::new(init.initialize_uart(&chipset));
let ps2ctrl = init.initialize_ps2(&chipset);
init.initialize_qemu_debug_port()?;
init.initialize_qemu_pvpanic(properties.into())?;
init.initialize_qemu_pvpanic(VirtualMachine::new(
self.instance_spec().board.cpus,
properties,
))?;
init.initialize_network_devices(&chipset).await?;

#[cfg(not(feature = "omicron-build"))]
Expand Down Expand Up @@ -378,12 +381,11 @@ impl<'a> VmEnsureActive<'a> {
/// Create an object used to sample kstats.
fn initialize_kstat_sampler(
log: &slog::Logger,
properties: &InstanceProperties,
spec: &Spec,
producer_registry: Option<ProducerRegistry>,
) -> Option<KstatSampler> {
let registry = producer_registry?;
let sampler = create_kstat_sampler(log, properties, spec)?;
let sampler = create_kstat_sampler(log, spec)?;

match registry.register_producer(sampler.clone()) {
Ok(_) => Some(sampler),
Expand Down
15 changes: 12 additions & 3 deletions bin/propolis-server/src/lib/vm/services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use slog::{error, info, Logger};
use crate::{
serial::SerialTaskControlMessage,
server::MetricsEndpointConfig,
spec::Spec,
stats::{ServerStats, VirtualMachine},
vnc::VncServer,
};
Expand Down Expand Up @@ -56,16 +57,23 @@ impl VmServices {
vm_properties: &InstanceProperties,
ensure_options: &super::EnsureOptions,
) -> Self {
let vm_objects = vm_objects.lock_shared().await;
let oximeter_state = if let Some(cfg) = &ensure_options.metrics_config {
let registry = ensure_options.oximeter_registry.as_ref().expect(
"should have a producer registry if metrics are configured",
);
register_oximeter_producer(log, cfg, registry, vm_properties).await
register_oximeter_producer(
log,
cfg,
registry,
vm_objects.instance_spec(),
vm_properties,
)
.await
} else {
OximeterState::default()
};

let vm_objects = vm_objects.lock_shared().await;
let vnc_server = ensure_options.vnc_server.clone();
if let Some(ramfb) = vm_objects.framebuffer() {
vnc_server.attach(vm_objects.ps2ctrl().clone(), ramfb.clone());
Expand Down Expand Up @@ -110,10 +118,11 @@ async fn register_oximeter_producer(
log: &slog::Logger,
cfg: &MetricsEndpointConfig,
registry: &ProducerRegistry,
spec: &Spec,
vm_properties: &InstanceProperties,
) -> OximeterState {
let mut oximeter_state = OximeterState::default();
let virtual_machine = VirtualMachine::from(vm_properties);
let virtual_machine = VirtualMachine::new(spec.board.cpus, vm_properties);

// Create the server itself.
//
Expand Down
Loading
Loading