Skip to content

Commit

Permalink
views::IpPoolSilo -> views::IpPoolSiloLink, params::IpPoolSiloLink ->…
Browse files Browse the repository at this point in the history
… params::IpPoolLinkSilo
  • Loading branch information
david-crespo committed Jan 19, 2024
1 parent b489f09 commit 7685d47
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 67 deletions.
4 changes: 2 additions & 2 deletions end-to-end-tests/src/bin/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use end_to_end_tests::helpers::{generate_name, get_system_ip_pool};
use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError};
use oxide_client::types::{
ByteCount, DeviceAccessTokenRequest, DeviceAuthRequest, DeviceAuthVerify,
DiskCreate, DiskSource, IpPoolCreate, IpPoolSiloLink, IpRange, Ipv4Range,
DiskCreate, DiskSource, IpPoolCreate, IpPoolLinkSilo, IpRange, Ipv4Range,
NameOrId, SiloQuotasUpdate,
};
use oxide_client::{
Expand Down Expand Up @@ -51,7 +51,7 @@ async fn main() -> Result<()> {
client
.ip_pool_silo_link()
.pool(pool_name)
.body(IpPoolSiloLink {
.body(IpPoolLinkSilo {
silo: NameOrId::Name(params.silo_name().parse().unwrap()),
is_default: true,
})
Expand Down
2 changes: 1 addition & 1 deletion nexus/db-model/src/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub struct IpPoolResource {
pub is_default: bool,
}

impl From<IpPoolResource> for views::IpPoolSilo {
impl From<IpPoolResource> for views::IpPoolSiloLink {
fn from(assoc: IpPoolResource) -> Self {
Self {
ip_pool_id: assoc.ip_pool_id,
Expand Down
2 changes: 1 addition & 1 deletion nexus/src/app/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl super::Nexus {
&self,
opctx: &OpContext,
pool_lookup: &lookup::IpPool<'_>,
silo_link: &params::IpPoolSiloLink,
silo_link: &params::IpPoolLinkSilo,
) -> CreateResult<db::model::IpPoolResource> {
let (authz_pool,) =
pool_lookup.lookup_for(authz::Action::Modify).await?;
Expand Down
10 changes: 5 additions & 5 deletions nexus/src/external_api/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1546,7 +1546,7 @@ async fn ip_pool_silo_list(
// whatever the thing is. Still... all we'd have to do to make this usable
// in both places would be to make it { ...IpPool, silo_id, silo_name,
// is_default }
) -> Result<HttpResponseOk<ResultsPage<views::IpPoolSilo>>, HttpError> {
) -> Result<HttpResponseOk<ResultsPage<views::IpPoolSiloLink>>, HttpError> {
let apictx = rqctx.context();
let handler = async {
let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
Expand All @@ -1568,7 +1568,7 @@ async fn ip_pool_silo_list(
Ok(HttpResponseOk(ScanById::results_page(
&query,
assocs,
&|_, x: &views::IpPoolSilo| x.silo_id,
&|_, x: &views::IpPoolSiloLink| x.silo_id,
)?))
};
apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
Expand All @@ -1583,8 +1583,8 @@ async fn ip_pool_silo_list(
async fn ip_pool_silo_link(
rqctx: RequestContext<Arc<ServerContext>>,
path_params: Path<params::IpPoolPath>,
resource_assoc: TypedBody<params::IpPoolSiloLink>,
) -> Result<HttpResponseCreated<views::IpPoolSilo>, HttpError> {
resource_assoc: TypedBody<params::IpPoolLinkSilo>,
) -> Result<HttpResponseCreated<views::IpPoolSiloLink>, HttpError> {
let apictx = rqctx.context();
let handler = async {
let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
Expand Down Expand Up @@ -1638,7 +1638,7 @@ async fn ip_pool_silo_update(
rqctx: RequestContext<Arc<ServerContext>>,
path_params: Path<params::IpPoolSiloPath>,
update: TypedBody<params::IpPoolSiloUpdate>,
) -> Result<HttpResponseOk<views::IpPoolSilo>, HttpError> {
) -> Result<HttpResponseOk<views::IpPoolSiloLink>, HttpError> {
let apictx = rqctx.context();
let handler = async {
let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
Expand Down
4 changes: 2 additions & 2 deletions nexus/test-utils/src/resource_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,9 @@ pub async fn link_ip_pool(
is_default: bool,
) {
let link =
params::IpPoolSiloLink { silo: NameOrId::Id(*silo_id), is_default };
params::IpPoolLinkSilo { silo: NameOrId::Id(*silo_id), is_default };
let url = format!("/v1/system/ip-pools/{pool_name}/silos");
object_create::<params::IpPoolSiloLink, views::IpPoolSilo>(
object_create::<params::IpPoolLinkSilo, views::IpPoolSiloLink>(
client, &url, &link,
)
.await;
Expand Down
4 changes: 2 additions & 2 deletions nexus/tests/integration_tests/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,8 +629,8 @@ pub static DEMO_IP_POOL_UPDATE: Lazy<params::IpPoolUpdate> =
});
pub static DEMO_IP_POOL_SILOS_URL: Lazy<String> =
Lazy::new(|| format!("{}/silos", *DEMO_IP_POOL_URL));
pub static DEMO_IP_POOL_SILOS_BODY: Lazy<params::IpPoolSiloLink> =
Lazy::new(|| params::IpPoolSiloLink {
pub static DEMO_IP_POOL_SILOS_BODY: Lazy<params::IpPoolLinkSilo> =
Lazy::new(|| params::IpPoolLinkSilo {
silo: NameOrId::Id(DEFAULT_SILO.identity().id),
is_default: true, // necessary for demo instance create to go through
});
Expand Down
6 changes: 3 additions & 3 deletions nexus/tests/integration_tests/instances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3657,7 +3657,7 @@ async fn test_instance_ephemeral_ip_from_correct_pool(
);

// make pool2 default and create instance with default pool. check that it now it comes from pool2
let _: views::IpPoolSilo = object_put(
let _: views::IpPoolSiloLink = object_put(
client,
&format!("/v1/system/ip-pools/pool2/silos/{}", DEFAULT_SILO.id()),
&params::IpPoolSiloUpdate { is_default: true },
Expand Down Expand Up @@ -3788,11 +3788,11 @@ async fn test_instance_ephemeral_ip_from_orphan_pool(

// associate the pool with a different silo and we should get the same
// error on instance create
let params = params::IpPoolSiloLink {
let params = params::IpPoolLinkSilo {
silo: NameOrId::Name(cptestctx.silo_name.clone()),
is_default: false,
};
let _: views::IpPoolSilo =
let _: views::IpPoolSiloLink =
object_create(client, "/v1/system/ip-pools/orphan-pool/silos", &params)
.await;

Expand Down
51 changes: 29 additions & 22 deletions nexus/tests/integration_tests/ip_pools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use nexus_test_utils::resource_helpers::objects_list_page_authz;
use nexus_test_utils_macros::nexus_test;
use nexus_types::external_api::params;
use nexus_types::external_api::params::IpPoolCreate;
use nexus_types::external_api::params::IpPoolSiloLink;
use nexus_types::external_api::params::IpPoolLinkSilo;
use nexus_types::external_api::params::IpPoolSiloUpdate;
use nexus_types::external_api::params::IpPoolUpdate;
use nexus_types::external_api::shared::IpRange;
Expand All @@ -40,7 +40,7 @@ use nexus_types::external_api::shared::Ipv6Range;
use nexus_types::external_api::shared::SiloIdentityMode;
use nexus_types::external_api::views::IpPool;
use nexus_types::external_api::views::IpPoolRange;
use nexus_types::external_api::views::IpPoolSilo;
use nexus_types::external_api::views::IpPoolSiloLink;
use nexus_types::external_api::views::Silo;
use nexus_types::external_api::views::SiloIpPool;
use nexus_types::identity::Resource;
Expand Down Expand Up @@ -346,7 +346,7 @@ async fn test_ip_pool_service_no_cud(cptestctx: &ControlPlaneTestContext) {

// linking not allowed

// let link_body = params::IpPoolSiloLink {
// let link_body = params::IpPoolLinkSilo {
// silo: NameOrId::Name(cptestctx.silo_name.clone()),
// is_default: false,
// };
Expand Down Expand Up @@ -380,7 +380,7 @@ async fn test_ip_pool_silo_link(cptestctx: &ControlPlaneTestContext) {

// expect 404 on association if the specified silo doesn't exist
let nonexistent_silo_id = Uuid::new_v4();
let params = params::IpPoolSiloLink {
let params = params::IpPoolLinkSilo {
silo: NameOrId::Id(nonexistent_silo_id),
is_default: false,
};
Expand All @@ -404,8 +404,8 @@ async fn test_ip_pool_silo_link(cptestctx: &ControlPlaneTestContext) {
// associate by name with silo that exists
let silo = NameOrId::Name(cptestctx.silo_name.clone());
let params =
params::IpPoolSiloLink { silo: silo.clone(), is_default: false };
let _: IpPoolSilo =
params::IpPoolLinkSilo { silo: silo.clone(), is_default: false };
let _: IpPoolSiloLink =
object_create(client, "/v1/system/ip-pools/p0/silos", &params).await;

// second attempt to create the same link errors due to conflict
Expand All @@ -423,8 +423,11 @@ async fn test_ip_pool_silo_link(cptestctx: &ControlPlaneTestContext) {
let silo_id = object_get::<Silo>(client, &silo_url).await.identity.id;

let assocs_p0 = silos_for_pool(client, "p0").await;
let silo_link =
IpPoolSilo { ip_pool_id: p0.identity.id, silo_id, is_default: false };
let silo_link = IpPoolSiloLink {
ip_pool_id: p0.identity.id,
silo_id,
is_default: false,
};
assert_eq!(assocs_p0.items.len(), 1);
assert_eq!(assocs_p0.items[0], silo_link);

Expand All @@ -434,18 +437,22 @@ async fn test_ip_pool_silo_link(cptestctx: &ControlPlaneTestContext) {
assert_eq!(silo_pools[0].is_default, false);

// associate same silo to other pool by ID instead of name
let link_params = params::IpPoolSiloLink {
let link_params = params::IpPoolLinkSilo {
silo: NameOrId::Id(silo_id),
is_default: true,
};
let url = "/v1/system/ip-pools/p1/silos";
let _: IpPoolSilo = object_create(client, &url, &link_params).await;
let _: IpPoolSiloLink = object_create(client, &url, &link_params).await;

let silos_p1 = silos_for_pool(client, "p1").await;
assert_eq!(silos_p1.items.len(), 1);
assert_eq!(
silos_p1.items[0],
IpPoolSilo { ip_pool_id: p1.identity.id, is_default: true, silo_id }
IpPoolSiloLink {
ip_pool_id: p1.identity.id,
is_default: true,
silo_id
}
);

let silo_pools = pools_for_silo(client, silo_name).await;
Expand Down Expand Up @@ -525,10 +532,10 @@ async fn test_ip_pool_update_default(cptestctx: &ControlPlaneTestContext) {
// associate both pools with the test silo
let silo = NameOrId::Name(cptestctx.silo_name.clone());
let params =
params::IpPoolSiloLink { silo: silo.clone(), is_default: false };
let _: IpPoolSilo =
params::IpPoolLinkSilo { silo: silo.clone(), is_default: false };
let _: IpPoolSiloLink =
object_create(client, "/v1/system/ip-pools/p0/silos", &params).await;
let _: IpPoolSilo =
let _: IpPoolSiloLink =
object_create(client, "/v1/system/ip-pools/p1/silos", &params).await;

// now both are linked to the silo, neither is marked default
Expand All @@ -542,10 +549,10 @@ async fn test_ip_pool_update_default(cptestctx: &ControlPlaneTestContext) {

// make p0 default
let params = IpPoolSiloUpdate { is_default: true };
let _: IpPoolSilo = object_put(client, &p0_silo_url, &params).await;
let _: IpPoolSiloLink = object_put(client, &p0_silo_url, &params).await;

// making the same one default again is not an error
let _: IpPoolSilo = object_put(client, &p0_silo_url, &params).await;
let _: IpPoolSiloLink = object_put(client, &p0_silo_url, &params).await;

// now p0 is default
let silos_p0 = silos_for_pool(client, "p0").await;
Expand All @@ -563,7 +570,7 @@ async fn test_ip_pool_update_default(cptestctx: &ControlPlaneTestContext) {
let params = IpPoolSiloUpdate { is_default: true };
let p1_silo_url =
format!("/v1/system/ip-pools/p1/silos/{}", cptestctx.silo_name);
let _: IpPoolSilo = object_put(client, &p1_silo_url, &params).await;
let _: IpPoolSiloLink = object_put(client, &p1_silo_url, &params).await;

// p1 is now default
let silos_p1 = silos_for_pool(client, "p1").await;
Expand All @@ -577,7 +584,7 @@ async fn test_ip_pool_update_default(cptestctx: &ControlPlaneTestContext) {

// we can also unset default
let params = IpPoolSiloUpdate { is_default: false };
let _: IpPoolSilo = object_put(client, &p1_silo_url, &params).await;
let _: IpPoolSiloLink = object_put(client, &p1_silo_url, &params).await;

let silos_p1 = silos_for_pool(client, "p1").await;
assert_eq!(silos_p1.items.len(), 1);
Expand Down Expand Up @@ -629,9 +636,9 @@ fn get_names(pools: Vec<IpPool>) -> Vec<String> {
async fn silos_for_pool(
client: &ClientTestContext,
pool: &str,
) -> ResultsPage<IpPoolSilo> {
) -> ResultsPage<IpPoolSiloLink> {
let url = format!("/v1/system/ip-pools/{}/silos", pool);
objects_list_page_authz::<IpPoolSilo>(client, &url).await
objects_list_page_authz::<IpPoolSiloLink>(client, &url).await
}

async fn pools_for_silo(
Expand Down Expand Up @@ -1028,13 +1035,13 @@ async fn test_ip_range_delete_with_allocated_external_ip_fails(
.await;

// associate pool with default silo, which is the privileged user's silo
let params = IpPoolSiloLink {
let params = IpPoolLinkSilo {
silo: NameOrId::Id(DEFAULT_SILO.id()),
is_default: true,
};
NexusRequest::objects_post(client, &ip_pool_silos_url, &params)
.authn_as(AuthnMode::PrivilegedUser)
.execute_and_parse_unwrap::<IpPoolSilo>()
.execute_and_parse_unwrap::<IpPoolSiloLink>()
.await;

// Add an IP range to the pool
Expand Down
2 changes: 1 addition & 1 deletion nexus/types/src/external_api/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ pub struct IpPoolSiloPath {
}

#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
pub struct IpPoolSiloLink {
pub struct IpPoolLinkSilo {
pub silo: NameOrId,
/// When a pool is the default for a silo, floating IPs and instance
/// ephemeral IPs will come from that pool when no other pool is specified.
Expand Down
5 changes: 1 addition & 4 deletions nexus/types/src/external_api/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,13 +315,10 @@ pub struct SiloIpPool {
pub is_default: bool,
}

// TODO: rename IpPoolSilo or get rid of it somehow. we cannot have both
// IpPoolSilo and SiloIpPool. come on

/// A link between an IP pool and a silo that allows one to allocate IPs from
/// the pool within the silo
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
pub struct IpPoolSilo {
pub struct IpPoolSiloLink {
pub ip_pool_id: Uuid,
pub silo_id: Uuid,
/// When a pool is the default for a silo, floating IPs and instance
Expand Down
Loading

0 comments on commit 7685d47

Please sign in to comment.