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

Use opctx_alloc in start saga to query boundary switches #4274

Merged
merged 1 commit into from
Oct 13, 2023
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
4 changes: 2 additions & 2 deletions nexus/db-queries/src/authn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ impl Context {
)
}

/// Returns an authenticated context for the specific Silo user.
#[cfg(test)]
/// Returns an authenticated context for the specific Silo user. Not marked
/// as #[cfg(test)] so that this is available in integration tests.
pub fn for_test_user(
silo_user_id: Uuid,
silo_id: Uuid,
Expand Down
6 changes: 5 additions & 1 deletion nexus/src/app/sagas/instance_start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,16 +323,20 @@ async fn sis_dpd_ensure(
);
let datastore = osagactx.datastore();

// Querying sleds requires fleet access; use the instance allocator context
// for this.
let sled_uuid = sagactx.lookup::<Uuid>("sled_id")?;
let (.., sled) = LookupPath::new(&osagactx.nexus().opctx_alloc, &datastore)
.sled_id(sled_uuid)
.fetch()
.await
.map_err(ActionError::action_failed)?;

// Querying boundary switches also requires fleet access and the use of the
// instance allocator context.
let boundary_switches = osagactx
.nexus()
.boundary_switches(&opctx)
.boundary_switches(&osagactx.nexus().opctx_alloc)
.await
.map_err(ActionError::action_failed)?;

Expand Down
37 changes: 35 additions & 2 deletions nexus/src/app/test_interfaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ pub trait TestInterfaces {
id: &Uuid,
) -> Result<Option<Arc<SledAgentClient>>, Error>;

async fn instance_sled_by_id_with_opctx(
&self,
id: &Uuid,
opctx: &OpContext,
) -> Result<Option<Arc<SledAgentClient>>, Error>;

/// Returns the SledAgentClient for the sled running an instance to which a
/// disk is attached.
async fn disk_sled_by_id(
Expand All @@ -37,6 +43,12 @@ pub trait TestInterfaces {
instance_id: &Uuid,
) -> Result<Option<Uuid>, Error>;

async fn instance_sled_id_with_opctx(
&self,
instance_id: &Uuid,
opctx: &OpContext,
) -> Result<Option<Uuid>, Error>;

async fn set_disk_as_faulted(&self, disk_id: &Uuid) -> Result<bool, Error>;

fn set_samael_max_issue_delay(&self, max_issue_delay: chrono::Duration);
Expand All @@ -52,7 +64,20 @@ impl TestInterfaces for super::Nexus {
&self,
id: &Uuid,
) -> Result<Option<Arc<SledAgentClient>>, Error> {
let sled_id = self.instance_sled_id(id).await?;
let opctx = OpContext::for_tests(
self.log.new(o!()),
Arc::clone(&self.db_datastore),
);

self.instance_sled_by_id_with_opctx(id, &opctx).await
}

async fn instance_sled_by_id_with_opctx(
&self,
id: &Uuid,
opctx: &OpContext,
) -> Result<Option<Arc<SledAgentClient>>, Error> {
let sled_id = self.instance_sled_id_with_opctx(id, opctx).await?;
if let Some(sled_id) = sled_id {
Ok(Some(self.sled_client(&sled_id).await?))
} else {
Expand Down Expand Up @@ -83,14 +108,22 @@ impl TestInterfaces for super::Nexus {
Arc::clone(&self.db_datastore),
);

self.instance_sled_id_with_opctx(id, &opctx).await
}

async fn instance_sled_id_with_opctx(
&self,
id: &Uuid,
opctx: &OpContext,
) -> Result<Option<Uuid>, Error> {
let (.., authz_instance) = LookupPath::new(&opctx, &self.db_datastore)
.instance_id(*id)
.lookup_for(nexus_db_queries::authz::Action::Read)
.await?;

Ok(self
.datastore()
.instance_fetch_with_vmm(&opctx, &authz_instance)
.instance_fetch_with_vmm(opctx, &authz_instance)
.await?
.sled_id())
}
Expand Down
56 changes: 53 additions & 3 deletions nexus/tests/integration_tests/instances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3611,10 +3611,11 @@ async fn test_instance_create_in_silo(cptestctx: &ControlPlaneTestContext) {

// Create an instance using the authorization granted to the collaborator
// Silo User.
let instance_name = "collaborate-with-me";
let instance_params = params::InstanceCreate {
identity: IdentityMetadataCreateParams {
name: Name::try_from(String::from("ip-pool-test")).unwrap(),
description: String::from("instance to test IP Pool authz"),
name: Name::try_from(String::from(instance_name)).unwrap(),
description: String::from("instance to test creation in a silo"),
},
ncpus: InstanceCpuCount::try_from(2).unwrap(),
memory: ByteCount::from_gibibytes_u32(4),
Expand All @@ -3635,6 +3636,34 @@ async fn test_instance_create_in_silo(cptestctx: &ControlPlaneTestContext) {
.expect("Failed to create instance")
.parsed_body::<Instance>()
.expect("Failed to parse instance");

// Make sure the instance can actually start even though a collaborator
// created it.
let apictx = &cptestctx.server.apictx();
let nexus = &apictx.nexus;
let authn = AuthnMode::SiloUser(user_id);
let instance_url = get_instance_url(instance_name);
let instance = instance_get_as(&client, &instance_url, authn.clone()).await;
info!(&cptestctx.logctx.log, "test got instance"; "instance" => ?instance);
assert_eq!(instance.runtime.run_state, InstanceState::Starting);

// The default instance simulation function uses a test user that, while
// privileged, only has access to the default silo. Synthesize an operation
// context that grants access to the silo under test.
let opctx = OpContext::for_background(
cptestctx.logctx.log.new(o!()),
Arc::new(nexus_db_queries::authz::Authz::new(&cptestctx.logctx.log)),
nexus_db_queries::authn::Context::for_test_user(
user_id,
silo.identity.id,
nexus_db_queries::authn::SiloAuthnPolicy::try_from(&*DEFAULT_SILO)
.unwrap(),
),
nexus.datastore().clone(),
);
instance_simulate_with_opctx(nexus, &instance.identity.id, &opctx).await;
let instance = instance_get_as(&client, &instance_url, authn).await;
assert_eq!(instance.runtime.run_state, InstanceState::Running);
}

/// Test that appropriate OPTE V2P mappings are created and deleted.
Expand Down Expand Up @@ -3752,9 +3781,17 @@ async fn test_instance_v2p_mappings(cptestctx: &ControlPlaneTestContext) {
async fn instance_get(
client: &ClientTestContext,
instance_url: &str,
) -> Instance {
instance_get_as(client, instance_url, AuthnMode::PrivilegedUser).await
}

async fn instance_get_as(
client: &ClientTestContext,
instance_url: &str,
authn_as: AuthnMode,
) -> Instance {
NexusRequest::object_get(client, instance_url)
.authn_as(AuthnMode::PrivilegedUser)
.authn_as(authn_as)
.execute()
.await
.unwrap()
Expand Down Expand Up @@ -3857,6 +3894,19 @@ pub async fn instance_simulate(nexus: &Arc<Nexus>, id: &Uuid) {
sa.instance_finish_transition(*id).await;
}

pub async fn instance_simulate_with_opctx(
nexus: &Arc<Nexus>,
id: &Uuid,
opctx: &OpContext,
) {
let sa = nexus
.instance_sled_by_id_with_opctx(id, opctx)
.await
.unwrap()
.expect("instance must be on a sled to simulate a state change");
sa.instance_finish_transition(*id).await;
}

/// Simulates state transitions for the incarnation of the instance on the
/// supplied sled (which may not be the sled ID currently stored in the
/// instance's CRDB record).
Expand Down
Loading