From a177bb77157d53c0f7e0ff9f50aa2fd6a8983f52 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 8 Dec 2023 14:08:00 -0500 Subject: [PATCH 1/5] Explicitly use the alloc op context for dpd nat updates --- nexus/src/app/instance_network.rs | 5 +++-- nexus/src/app/sagas/instance_delete.rs | 6 +++++- nexus/src/app/sagas/instance_start.rs | 6 +++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/nexus/src/app/instance_network.rs b/nexus/src/app/instance_network.rs index abb8c744e1..1cef5e2c5a 100644 --- a/nexus/src/app/instance_network.rs +++ b/nexus/src/app/instance_network.rs @@ -438,6 +438,7 @@ impl super::Nexus { pub(crate) async fn instance_delete_dpd_config( &self, opctx: &OpContext, + opctx_alloc: &OpContext, authz_instance: &authz::Instance, ) -> Result<(), Error> { let log = &self.log; @@ -451,8 +452,6 @@ impl super::Nexus { .instance_lookup_external_ips(opctx, instance_id) .await?; - let boundary_switches = self.boundary_switches(opctx).await?; - let mut errors = vec![]; for entry in external_ips { // Soft delete the NAT entry @@ -478,6 +477,8 @@ impl super::Nexus { }?; } + let boundary_switches = self.boundary_switches(opctx_alloc).await?; + for switch in &boundary_switches { debug!(&self.log, "notifying dendrite of updates"; "instance_id" => %authz_instance.id(), diff --git a/nexus/src/app/sagas/instance_delete.rs b/nexus/src/app/sagas/instance_delete.rs index 7802312b10..34bb310846 100644 --- a/nexus/src/app/sagas/instance_delete.rs +++ b/nexus/src/app/sagas/instance_delete.rs @@ -134,7 +134,11 @@ async fn sid_delete_nat( osagactx .nexus() - .instance_delete_dpd_config(&opctx, &authz_instance) + .instance_delete_dpd_config( + &opctx, + &osagactx.nexus().opctx_alloc, + &authz_instance, + ) .await .map_err(ActionError::action_failed)?; diff --git a/nexus/src/app/sagas/instance_start.rs b/nexus/src/app/sagas/instance_start.rs index 76773d6369..e39fed6104 100644 --- a/nexus/src/app/sagas/instance_start.rs +++ b/nexus/src/app/sagas/instance_start.rs @@ -453,7 +453,11 @@ async fn sis_dpd_ensure_undo( osagactx .nexus() - .instance_delete_dpd_config(&opctx, &authz_instance) + .instance_delete_dpd_config( + &opctx, + &osagactx.nexus().opctx_alloc, + &authz_instance, + ) .await?; Ok(()) From 1d920818b3e2bbb53afdb508b6ef4ca87d124f62 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 8 Dec 2023 14:55:10 -0500 Subject: [PATCH 2/5] Add a delete to a silo instance to test instance delete flow --- nexus/tests/integration_tests/instances.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 33d4d15d23..bb7f70459c 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -3913,6 +3913,16 @@ async fn test_instance_create_in_silo(cptestctx: &ControlPlaneTestContext) { 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); + + // Delete the instance. + instance_post(&client, instance_name, InstanceOp::Stop).await; + instance_simulate(nexus, &instance.identity.id).await; + + NexusRequest::object_delete(client, &instance_url) + .authn_as(AuthnMode::SiloUser(user_id)) + .execute() + .await + .expect("Failed to delete the instance"); } /// Test that appropriate OPTE V2P mappings are created and deleted. From 8b04b36c61ff24215b3b62d4beebe176313ed107 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 8 Dec 2023 17:50:46 -0500 Subject: [PATCH 3/5] Make sure things are happening in the right context --- nexus/tests/integration_tests/instances.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index bb7f70459c..7b0d2ab32d 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -3914,10 +3914,23 @@ async fn test_instance_create_in_silo(cptestctx: &ControlPlaneTestContext) { let instance = instance_get_as(&client, &instance_url, authn).await; assert_eq!(instance.runtime.run_state, InstanceState::Running); - // Delete the instance. - instance_post(&client, instance_name, InstanceOp::Stop).await; + // Stop the instance + NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &format!("/v1/instances/{}/stop", instance.identity.id), + ) + .body(None as Option<&serde_json::Value>) + .expect_status(Some(StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::SiloUser(user_id)) + .execute() + .await + .expect("Failed to stop the instance"); instance_simulate(nexus, &instance.identity.id).await; + // Delete the instance NexusRequest::object_delete(client, &instance_url) .authn_as(AuthnMode::SiloUser(user_id)) .execute() From dc703e683b015fcd8b29eb08efd567cd3a200eed Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Mon, 11 Dec 2023 10:33:04 -0500 Subject: [PATCH 4/5] i got u fam --- nexus/tests/integration_tests/instances.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 7b0d2ab32d..6790ecf750 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -3928,7 +3928,8 @@ async fn test_instance_create_in_silo(cptestctx: &ControlPlaneTestContext) { .execute() .await .expect("Failed to stop the instance"); - instance_simulate(nexus, &instance.identity.id).await; + + instance_simulate_with_opctx(nexus, &instance.identity.id, &opctx).await; // Delete the instance NexusRequest::object_delete(client, &instance_url) From 515d0cf294eb5783eb2bb3930dabc2a83e6b94f3 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 11 Dec 2023 12:45:28 -0500 Subject: [PATCH 5/5] Pull opctx_alloc from self instead of passing it explicitly --- nexus/src/app/instance_network.rs | 4 ++-- nexus/src/app/sagas/instance_delete.rs | 6 +----- nexus/src/app/sagas/instance_start.rs | 6 +----- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/nexus/src/app/instance_network.rs b/nexus/src/app/instance_network.rs index 1cef5e2c5a..3db749f43b 100644 --- a/nexus/src/app/instance_network.rs +++ b/nexus/src/app/instance_network.rs @@ -438,7 +438,6 @@ impl super::Nexus { pub(crate) async fn instance_delete_dpd_config( &self, opctx: &OpContext, - opctx_alloc: &OpContext, authz_instance: &authz::Instance, ) -> Result<(), Error> { let log = &self.log; @@ -477,7 +476,8 @@ impl super::Nexus { }?; } - let boundary_switches = self.boundary_switches(opctx_alloc).await?; + let boundary_switches = + self.boundary_switches(&self.opctx_alloc).await?; for switch in &boundary_switches { debug!(&self.log, "notifying dendrite of updates"; diff --git a/nexus/src/app/sagas/instance_delete.rs b/nexus/src/app/sagas/instance_delete.rs index 34bb310846..7802312b10 100644 --- a/nexus/src/app/sagas/instance_delete.rs +++ b/nexus/src/app/sagas/instance_delete.rs @@ -134,11 +134,7 @@ async fn sid_delete_nat( osagactx .nexus() - .instance_delete_dpd_config( - &opctx, - &osagactx.nexus().opctx_alloc, - &authz_instance, - ) + .instance_delete_dpd_config(&opctx, &authz_instance) .await .map_err(ActionError::action_failed)?; diff --git a/nexus/src/app/sagas/instance_start.rs b/nexus/src/app/sagas/instance_start.rs index e39fed6104..76773d6369 100644 --- a/nexus/src/app/sagas/instance_start.rs +++ b/nexus/src/app/sagas/instance_start.rs @@ -453,11 +453,7 @@ async fn sis_dpd_ensure_undo( osagactx .nexus() - .instance_delete_dpd_config( - &opctx, - &osagactx.nexus().opctx_alloc, - &authz_instance, - ) + .instance_delete_dpd_config(&opctx, &authz_instance) .await?; Ok(())