Skip to content
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
1 change: 0 additions & 1 deletion common/src/api/internal/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//! Internally facing APIs.

pub mod bootstrap_agent;
pub mod nexus;
pub mod sled_agent;
34 changes: 0 additions & 34 deletions common/src/api/internal/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,6 @@ use crate::api::{external, internal};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use std::fmt::{Debug, Display, Formatter, Result as FormatResult};
use uuid::Uuid;

/// Sent from to a sled agent to establish the runtime state of a Disk
#[derive(Serialize, Deserialize, JsonSchema)]
pub struct DiskEnsureBody {
/// Last runtime state of the Disk known to Nexus (used if the agent has
/// never seen this Disk before).
pub initial_runtime: internal::nexus::DiskRuntimeState,
/// requested runtime state of the Disk
pub target: DiskStateRequested,
}

///Used to request a Disk state change
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)]
#[serde(rename_all = "lowercase", tag = "state", content = "instance")]
pub enum DiskStateRequested {
Detached,
Attached(Uuid),
Destroyed,
Faulted,
}

impl DiskStateRequested {
/// Returns whether the requested state is attached to an Instance or not.
pub fn is_attached(&self) -> bool {
match self {
DiskStateRequested::Detached => false,
DiskStateRequested::Destroyed => false,
DiskStateRequested::Faulted => false,

DiskStateRequested::Attached(_) => true,
}
}
}

/// Describes the instance hardware.
#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)]
Expand Down
21 changes: 0 additions & 21 deletions common/src/sled_agent_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,27 +201,6 @@ impl From<types::DiskState> for crate::api::external::DiskState {
}
}

impl From<crate::api::internal::sled_agent::DiskStateRequested>
for types::DiskStateRequested
{
fn from(s: crate::api::internal::sled_agent::DiskStateRequested) -> Self {
match s {
crate::api::internal::sled_agent::DiskStateRequested::Detached => {
Self::Detached
}
crate::api::internal::sled_agent::DiskStateRequested::Attached(
u,
) => Self::Attached(u),
crate::api::internal::sled_agent::DiskStateRequested::Destroyed => {
Self::Destroyed
}
crate::api::internal::sled_agent::DiskStateRequested::Faulted => {
Self::Faulted
}
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahl because nexus refers to the generated type all the time, there is no more conversion

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome

impl From<crate::api::internal::sled_agent::InstanceHardware>
for types::InstanceHardware
{
Expand Down
32 changes: 19 additions & 13 deletions nexus/src/nexus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ use omicron_common::api::external::VpcSubnetUpdateParams;
use omicron_common::api::external::VpcUpdateParams;
use omicron_common::api::internal::nexus;
use omicron_common::api::internal::nexus::DiskRuntimeState;
use omicron_common::api::internal::sled_agent::DiskStateRequested;
use omicron_common::api::internal::sled_agent::InstanceRuntimeStateRequested;
use omicron_common::api::internal::sled_agent::InstanceStateRequested;
use omicron_common::backoff;
use omicron_common::bail_unless;
use omicron_common::sled_agent_client;
use omicron_common::OximeterClient;
use omicron_common::SledAgentClient;
use oximeter_producer::register;
Expand Down Expand Up @@ -1060,16 +1060,17 @@ impl Nexus {
// TODO: Populate this with an appropriate NIC.
// See also: sic_create_instance_record in sagas.rs for a similar
// construction.
let instance_hardware =
omicron_common::sled_agent_client::types::InstanceHardware {
runtime: omicron_common::sled_agent_client::types::InstanceRuntimeState::from(runtime),
nics: vec![],
};
let instance_hardware = sled_agent_client::types::InstanceHardware {
runtime: sled_agent_client::types::InstanceRuntimeState::from(
runtime,
),
nics: vec![],
};

let new_runtime = sa
.instance_put(
&instance.id(),
&omicron_common::sled_agent_client::types::InstanceEnsureBody {
&sled_agent_client::types::InstanceEnsureBody {
initial: instance_hardware,
target: requested.into(),
},
Expand Down Expand Up @@ -1266,7 +1267,9 @@ impl Nexus {
self.disk_set_runtime(
&disk,
self.instance_sled(&instance).await?,
DiskStateRequested::Attached(*instance_id),
sled_agent_client::types::DiskStateRequested::Attached(
*instance_id,
),
)
.await?;
let disk = self.db_datastore.disk_fetch(&disk.id()).await?;
Expand Down Expand Up @@ -1336,7 +1339,7 @@ impl Nexus {
self.disk_set_runtime(
&disk,
self.instance_sled(&instance).await?,
DiskStateRequested::Detached,
sled_agent_client::types::DiskStateRequested::Detached,
)
.await?;
Ok(())
Expand All @@ -1350,7 +1353,7 @@ impl Nexus {
&self,
disk: &db::model::Disk,
sa: Arc<SledAgentClient>,
requested: DiskStateRequested,
requested: sled_agent_client::types::DiskStateRequested,
) -> Result<(), Error> {
let runtime: DiskRuntimeState = disk.runtime().into();

Expand All @@ -1361,9 +1364,12 @@ impl Nexus {
let new_runtime = sa
.disk_put(
&disk.id(),
&omicron_common::sled_agent_client::types::DiskEnsureBody {
initial_runtime: omicron_common::sled_agent_client::types::DiskRuntimeState::from(runtime),
target: omicron_common::sled_agent_client::types::DiskStateRequested::from(requested),
&sled_agent_client::types::DiskEnsureBody {
initial_runtime:
sled_agent_client::types::DiskRuntimeState::from(
runtime,
),
target: requested,
},
)
.await
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/bootstrap/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

use super::client::types as bootstrap_types;
use super::client::Client as BootstrapClient;
use super::views::ShareResponse;
use omicron_common::api::external::Error;
use omicron_common::api::internal::bootstrap_agent::ShareResponse;
use omicron_common::packaging::sha256_digest;

use slog::Logger;
Expand Down
4 changes: 1 addition & 3 deletions sled-agent/src/bootstrap/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ use dropshot::HttpError;
use dropshot::HttpResponseOk;
use dropshot::RequestContext;
use dropshot::TypedBody;
use omicron_common::api::internal::bootstrap_agent::{
ShareRequest, ShareResponse,
};
use std::sync::Arc;

use super::agent::Agent;
use super::{params::ShareRequest, views::ShareResponse};

/// Returns a description of the bootstrap agent API
pub fn ba_api() -> ApiDescription<Arc<Agent>> {
Expand Down
2 changes: 2 additions & 0 deletions sled-agent/src/bootstrap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ pub mod agent;
mod client;
pub mod config;
mod http_entrypoints;
mod params;
pub mod server;
mod views;
11 changes: 11 additions & 0 deletions sled-agent/src/bootstrap/params.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//! Request body types for the bootstrap agent

use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

/// Identity signed by local RoT and Oxide certificate chain.
#[derive(Serialize, Deserialize, JsonSchema)]
pub struct ShareRequest {
// TODO-completeness: format TBD; currently opaque.
pub identity: Vec<u8>,
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
//! APIs exposed by the bootstrap agent
//! Response types for the bootstrap agent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why pull apart inputs and outputs?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related: I asked this on chat, but I'll ask it more publicly -

Are "views" supposed to be 1:1 with response types, or with DB representations?
This PR implies the former ("response types") but #374 implies the latter ("Views are public lenses onto the DB models.").

I'm especially curious about responses that do not represent data stored in a DB - does that still make sense to store in "view.rs"? If so, would it be more accurate to call these "requests.rs" and "responses.rs"? And in that case, I think Adam's question still applies - why split 'em?

Copy link
Contributor Author

@david-crespo david-crespo Nov 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right to point out the disparity — I'll update #374 to say they're response bodies, most of which are lenses onto DB models.

I'm really just following a convention set by the big fullstack frameworks in various languages — Rails, Django, Phoenix, Laravel — which all call response bodies "views", though usually those are HTML responses. In Rails they call the JSON serializers "serializers". I guess they call them serializers in Django too. They distinguish views and serializers from controllers (route handlers) and models.

They're not as consistent about what they call request bodies, probably because in the dynamic languages the request bodies don't have to be defined anywhere outside of the request handler.

requests.rs and responses.rs would be fine names, though they may obscure that we're talking about the bodies in both cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to merge as-is because we can rearrange easily later. The hard part, as you'll see, is moving things out of common. Moving things around within sled-agent or within nexus is easy.

use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

/// Identity signed by local RoT and Oxide certificate chain.
#[derive(Serialize, Deserialize, JsonSchema)]
pub struct ShareRequest {
// TODO-completeness: format TBD; currently opaque.
pub identity: Vec<u8>,
}

/// Sent between bootstrap agents to establish trust quorum.
#[derive(Serialize, Deserialize, JsonSchema)]
pub struct ShareResponse {
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/common/disk.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! Describes the states of network-attached storage.

use crate::params::DiskStateRequested;
use chrono::Utc;
use omicron_common::api::external::DiskState;
use omicron_common::api::external::Error;
use omicron_common::api::internal::nexus::DiskRuntimeState;
use omicron_common::api::internal::sled_agent::DiskStateRequested;
use propolis_client::api::DiskAttachmentState as PropolisDiskState;
use uuid::Uuid;

Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/http_entrypoints.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! HTTP entrypoint functions for the sled agent's exposed API

use super::params::DiskEnsureBody;
use dropshot::endpoint;
use dropshot::ApiDescription;
use dropshot::HttpError;
Expand All @@ -9,7 +10,6 @@ use dropshot::RequestContext;
use dropshot::TypedBody;
use omicron_common::api::internal::nexus::DiskRuntimeState;
use omicron_common::api::internal::nexus::InstanceRuntimeState;
use omicron_common::api::internal::sled_agent::DiskEnsureBody;
use omicron_common::api::internal::sled_agent::InstanceEnsureBody;
use schemars::JsonSchema;
use serde::Deserialize;
Expand Down
1 change: 1 addition & 0 deletions sled-agent/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod http_entrypoints;
mod illumos;
mod instance;
mod instance_manager;
mod params;
pub mod server;
mod sled_agent;

Expand Down
37 changes: 37 additions & 0 deletions sled-agent/src/params.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use omicron_common::api::internal::nexus::DiskRuntimeState;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use uuid::Uuid;

///Used to request a Disk state change
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)]
#[serde(rename_all = "lowercase", tag = "state", content = "instance")]
pub enum DiskStateRequested {
Detached,
Attached(Uuid),
Destroyed,
Faulted,
}

impl DiskStateRequested {
/// Returns whether the requested state is attached to an Instance or not.
pub fn is_attached(&self) -> bool {
match self {
DiskStateRequested::Detached => false,
DiskStateRequested::Destroyed => false,
DiskStateRequested::Faulted => false,

DiskStateRequested::Attached(_) => true,
}
}
}

/// Sent from to a sled agent to establish the runtime state of a Disk
#[derive(Serialize, Deserialize, JsonSchema)]
pub struct DiskEnsureBody {
/// Last runtime state of the Disk known to Nexus (used if the agent has
/// never seen this Disk before).
pub initial_runtime: DiskRuntimeState,
/// requested runtime state of the Disk
pub target: DiskStateRequested,
}
2 changes: 1 addition & 1 deletion sled-agent/src/sim/collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ impl<S: Simulatable + 'static> SimCollection<S> {

#[cfg(test)]
mod test {
use crate::params::DiskStateRequested;
use crate::sim::collection::SimObject;
use crate::sim::disk::SimDisk;
use crate::sim::instance::SimInstance;
Expand All @@ -344,7 +345,6 @@ mod test {
use omicron_common::api::external::InstanceState;
use omicron_common::api::internal::nexus::DiskRuntimeState;
use omicron_common::api::internal::nexus::InstanceRuntimeState;
use omicron_common::api::internal::sled_agent::DiskStateRequested;
use omicron_common::api::internal::sled_agent::InstanceRuntimeStateRequested;
use omicron_common::api::internal::sled_agent::InstanceStateRequested;
use omicron_test_utils::dev::test_setup_log;
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/sim/disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
* Simulated sled agent implementation
*/

use crate::params::DiskStateRequested;
use crate::sim::simulatable::Simulatable;
use async_trait::async_trait;
use omicron_common::api::external::DiskState;
use omicron_common::api::external::Error;
use omicron_common::api::external::Generation;
use omicron_common::api::internal::nexus::DiskRuntimeState;
use omicron_common::api::internal::sled_agent::DiskStateRequested;
use omicron_common::NexusClient;
use propolis_client::api::DiskAttachmentState as PropolisDiskState;
use std::sync::Arc;
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/sim/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* HTTP entrypoint functions for the sled agent's exposed API
*/

use crate::params::DiskEnsureBody;
use dropshot::endpoint;
use dropshot::ApiDescription;
use dropshot::HttpError;
Expand All @@ -12,7 +13,6 @@ use dropshot::RequestContext;
use dropshot::TypedBody;
use omicron_common::api::internal::nexus::DiskRuntimeState;
use omicron_common::api::internal::nexus::InstanceRuntimeState;
use omicron_common::api::internal::sled_agent::DiskEnsureBody;
use omicron_common::api::internal::sled_agent::InstanceEnsureBody;
use schemars::JsonSchema;
use serde::Deserialize;
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/sim/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
* Simulated sled agent implementation
*/

use crate::params::DiskStateRequested;
use omicron_common::api::external::Error;
use omicron_common::api::internal::nexus::DiskRuntimeState;
use omicron_common::api::internal::nexus::InstanceRuntimeState;
use omicron_common::api::internal::sled_agent::DiskStateRequested;
use omicron_common::api::internal::sled_agent::InstanceHardware;
use omicron_common::api::internal::sled_agent::InstanceRuntimeStateRequested;
use omicron_common::NexusClient;
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/sled_agent.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Sled agent implementation

use crate::params::DiskStateRequested;
use omicron_common::api::{
external::Error, internal::nexus::DiskRuntimeState,
internal::nexus::InstanceRuntimeState,
internal::sled_agent::DiskStateRequested,
internal::sled_agent::InstanceHardware,
internal::sled_agent::InstanceRuntimeStateRequested,
};
Expand Down