Skip to content

Commit

Permalink
Revalidate consumers of instance_lookup_external_ips
Browse files Browse the repository at this point in the history
  • Loading branch information
FelixMcFelix committed Dec 28, 2023
1 parent 21d5776 commit dcdec48
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 3 deletions.
1 change: 1 addition & 0 deletions nexus/db-queries/src/db/datastore/external_ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ impl DataStore {
}

/// Fetch all external IP addresses of any kind for the provided instance
/// in all attachment states.
pub async fn instance_lookup_external_ips(
&self,
opctx: &OpContext,
Expand Down
5 changes: 4 additions & 1 deletion nexus/src/app/external_ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use crate::external_api::views::ExternalIp;
use crate::external_api::views::FloatingIp;
use nexus_db_model::IpAttachState;
use nexus_db_queries::authz;
use nexus_db_queries::context::OpContext;
use nexus_db_queries::db::lookup;
Expand Down Expand Up @@ -34,7 +35,9 @@ impl super::Nexus {
.await?
.into_iter()
.filter_map(|ip| {
if ip.kind == IpKind::SNat {
if ip.kind == IpKind::SNat
|| ip.state != IpAttachState::Attached
{
None
} else {
Some(ip.try_into().unwrap())
Expand Down
10 changes: 10 additions & 0 deletions nexus/src/app/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::external_api::params;
use cancel_safe_futures::prelude::*;
use futures::future::Fuse;
use futures::{FutureExt, SinkExt, StreamExt};
use nexus_db_model::IpAttachState;
use nexus_db_model::IpKind;
use nexus_db_queries::authn;
use nexus_db_queries::authz;
Expand Down Expand Up @@ -1054,6 +1055,15 @@ impl super::Nexus {
));
}

// If there are any external IPs not yet fully attached/detached,then
// there are attach/detach sagas in progress. That should complete in
// its own time, so return a 503 to indicate a possible retry.
if external_ips.iter().any(|v| v.state != IpAttachState::Attached) {
return Err(Error::unavail(
"External IP attach/detach is in progress during instance_ensure_registered"
));
}

// Partition remaining external IPs by class: we can have at most
// one ephemeral ip.
let (ephemeral_ips, floating_ips): (Vec<_>, Vec<_>) = external_ips
Expand Down
4 changes: 2 additions & 2 deletions nexus/src/app/sagas/instance_ip_attach.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ use uuid::Uuid;
// The main means of access control here is an external IP's `state`.
// Entering either saga begins with an atomic swap from Attached/Detached
// to Attaching/Detaching. This prevents concurrent attach/detach on the
// same EIP, and prevents instance start from executing with an
// Error::unavail.
// same EIP, and prevents instance start and migrate from completing with an
// Error::unavail via instance_ensure_registered and/or DPD.
//
// Overlap with stop is handled by treating comms failures with
// sled-agent as temporary errors and unwinding. For the delete case, we
Expand Down

0 comments on commit dcdec48

Please sign in to comment.