Skip to content

Commit 60814fc

Browse files
committed
fix(core): address power policy review feedback
Signed-off-by: Patrice Breton <pbreton@nvidia.com>
1 parent d223eb3 commit 60814fc

6 files changed

Lines changed: 76 additions & 5 deletions

File tree

crates/api-core/src/handlers/vpc.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ pub(crate) async fn update(
176176
// against the VPC's persisted tenant and virtualization type.
177177
if vpc_update.network_security_group_id.is_some()
178178
|| vpc_update.routing_profile_overrides.is_some()
179+
|| vpc_update.power_resource_group.is_some()
179180
{
180181
let Some(vpc) = db::vpc::find_by(
181182
&mut txn,

crates/api-core/src/instance/mod.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,10 @@ pub(crate) struct InstanceAllocationRequest {
248248
pub(crate) allow_unhealthy_machine: bool,
249249
}
250250

251+
fn normalize_created_power_profile(power_profile: Option<String>) -> Option<String> {
252+
power_profile.filter(|profile| !profile.is_empty())
253+
}
254+
251255
impl TryFrom<rpc::InstanceAllocationRequest> for InstanceAllocationRequest {
252256
type Error = CarbideError;
253257

@@ -268,7 +272,10 @@ impl TryFrom<rpc::InstanceAllocationRequest> for InstanceAllocationRequest {
268272
.config
269273
.ok_or(RpcDataConversionError::MissingArgument("config"))?;
270274

271-
let config = InstanceConfig::try_from(config)?;
275+
let mut config = InstanceConfig::try_from(config)?;
276+
// Empty power-policy values are clear sentinels only on update. During
277+
// creation there is no association to clear, so persist them as unset.
278+
config.power_profile = normalize_created_power_profile(config.power_profile);
272279

273280
// If the Tenant provides an instance ID use this one
274281
// Otherwise create a random ID
@@ -2381,6 +2388,22 @@ mod tests {
23812388

23822389
use super::*;
23832390

2391+
#[test]
2392+
fn instance_creation_power_profile_semantics() {
2393+
value_scenarios!(
2394+
run = |power_profile| normalize_created_power_profile(power_profile);
2395+
"non-empty profile is preserved" {
2396+
Some("balanced".to_string()) => Some("balanced".to_string()),
2397+
}
2398+
"empty profile is treated as unset" {
2399+
Some(String::new()) => None,
2400+
}
2401+
"omitted profile remains unset" {
2402+
None => None,
2403+
}
2404+
);
2405+
}
2406+
23842407
#[test]
23852408
fn build_requested_linknet_prefix_accepts_host_end_rejects_dpu_end() {
23862409
// The host must take the odd (::1) end of the linknet; the even end is the

crates/api-core/src/tests/vpc.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,23 @@ async fn create_vpc(pool: sqlx::PgPool) -> Result<(), Box<dyn std::error::Error>
376376

377377
let no_org_vpc_id: VpcId = no_org_vpc.id.expect("should have id");
378378

379+
// A power-resource-group-only update still validates that the VPC exists.
380+
let unknown_vpc_id = VpcId::from(uuid::Uuid::new_v4());
381+
let status = env
382+
.api
383+
.update_vpc(tonic::Request::new(rpc::forge::VpcUpdateRequest {
384+
id: Some(unknown_vpc_id),
385+
if_version_match: None,
386+
metadata: None,
387+
network_security_group_id: None,
388+
default_nvlink_logical_partition_id: None,
389+
routing_profile_overrides: None,
390+
power_resource_group: Some("power-group".to_string()),
391+
}))
392+
.await
393+
.expect_err("updating an unknown VPC should fail");
394+
assert_eq!(status.code(), tonic::Code::NotFound);
395+
379396
// Try to update to invalid metadata
380397
for (invalid_metadata, expected_err) in common::metadata::invalid_metadata_testcases(true) {
381398
let invalid_updated_vpc = env

crates/api-db/migrations/20260806120001_power_provisioning_policy.sql renamed to crates/api-db/migrations/20260814015021_power_provisioning_policy.sql

File renamed without changes.

crates/rpc/proto/forge.proto

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,7 +1746,9 @@ message VpcConfig {
17461746
optional uint32 vni = 6;
17471747
optional string routing_profile_type = 7;
17481748
optional VpcRoutingProfileOverrides routing_profile_overrides = 8;
1749-
// Resource group managed by the external power provisioning service.
1749+
// Resource group managed by the external power provisioning service. Unset
1750+
// means that the VPC has no associated resource group; empty values are not
1751+
// persisted.
17501752
optional string power_resource_group = 9;
17511753
}
17521754

@@ -1853,7 +1855,8 @@ message VpcCreationRequest {
18531855
// Properties to overlay on the selected named routing profile. Properties
18541856
// left unset inherit from the base profile.
18551857
optional VpcRoutingProfileOverrides routing_profile_overrides = 18;
1856-
// Resource group managed by the external power provisioning service.
1858+
// Resource group managed by the external power provisioning service. During
1859+
// creation, omission or an empty value creates no association.
18571860
optional string power_resource_group = 19;
18581861
}
18591862

@@ -3125,7 +3128,8 @@ message InstanceConfig {
31253128

31263129
InstanceSpxConfig spxconfig = 25;
31273130

3128-
// Power profile managed by the external power provisioning service. In an
3131+
// Power profile managed by the external power provisioning service. During
3132+
// creation, omission or an empty value creates no association. In an
31293133
// InstanceConfigUpdateRequest, omission preserves the current profile for
31303134
// compatibility with older clients, an empty string explicitly clears it,
31313135
// and a non-empty value replaces it.

crates/rpc/src/model/vpc.rs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,9 @@ impl TryFrom<rpc::forge::VpcCreationRequest> for NewVpc {
268268
.routing_profile_overrides
269269
.map(TryInto::try_into)
270270
.transpose()?,
271-
power_resource_group: value.power_resource_group,
271+
power_resource_group: value
272+
.power_resource_group
273+
.filter(|resource_group| !resource_group.is_empty()),
272274
network_virtualization_type: virt_type,
273275
metadata,
274276
})
@@ -480,6 +482,30 @@ mod tests {
480482
assert_eq!(omitted.power_resource_group, None);
481483
}
482484

485+
#[test]
486+
fn vpc_creation_power_resource_group_semantics() {
487+
value_scenarios!(
488+
run = |power_resource_group| {
489+
NewVpc::try_from(rpc::forge::VpcCreationRequest {
490+
tenant_organization_id: "tenant-1".to_string(),
491+
power_resource_group,
492+
..Default::default()
493+
})
494+
.expect("creation request should be valid")
495+
.power_resource_group
496+
};
497+
"non-empty resource group is preserved" {
498+
Some("power-group".to_string()) => Some("power-group".to_string()),
499+
}
500+
"empty resource group is treated as unset" {
501+
Some(String::new()) => None,
502+
}
503+
"omitted resource group remains unset" {
504+
None => None,
505+
}
506+
);
507+
}
508+
483509
// `VpcSearchFilter::from` is a total conversion, so we project its output to
484510
// the fields the originals asserted: name, tenant_org_id, and the label as its
485511
// (key, value) pair (None when no label is present).

0 commit comments

Comments
 (0)