From da168ae7b10ef7c0e346de943420c986be93aa00 Mon Sep 17 00:00:00 2001 From: Pietro Date: Tue, 9 Jun 2026 14:02:13 +0000 Subject: [PATCH 1/3] feat(nns): add DeployGuestosToAllCloudEngines proposal Adds a new NNS proposal that upgrades every CloudEngine subnet to a given elected GuestOS version in one shot, instead of submitting one DeployGuestosToAllSubnetNodes proposal per subnet. Registry: new `deploy_guestos_to_all_cloud_engines` endpoint (governance-only) that iterates the subnet list, filters subnets whose subnet_type is CloudEngine, and updates their replica_version_id in a single atomic mutation. Non-CloudEngine subnets are left untouched. The affected set is resolved from the registry at execution time rather than captured in the payload, so the proposal always acts on the CloudEngine fleet as it exists when it runs. Governance: new NnsFunction variant NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES (= 59), routed to the registry endpoint under the IcOsVersionDeployment topic. --- rs/nns/governance/api/src/types.rs | 11 + .../ic_nns_governance/pb/v1/governance.proto | 6 + .../src/gen/ic_nns_governance.pb.v1.rs | 11 + rs/nns/governance/src/pb/conversions/mod.rs | 6 + .../src/proposals/execute_nns_function.rs | 21 ++ .../proposals/execute_nns_function_tests.rs | 20 ++ rs/nns/governance/unreleased_changelog.md | 4 + rs/registry/canister/canister/canister.rs | 13 + rs/registry/canister/canister/registry.did | 7 + .../canister/canister/registry_test.did | 7 + .../do_deploy_guestos_to_all_cloud_engines.rs | 236 ++++++++++++++++++ rs/registry/canister/src/mutations/mod.rs | 1 + rs/registry/canister/unreleased_changelog.md | 5 + 13 files changed, 348 insertions(+) create mode 100644 rs/registry/canister/src/mutations/do_deploy_guestos_to_all_cloud_engines.rs diff --git a/rs/nns/governance/api/src/types.rs b/rs/nns/governance/api/src/types.rs index e970d8e4a7c4..d4f46e16e724 100644 --- a/rs/nns/governance/api/src/types.rs +++ b/rs/nns/governance/api/src/types.rs @@ -4292,6 +4292,11 @@ pub enum NnsFunction { /// `SetupInitialDKG` requests without an explicit subnet id are routed to the /// calling subnet (NNS). SetDefaultInitialDkgSubnet = 58, + /// Deploy a GuestOS version to every CloudEngine subnet at once. The version + /// must be contained in the list of elected GuestOS versions. The set of + /// affected subnets is resolved at execution time from the registry (all + /// subnets whose subnet_type is CloudEngine), not captured in the payload. + DeployGuestosToAllCloudEngines = 59, } impl NnsFunction { /// String value of the enum field names used in the ProtoBuf definition. @@ -4380,6 +4385,9 @@ impl NnsFunction { NnsFunction::SetDefaultInitialDkgSubnet => { "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET" } + NnsFunction::DeployGuestosToAllCloudEngines => { + "NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES" + } } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -4463,6 +4471,9 @@ impl NnsFunction { "NNS_FUNCTION_SPLIT_SUBNET" => Some(Self::SplitSubnet), "NNS_FUNCTION_DELETE_SUBNET" => Some(Self::DeleteSubnet), "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET" => Some(Self::SetDefaultInitialDkgSubnet), + "NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES" => { + Some(Self::DeployGuestosToAllCloudEngines) + } _ => None, } } diff --git a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto index 71b5afff82f4..ced24390fba4 100644 --- a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto +++ b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto @@ -511,6 +511,12 @@ enum NnsFunction { // `SetupInitialDKG` requests without an explicit subnet id are routed to the // calling subnet (NNS). NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET = 58; + + // Deploy a GuestOS version to every CloudEngine subnet at once. The version + // must be contained in the list of elected GuestOS versions. The set of + // affected subnets is resolved at execution time from the registry (all + // subnets whose subnet_type is CloudEngine), not captured in the payload. + NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES = 59; } // Payload of a proposal that calls a function on another NNS diff --git a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs index 3a5611e1b0b2..c0546951bd0c 100644 --- a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs @@ -5286,6 +5286,11 @@ pub enum NnsFunction { /// `SetupInitialDKG` requests without an explicit subnet id are routed to the /// calling subnet (NNS). SetDefaultInitialDkgSubnet = 58, + /// Deploy a GuestOS version to every CloudEngine subnet at once. The version + /// must be contained in the list of elected GuestOS versions. The set of + /// affected subnets is resolved at execution time from the registry (all + /// subnets whose subnet_type is CloudEngine), not captured in the payload. + DeployGuestosToAllCloudEngines = 59, } impl NnsFunction { /// String value of the enum field names used in the ProtoBuf definition. @@ -5362,6 +5367,9 @@ impl NnsFunction { Self::SplitSubnet => "NNS_FUNCTION_SPLIT_SUBNET", Self::DeleteSubnet => "NNS_FUNCTION_DELETE_SUBNET", Self::SetDefaultInitialDkgSubnet => "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET", + Self::DeployGuestosToAllCloudEngines => { + "NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES" + } } } /// Creates an enum from field names used in the ProtoBuf definition. @@ -5445,6 +5453,9 @@ impl NnsFunction { "NNS_FUNCTION_SPLIT_SUBNET" => Some(Self::SplitSubnet), "NNS_FUNCTION_DELETE_SUBNET" => Some(Self::DeleteSubnet), "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET" => Some(Self::SetDefaultInitialDkgSubnet), + "NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES" => { + Some(Self::DeployGuestosToAllCloudEngines) + } _ => None, } } diff --git a/rs/nns/governance/src/pb/conversions/mod.rs b/rs/nns/governance/src/pb/conversions/mod.rs index c0ecae923268..17e2f5cf2f28 100644 --- a/rs/nns/governance/src/pb/conversions/mod.rs +++ b/rs/nns/governance/src/pb/conversions/mod.rs @@ -3885,6 +3885,9 @@ impl From for api::NnsFunction { pb::NnsFunction::SetDefaultInitialDkgSubnet => { api::NnsFunction::SetDefaultInitialDkgSubnet } + pb::NnsFunction::DeployGuestosToAllCloudEngines => { + api::NnsFunction::DeployGuestosToAllCloudEngines + } } } } @@ -3985,6 +3988,9 @@ impl From for pb::NnsFunction { api::NnsFunction::SetDefaultInitialDkgSubnet => { pb::NnsFunction::SetDefaultInitialDkgSubnet } + api::NnsFunction::DeployGuestosToAllCloudEngines => { + pb::NnsFunction::DeployGuestosToAllCloudEngines + } } } } diff --git a/rs/nns/governance/src/proposals/execute_nns_function.rs b/rs/nns/governance/src/proposals/execute_nns_function.rs index 7af600eed28e..8c7ac3de7d4b 100644 --- a/rs/nns/governance/src/proposals/execute_nns_function.rs +++ b/rs/nns/governance/src/proposals/execute_nns_function.rs @@ -459,6 +459,7 @@ pub enum ValidNnsFunction { SplitSubnet, DeleteSubnet, SetDefaultInitialDkgSubnet, + DeployGuestosToAllCloudEngines, } impl ValidNnsFunction { @@ -468,6 +469,7 @@ impl ValidNnsFunction { ValidNnsFunction::HardResetNnsRootToVersion | ValidNnsFunction::ReviseElectedGuestosVersions | ValidNnsFunction::DeployGuestosToAllSubnetNodes + | ValidNnsFunction::DeployGuestosToAllCloudEngines ) } @@ -506,6 +508,9 @@ impl ValidNnsFunction { ValidNnsFunction::DeployGuestosToAllSubnetNodes => { (REGISTRY_CANISTER_ID, "deploy_guestos_to_all_subnet_nodes") } + ValidNnsFunction::DeployGuestosToAllCloudEngines => { + (REGISTRY_CANISTER_ID, "deploy_guestos_to_all_cloud_engines") + } ValidNnsFunction::ReviseElectedHostosVersions => { (REGISTRY_CANISTER_ID, "revise_elected_hostos_versions") } @@ -630,6 +635,7 @@ impl ValidNnsFunction { ValidNnsFunction::DeployHostosToSomeNodes | ValidNnsFunction::DeployGuestosToAllSubnetNodes + | ValidNnsFunction::DeployGuestosToAllCloudEngines | ValidNnsFunction::DeployGuestosToSomeApiBoundaryNodes | ValidNnsFunction::DeployGuestosToAllUnassignedNodes => Topic::IcOsVersionDeployment, @@ -667,6 +673,9 @@ impl ValidNnsFunction { ValidNnsFunction::UpdateConfigOfSubnet => "Update Subnet Config", ValidNnsFunction::AssignNoid => "Assign Node Operator ID (NOID)", ValidNnsFunction::DeployGuestosToAllSubnetNodes => "Deploy GuestOS To All Subnet Nodes", + ValidNnsFunction::DeployGuestosToAllCloudEngines => { + "Deploy GuestOS To All Cloud Engines" + } ValidNnsFunction::ClearProvisionalWhitelist => "Clear Provisional Whitelist", ValidNnsFunction::RemoveNodesFromSubnet => "Remove Node from Subnet", ValidNnsFunction::SetAuthorizedSubnetworks => "Set Authorized Subnets", @@ -767,6 +776,15 @@ impl ValidNnsFunction { The version must be contained in the list of elected GuestOS versions.\n\n\ The upgrade is completed when the subnet creates the next regular CUP." } + ValidNnsFunction::DeployGuestosToAllCloudEngines => { + "Deploy a GuestOS version to every CloudEngine subnet at once. The proposal \ + changes the GuestOS version used on all subnets whose subnet type is \ + CloudEngine.\n\n\ + The version must be contained in the list of elected GuestOS versions.\n\n\ + The set of affected subnets is resolved at execution time from the registry, \ + not captured in the proposal payload. The upgrade is completed when each \ + subnet creates the next regular CUP." + } ValidNnsFunction::ClearProvisionalWhitelist => { "Clear the provisional whitelist, which allows the listed principals to create \ Canisters with cycles. The mechanism is only needed for bootstrap and testing and \ @@ -1036,6 +1054,9 @@ impl TryFrom for ValidNnsFunction { NnsFunction::SetDefaultInitialDkgSubnet => { Ok(ValidNnsFunction::SetDefaultInitialDkgSubnet) } + NnsFunction::DeployGuestosToAllCloudEngines => { + Ok(ValidNnsFunction::DeployGuestosToAllCloudEngines) + } // Obsolete functions - based on check_obsolete NnsFunction::BlessReplicaVersion | NnsFunction::RetireReplicaVersion => { diff --git a/rs/nns/governance/src/proposals/execute_nns_function_tests.rs b/rs/nns/governance/src/proposals/execute_nns_function_tests.rs index bf13e1666b06..008f6fa0fb38 100644 --- a/rs/nns/governance/src/proposals/execute_nns_function_tests.rs +++ b/rs/nns/governance/src/proposals/execute_nns_function_tests.rs @@ -124,6 +124,26 @@ fn test_execute_nns_function_try_from_errors() { } } +#[test] +fn test_deploy_guestos_to_all_cloud_engines_routing() { + use ic_nns_constants::REGISTRY_CANISTER_ID; + + let valid = ValidExecuteNnsFunction::try_from(ExecuteNnsFunction { + nns_function: NnsFunction::DeployGuestosToAllCloudEngines as i32, + payload: vec![], + }) + .expect("DeployGuestosToAllCloudEngines should be a valid NNS function"); + + assert_eq!( + valid.nns_function, + ValidNnsFunction::DeployGuestosToAllCloudEngines + ); + assert_eq!( + valid.nns_function.canister_and_function(), + (REGISTRY_CANISTER_ID, "deploy_guestos_to_all_cloud_engines"), + ); +} + // This tests a "normal" NNS function where the payload is translated through a candid file fetched // by the `canister_metadata` method on the management canister. #[tokio::test] diff --git a/rs/nns/governance/unreleased_changelog.md b/rs/nns/governance/unreleased_changelog.md index 25478837d207..3381b2a9708c 100644 --- a/rs/nns/governance/unreleased_changelog.md +++ b/rs/nns/governance/unreleased_changelog.md @@ -13,6 +13,10 @@ on the process that this file is part of, see proposes to set or unset the default subnet to which `SetupInitialDKG` management canister calls are routed when no subnet is specified explicitly in the request. +* Added a new `NnsFunction` variant `DeployGuestosToAllCloudEngines`, which + proposes to deploy a given (elected) GuestOS version to every CloudEngine + subnet at once. The set of affected subnets is resolved from the registry at + execution time rather than captured in the proposal payload. ## Changed diff --git a/rs/registry/canister/canister/canister.rs b/rs/registry/canister/canister/canister.rs index d128079e1def..9d6eec619d37 100644 --- a/rs/registry/canister/canister/canister.rs +++ b/rs/registry/canister/canister/canister.rs @@ -47,6 +47,7 @@ use registry_canister::{ do_change_subnet_membership::ChangeSubnetMembershipPayload, do_create_subnet::{CreateSubnetPayload, NewSubnet}, do_delete_subnet::DeleteSubnetPayload, + do_deploy_guestos_to_all_cloud_engines::DeployGuestosToAllCloudEnginesPayload, do_deploy_guestos_to_all_subnet_nodes::DeployGuestosToAllSubnetNodesPayload, do_deploy_guestos_to_all_unassigned_nodes::DeployGuestosToAllUnassignedNodesPayload, do_migrate_node_operator_directly::MigrateNodeOperatorPayload, @@ -563,6 +564,18 @@ fn deploy_guestos_to_all_subnet_nodes_(payload: DeployGuestosToAllSubnetNodesPay recertify_registry(); } +#[unsafe(export_name = "canister_update deploy_guestos_to_all_cloud_engines")] +fn deploy_guestos_to_all_cloud_engines() { + check_caller_is_governance_and_log("deploy_guestos_to_all_cloud_engines"); + over(candid_one, deploy_guestos_to_all_cloud_engines_); +} + +#[candid_method(update, rename = "deploy_guestos_to_all_cloud_engines")] +fn deploy_guestos_to_all_cloud_engines_(payload: DeployGuestosToAllCloudEnginesPayload) { + registry_mut().do_deploy_guestos_to_all_cloud_engines(payload); + recertify_registry(); +} + // TODO[NNS1-3000]: Remove this endpoint once mainnet NNS Governance starts calling the new // TODO[NNS1-3000]: `revise_elected_hostos_versions` endpoint. #[unsafe(export_name = "canister_update update_elected_hostos_versions")] diff --git a/rs/registry/canister/canister/registry.did b/rs/registry/canister/canister/registry.did index 9aea72cb4b98..f5d3c602906b 100644 --- a/rs/registry/canister/canister/registry.did +++ b/rs/registry/canister/canister/registry.did @@ -155,6 +155,10 @@ type DataCenterRecord = record { owner : text; }; +type DeployGuestosToAllCloudEnginesPayload = record { + replica_version_id : text; +}; + type DeployGuestosToAllSubnetNodesPayload = record { subnet_id : principal; replica_version_id : text; @@ -620,6 +624,9 @@ service : { complete_canister_migration : (CompleteCanisterMigrationPayload) -> (); create_subnet : (CreateSubnetPayload) -> (CreateSubnetResponse); delete_subnet : (DeleteSubnetPayload) -> (DeleteSubnetResponse); + deploy_guestos_to_all_cloud_engines : ( + DeployGuestosToAllCloudEnginesPayload + ) -> (); deploy_guestos_to_all_subnet_nodes : ( DeployGuestosToAllSubnetNodesPayload ) -> (); diff --git a/rs/registry/canister/canister/registry_test.did b/rs/registry/canister/canister/registry_test.did index 386a330a1972..ebe875cf5d5a 100644 --- a/rs/registry/canister/canister/registry_test.did +++ b/rs/registry/canister/canister/registry_test.did @@ -155,6 +155,10 @@ type DataCenterRecord = record { owner : text; }; +type DeployGuestosToAllCloudEnginesPayload = record { + replica_version_id : text; +}; + type DeployGuestosToAllSubnetNodesPayload = record { subnet_id : principal; replica_version_id : text; @@ -620,6 +624,9 @@ service : { complete_canister_migration : (CompleteCanisterMigrationPayload) -> (); create_subnet : (CreateSubnetPayload) -> (CreateSubnetResponse); delete_subnet : (DeleteSubnetPayload) -> (DeleteSubnetResponse); + deploy_guestos_to_all_cloud_engines : ( + DeployGuestosToAllCloudEnginesPayload + ) -> (); deploy_guestos_to_all_subnet_nodes : ( DeployGuestosToAllSubnetNodesPayload ) -> (); diff --git a/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_cloud_engines.rs b/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_cloud_engines.rs new file mode 100644 index 000000000000..79e83b5f7c59 --- /dev/null +++ b/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_cloud_engines.rs @@ -0,0 +1,236 @@ +use crate::{ + common::LOG_PREFIX, + mutations::common::{check_replica_version_is_elected, get_subnet_ids_from_subnet_list}, + registry::Registry, +}; + +use candid::{CandidType, Deserialize}; +#[cfg(target_arch = "wasm32")] +use dfn_core::println; +use ic_registry_keys::make_subnet_record_key; +use ic_registry_subnet_type::SubnetType; +use ic_registry_transport::pb::v1::{RegistryMutation, registry_mutation}; +use prost::Message; +use serde::Serialize; + +impl Registry { + /// Deploys the given (elected) GuestOS version to every CloudEngine subnet. + /// + /// Unlike `do_deploy_guestos_to_all_subnet_nodes`, which targets a single + /// subnet, this enumerates the current `subnet_list` and updates the + /// `replica_version_id` of every subnet whose `subnet_type` is + /// `SubnetType::CloudEngine`. The set of affected subnets is therefore + /// resolved at execution time rather than captured in the payload, so the + /// proposal always acts on the CloudEngine fleet as it exists when it runs. + /// + /// All updates are applied as a single atomic registry mutation: either the + /// whole fleet moves to the new version, or nothing does. + pub fn do_deploy_guestos_to_all_cloud_engines( + &mut self, + payload: DeployGuestosToAllCloudEnginesPayload, + ) { + println!("{LOG_PREFIX}do_deploy_guestos_to_all_cloud_engines: {payload:?}"); + + check_replica_version_is_elected(self, &payload.replica_version_id); + + let cloud_engine_type = i32::from(SubnetType::CloudEngine); + let mut mutations = vec![]; + for subnet_id in get_subnet_ids_from_subnet_list(self.get_subnet_list_record()) { + let mut subnet_record = self.get_subnet_or_panic(subnet_id); + + // Only CloudEngine subnets are affected; everything else is left untouched. + if subnet_record.subnet_type != cloud_engine_type { + continue; + } + + // Skip subnets that are already on the requested version to avoid + // churning the registry with no-op updates. + if subnet_record.replica_version_id == payload.replica_version_id { + continue; + } + + subnet_record.replica_version_id = payload.replica_version_id.clone(); + mutations.push(RegistryMutation { + mutation_type: registry_mutation::Type::Update as i32, + key: make_subnet_record_key(subnet_id).into_bytes(), + value: subnet_record.encode_to_vec(), + }); + } + + println!( + "{LOG_PREFIX}do_deploy_guestos_to_all_cloud_engines: updating {} CloudEngine subnet(s) to version {}", + mutations.len(), + payload.replica_version_id, + ); + + // Check invariants before applying mutations. An empty mutation list is + // a no-op (e.g. when no CloudEngine subnets exist, or all are already on + // the requested version). + self.maybe_apply_mutation_internal(mutations) + } +} + +/// The argument of a command to update the replica version of every CloudEngine +/// subnet to a specific version. +/// +/// The subnets will be mutated only if the given version is, indeed, elected. +#[derive(Clone, Eq, PartialEq, Debug, CandidType, Deserialize, Serialize)] +pub struct DeployGuestosToAllCloudEnginesPayload { + /// The new Replica version to deploy to all CloudEngine subnets. + pub replica_version_id: String, +} + +#[cfg(test)] +mod tests { + use ic_base_types::{NodeId, SubnetId}; + use ic_protobuf::registry::crypto::v1::PublicKey; + use ic_protobuf::registry::node::v1::NodeRewardType; + use ic_protobuf::registry::replica_version::v1::ReplicaVersionRecord; + use ic_protobuf::registry::subnet::v1::CanisterCyclesCostSchedule; + use ic_registry_keys::make_replica_version_key; + use ic_registry_subnet_type::SubnetType; + use ic_registry_transport::insert; + use ic_test_utilities_types::ids::subnet_test_id; + use maplit::btreemap; + use prost::Message; + + use crate::common::test_helpers::{ + add_fake_subnet, get_invariant_compliant_subnet_record, invariant_compliant_registry, + prepare_registry_with_nodes, prepare_registry_with_nodes_and_reward_type, + }; + use crate::registry::Registry; + + use super::DeployGuestosToAllCloudEnginesPayload; + + const TARGET_VERSION: &str = "version"; + + /// Elects `TARGET_VERSION` so the deploy passes the elected-version check. + fn elect_target_version(registry: &mut Registry) { + registry.maybe_apply_mutation_internal(vec![insert( + make_replica_version_key(TARGET_VERSION), + ReplicaVersionRecord { + release_package_sha256_hex: "".into(), + release_package_urls: vec![], + guest_launch_measurements: None, + } + .encode_to_vec(), + )]); + } + + /// Adds a CloudEngine subnet backed by `node_id` and returns its id. The + /// node must have reward type `Type4` and the subnet uses the free cycles + /// cost schedule, both required by registry invariants for CloudEngines. + fn add_cloud_engine_subnet( + registry: &mut Registry, + node_id: NodeId, + dkg_pk: PublicKey, + subnet_index: u64, + ) -> SubnetId { + let mut subnet_record = get_invariant_compliant_subnet_record(vec![node_id]); + subnet_record.subnet_type = i32::from(SubnetType::CloudEngine); + subnet_record.canister_cycles_cost_schedule = i32::from(CanisterCyclesCostSchedule::Free); + add_subnet(registry, node_id, dkg_pk, subnet_index, subnet_record) + } + + /// Adds an application subnet backed by `node_id` and returns its id. + fn add_application_subnet( + registry: &mut Registry, + node_id: NodeId, + dkg_pk: PublicKey, + subnet_index: u64, + ) -> SubnetId { + let mut subnet_record = get_invariant_compliant_subnet_record(vec![node_id]); + subnet_record.subnet_type = i32::from(SubnetType::Application); + add_subnet(registry, node_id, dkg_pk, subnet_index, subnet_record) + } + + fn add_subnet( + registry: &mut Registry, + node_id: NodeId, + dkg_pk: PublicKey, + subnet_index: u64, + subnet_record: ic_protobuf::registry::subnet::v1::SubnetRecord, + ) -> SubnetId { + let mut subnet_list_record = registry.get_subnet_list_record(); + let subnet_id = subnet_test_id(subnet_index); + registry.maybe_apply_mutation_internal(add_fake_subnet( + subnet_id, + &mut subnet_list_record, + subnet_record, + &btreemap!(node_id => dkg_pk), + )); + subnet_id + } + + fn replica_version_of(registry: &Registry, subnet_id: SubnetId) -> String { + registry.get_subnet_or_panic(subnet_id).replica_version_id + } + + #[test] + #[should_panic(expected = "'version' is NOT elected")] + fn should_panic_if_version_not_elected() { + let mut registry = invariant_compliant_registry(0); + + registry.do_deploy_guestos_to_all_cloud_engines(DeployGuestosToAllCloudEnginesPayload { + replica_version_id: TARGET_VERSION.into(), + }); + } + + #[test] + fn should_upgrade_only_cloud_engine_subnets() { + let mut registry = invariant_compliant_registry(0); + elect_target_version(&mut registry); + + // CloudEngine subnets require nodes with reward type Type4; the + // application subnet must use a non-Type4 node. Distinct mutation-id + // ranges keep the generated node IPs/domains from colliding. + let (ce_req, ce_nodes) = + prepare_registry_with_nodes_and_reward_type(1, 2, NodeRewardType::Type4); + registry.maybe_apply_mutation_internal(ce_req.mutations); + let (app_req, app_nodes) = prepare_registry_with_nodes(10, 1); + registry.maybe_apply_mutation_internal(app_req.mutations); + + let mut ce_nodes = ce_nodes.into_iter(); + let (n1, pk1) = ce_nodes.next().unwrap(); + let (n2, pk2) = ce_nodes.next().unwrap(); + let (app_node, app_pk) = app_nodes.into_iter().next().unwrap(); + + let cloud_engine_a = add_cloud_engine_subnet(&mut registry, n1, pk1, 1001); + let cloud_engine_b = add_cloud_engine_subnet(&mut registry, n2, pk2, 1002); + let application = add_application_subnet(&mut registry, app_node, app_pk, 1003); + + registry.do_deploy_guestos_to_all_cloud_engines(DeployGuestosToAllCloudEnginesPayload { + replica_version_id: TARGET_VERSION.into(), + }); + + assert_eq!( + replica_version_of(®istry, cloud_engine_a), + TARGET_VERSION + ); + assert_eq!( + replica_version_of(®istry, cloud_engine_b), + TARGET_VERSION + ); + // The application subnet must be untouched. + assert_ne!(replica_version_of(®istry, application), TARGET_VERSION); + } + + #[test] + fn should_be_noop_when_no_cloud_engine_subnets_exist() { + let mut registry = invariant_compliant_registry(0); + elect_target_version(&mut registry); + + let (app_req, app_nodes) = prepare_registry_with_nodes(1, 1); + registry.maybe_apply_mutation_internal(app_req.mutations); + let (app_node, app_pk) = app_nodes.into_iter().next().unwrap(); + add_application_subnet(&mut registry, app_node, app_pk, 1001); + let version_before = registry.latest_version(); + + registry.do_deploy_guestos_to_all_cloud_engines(DeployGuestosToAllCloudEnginesPayload { + replica_version_id: TARGET_VERSION.into(), + }); + + // No CloudEngine subnets means no mutations, hence no version bump. + assert_eq!(registry.latest_version(), version_before); + } +} diff --git a/rs/registry/canister/src/mutations/mod.rs b/rs/registry/canister/src/mutations/mod.rs index 871e33b0cf6a..f0ce12147e25 100644 --- a/rs/registry/canister/src/mutations/mod.rs +++ b/rs/registry/canister/src/mutations/mod.rs @@ -10,6 +10,7 @@ pub mod do_change_subnet_membership; pub mod do_clear_provisional_whitelist; pub mod do_create_subnet; pub mod do_delete_subnet; +pub mod do_deploy_guestos_to_all_cloud_engines; pub mod do_deploy_guestos_to_all_subnet_nodes; pub mod do_deploy_guestos_to_all_unassigned_nodes; pub mod do_migrate_canisters; diff --git a/rs/registry/canister/unreleased_changelog.md b/rs/registry/canister/unreleased_changelog.md index 93c14851bbe2..6523f430505a 100644 --- a/rs/registry/canister/unreleased_changelog.md +++ b/rs/registry/canister/unreleased_changelog.md @@ -21,6 +21,11 @@ on the process that this file is part of, see * Added `vcpu_type` to `GuestLaunchMeasurementMetadata` to record the virtual CPU type used for a guest launch measurement. * Added a new endpoint `get_subnet` to the registry canister, returning the subnet record of a given subnet. +* Added a new endpoint `deploy_guestos_to_all_cloud_engines` to the registry + canister, which sets the `replica_version_id` of every CloudEngine subnet to a + given (elected) GuestOS version in a single atomic mutation. The set of + affected subnets is resolved from the registry at execution time (all subnets + whose `subnet_type` is `CloudEngine`); other subnet types are left untouched. ## Changed From dbaa0a5558d2bb777185132620a8d54d810cc7ac Mon Sep 17 00:00:00 2001 From: Pietro Date: Tue, 9 Jun 2026 16:25:04 +0000 Subject: [PATCH 2/3] refactor(nns): take explicit subnet list instead of all CloudEngines Reworks the proposal to operate on an explicit list of subnets rather than implicitly targeting all CloudEngine subnets: - NnsFunction NNS_FUNCTION_UPDATE_GUESTOS_VERSION_FOR_SUBNETS (= 59) replaces NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES. - Registry endpoint update_guestos_version_for_subnets takes UpdateGuestosVersionForSubnetsPayload { subnet_ids: Vec, replica_version_id } and updates exactly the listed subnets (no subnet_type gating) in one atomic mutation. Rejects empty/duplicate lists and unknown subnets; skips subnets already on the requested version. This makes the affected set explicit and reviewable in the proposal, and generalizes the batch upgrade beyond CloudEngine subnets. --- rs/nns/governance/api/src/types.rs | 19 +- .../ic_nns_governance/pb/v1/governance.proto | 11 +- .../src/gen/ic_nns_governance.pb.v1.rs | 19 +- rs/nns/governance/src/pb/conversions/mod.rs | 8 +- .../src/proposals/execute_nns_function.rs | 29 +-- .../proposals/execute_nns_function_tests.rs | 10 +- rs/nns/governance/unreleased_changelog.md | 7 +- rs/registry/canister/canister/canister.rs | 16 +- rs/registry/canister/canister/registry.did | 13 +- .../canister/canister/registry_test.did | 13 +- .../do_deploy_guestos_to_all_cloud_engines.rs | 236 ----------------- .../do_update_guestos_version_for_subnets.rs | 246 ++++++++++++++++++ rs/registry/canister/src/mutations/mod.rs | 2 +- rs/registry/canister/unreleased_changelog.md | 10 +- 14 files changed, 323 insertions(+), 316 deletions(-) delete mode 100644 rs/registry/canister/src/mutations/do_deploy_guestos_to_all_cloud_engines.rs create mode 100644 rs/registry/canister/src/mutations/do_update_guestos_version_for_subnets.rs diff --git a/rs/nns/governance/api/src/types.rs b/rs/nns/governance/api/src/types.rs index d4f46e16e724..e76511846f2b 100644 --- a/rs/nns/governance/api/src/types.rs +++ b/rs/nns/governance/api/src/types.rs @@ -4292,11 +4292,12 @@ pub enum NnsFunction { /// `SetupInitialDKG` requests without an explicit subnet id are routed to the /// calling subnet (NNS). SetDefaultInitialDkgSubnet = 58, - /// Deploy a GuestOS version to every CloudEngine subnet at once. The version - /// must be contained in the list of elected GuestOS versions. The set of - /// affected subnets is resolved at execution time from the registry (all - /// subnets whose subnet_type is CloudEngine), not captured in the payload. - DeployGuestosToAllCloudEngines = 59, + /// Deploy a GuestOS version to an explicit list of subnets at once. The + /// proposal changes the GuestOS version used on every subnet in the provided + /// list. The version must be contained in the list of elected GuestOS + /// versions. The upgrade is completed when each subnet creates the next + /// regular CUP. + UpdateGuestosVersionForSubnets = 59, } impl NnsFunction { /// String value of the enum field names used in the ProtoBuf definition. @@ -4385,8 +4386,8 @@ impl NnsFunction { NnsFunction::SetDefaultInitialDkgSubnet => { "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET" } - NnsFunction::DeployGuestosToAllCloudEngines => { - "NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES" + NnsFunction::UpdateGuestosVersionForSubnets => { + "NNS_FUNCTION_UPDATE_GUESTOS_VERSION_FOR_SUBNETS" } } } @@ -4471,8 +4472,8 @@ impl NnsFunction { "NNS_FUNCTION_SPLIT_SUBNET" => Some(Self::SplitSubnet), "NNS_FUNCTION_DELETE_SUBNET" => Some(Self::DeleteSubnet), "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET" => Some(Self::SetDefaultInitialDkgSubnet), - "NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES" => { - Some(Self::DeployGuestosToAllCloudEngines) + "NNS_FUNCTION_UPDATE_GUESTOS_VERSION_FOR_SUBNETS" => { + Some(Self::UpdateGuestosVersionForSubnets) } _ => None, } diff --git a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto index ced24390fba4..e9f7061f2718 100644 --- a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto +++ b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto @@ -512,11 +512,12 @@ enum NnsFunction { // calling subnet (NNS). NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET = 58; - // Deploy a GuestOS version to every CloudEngine subnet at once. The version - // must be contained in the list of elected GuestOS versions. The set of - // affected subnets is resolved at execution time from the registry (all - // subnets whose subnet_type is CloudEngine), not captured in the payload. - NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES = 59; + // Deploy a GuestOS version to an explicit list of subnets at once. The + // proposal changes the GuestOS version used on every subnet in the provided + // list. The version must be contained in the list of elected GuestOS + // versions. The upgrade is completed when each subnet creates the next + // regular CUP. + NNS_FUNCTION_UPDATE_GUESTOS_VERSION_FOR_SUBNETS = 59; } // Payload of a proposal that calls a function on another NNS diff --git a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs index c0546951bd0c..7f7f34d0d246 100644 --- a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs @@ -5286,11 +5286,12 @@ pub enum NnsFunction { /// `SetupInitialDKG` requests without an explicit subnet id are routed to the /// calling subnet (NNS). SetDefaultInitialDkgSubnet = 58, - /// Deploy a GuestOS version to every CloudEngine subnet at once. The version - /// must be contained in the list of elected GuestOS versions. The set of - /// affected subnets is resolved at execution time from the registry (all - /// subnets whose subnet_type is CloudEngine), not captured in the payload. - DeployGuestosToAllCloudEngines = 59, + /// Deploy a GuestOS version to an explicit list of subnets at once. The + /// proposal changes the GuestOS version used on every subnet in the provided + /// list. The version must be contained in the list of elected GuestOS + /// versions. The upgrade is completed when each subnet creates the next + /// regular CUP. + UpdateGuestosVersionForSubnets = 59, } impl NnsFunction { /// String value of the enum field names used in the ProtoBuf definition. @@ -5367,8 +5368,8 @@ impl NnsFunction { Self::SplitSubnet => "NNS_FUNCTION_SPLIT_SUBNET", Self::DeleteSubnet => "NNS_FUNCTION_DELETE_SUBNET", Self::SetDefaultInitialDkgSubnet => "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET", - Self::DeployGuestosToAllCloudEngines => { - "NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES" + Self::UpdateGuestosVersionForSubnets => { + "NNS_FUNCTION_UPDATE_GUESTOS_VERSION_FOR_SUBNETS" } } } @@ -5453,8 +5454,8 @@ impl NnsFunction { "NNS_FUNCTION_SPLIT_SUBNET" => Some(Self::SplitSubnet), "NNS_FUNCTION_DELETE_SUBNET" => Some(Self::DeleteSubnet), "NNS_FUNCTION_SET_DEFAULT_INITIAL_DKG_SUBNET" => Some(Self::SetDefaultInitialDkgSubnet), - "NNS_FUNCTION_DEPLOY_GUESTOS_TO_ALL_CLOUD_ENGINES" => { - Some(Self::DeployGuestosToAllCloudEngines) + "NNS_FUNCTION_UPDATE_GUESTOS_VERSION_FOR_SUBNETS" => { + Some(Self::UpdateGuestosVersionForSubnets) } _ => None, } diff --git a/rs/nns/governance/src/pb/conversions/mod.rs b/rs/nns/governance/src/pb/conversions/mod.rs index 17e2f5cf2f28..ba60b6007dd5 100644 --- a/rs/nns/governance/src/pb/conversions/mod.rs +++ b/rs/nns/governance/src/pb/conversions/mod.rs @@ -3885,8 +3885,8 @@ impl From for api::NnsFunction { pb::NnsFunction::SetDefaultInitialDkgSubnet => { api::NnsFunction::SetDefaultInitialDkgSubnet } - pb::NnsFunction::DeployGuestosToAllCloudEngines => { - api::NnsFunction::DeployGuestosToAllCloudEngines + pb::NnsFunction::UpdateGuestosVersionForSubnets => { + api::NnsFunction::UpdateGuestosVersionForSubnets } } } @@ -3988,8 +3988,8 @@ impl From for pb::NnsFunction { api::NnsFunction::SetDefaultInitialDkgSubnet => { pb::NnsFunction::SetDefaultInitialDkgSubnet } - api::NnsFunction::DeployGuestosToAllCloudEngines => { - pb::NnsFunction::DeployGuestosToAllCloudEngines + api::NnsFunction::UpdateGuestosVersionForSubnets => { + pb::NnsFunction::UpdateGuestosVersionForSubnets } } } diff --git a/rs/nns/governance/src/proposals/execute_nns_function.rs b/rs/nns/governance/src/proposals/execute_nns_function.rs index 8c7ac3de7d4b..d5a14120bdcf 100644 --- a/rs/nns/governance/src/proposals/execute_nns_function.rs +++ b/rs/nns/governance/src/proposals/execute_nns_function.rs @@ -459,7 +459,7 @@ pub enum ValidNnsFunction { SplitSubnet, DeleteSubnet, SetDefaultInitialDkgSubnet, - DeployGuestosToAllCloudEngines, + UpdateGuestosVersionForSubnets, } impl ValidNnsFunction { @@ -469,7 +469,7 @@ impl ValidNnsFunction { ValidNnsFunction::HardResetNnsRootToVersion | ValidNnsFunction::ReviseElectedGuestosVersions | ValidNnsFunction::DeployGuestosToAllSubnetNodes - | ValidNnsFunction::DeployGuestosToAllCloudEngines + | ValidNnsFunction::UpdateGuestosVersionForSubnets ) } @@ -508,8 +508,8 @@ impl ValidNnsFunction { ValidNnsFunction::DeployGuestosToAllSubnetNodes => { (REGISTRY_CANISTER_ID, "deploy_guestos_to_all_subnet_nodes") } - ValidNnsFunction::DeployGuestosToAllCloudEngines => { - (REGISTRY_CANISTER_ID, "deploy_guestos_to_all_cloud_engines") + ValidNnsFunction::UpdateGuestosVersionForSubnets => { + (REGISTRY_CANISTER_ID, "update_guestos_version_for_subnets") } ValidNnsFunction::ReviseElectedHostosVersions => { (REGISTRY_CANISTER_ID, "revise_elected_hostos_versions") @@ -635,7 +635,7 @@ impl ValidNnsFunction { ValidNnsFunction::DeployHostosToSomeNodes | ValidNnsFunction::DeployGuestosToAllSubnetNodes - | ValidNnsFunction::DeployGuestosToAllCloudEngines + | ValidNnsFunction::UpdateGuestosVersionForSubnets | ValidNnsFunction::DeployGuestosToSomeApiBoundaryNodes | ValidNnsFunction::DeployGuestosToAllUnassignedNodes => Topic::IcOsVersionDeployment, @@ -673,8 +673,8 @@ impl ValidNnsFunction { ValidNnsFunction::UpdateConfigOfSubnet => "Update Subnet Config", ValidNnsFunction::AssignNoid => "Assign Node Operator ID (NOID)", ValidNnsFunction::DeployGuestosToAllSubnetNodes => "Deploy GuestOS To All Subnet Nodes", - ValidNnsFunction::DeployGuestosToAllCloudEngines => { - "Deploy GuestOS To All Cloud Engines" + ValidNnsFunction::UpdateGuestosVersionForSubnets => { + "Update GuestOS Version For Subnets" } ValidNnsFunction::ClearProvisionalWhitelist => "Clear Provisional Whitelist", ValidNnsFunction::RemoveNodesFromSubnet => "Remove Node from Subnet", @@ -776,14 +776,11 @@ impl ValidNnsFunction { The version must be contained in the list of elected GuestOS versions.\n\n\ The upgrade is completed when the subnet creates the next regular CUP." } - ValidNnsFunction::DeployGuestosToAllCloudEngines => { - "Deploy a GuestOS version to every CloudEngine subnet at once. The proposal \ - changes the GuestOS version used on all subnets whose subnet type is \ - CloudEngine.\n\n\ + ValidNnsFunction::UpdateGuestosVersionForSubnets => { + "Deploy a GuestOS version to an explicit list of subnets at once. The proposal \ + changes the GuestOS version used on every subnet in the provided list.\n\n\ The version must be contained in the list of elected GuestOS versions.\n\n\ - The set of affected subnets is resolved at execution time from the registry, \ - not captured in the proposal payload. The upgrade is completed when each \ - subnet creates the next regular CUP." + The upgrade is completed when each subnet creates the next regular CUP." } ValidNnsFunction::ClearProvisionalWhitelist => { "Clear the provisional whitelist, which allows the listed principals to create \ @@ -1054,8 +1051,8 @@ impl TryFrom for ValidNnsFunction { NnsFunction::SetDefaultInitialDkgSubnet => { Ok(ValidNnsFunction::SetDefaultInitialDkgSubnet) } - NnsFunction::DeployGuestosToAllCloudEngines => { - Ok(ValidNnsFunction::DeployGuestosToAllCloudEngines) + NnsFunction::UpdateGuestosVersionForSubnets => { + Ok(ValidNnsFunction::UpdateGuestosVersionForSubnets) } // Obsolete functions - based on check_obsolete diff --git a/rs/nns/governance/src/proposals/execute_nns_function_tests.rs b/rs/nns/governance/src/proposals/execute_nns_function_tests.rs index 008f6fa0fb38..0c5c054a5e95 100644 --- a/rs/nns/governance/src/proposals/execute_nns_function_tests.rs +++ b/rs/nns/governance/src/proposals/execute_nns_function_tests.rs @@ -125,22 +125,22 @@ fn test_execute_nns_function_try_from_errors() { } #[test] -fn test_deploy_guestos_to_all_cloud_engines_routing() { +fn test_update_guestos_version_for_subnets_routing() { use ic_nns_constants::REGISTRY_CANISTER_ID; let valid = ValidExecuteNnsFunction::try_from(ExecuteNnsFunction { - nns_function: NnsFunction::DeployGuestosToAllCloudEngines as i32, + nns_function: NnsFunction::UpdateGuestosVersionForSubnets as i32, payload: vec![], }) - .expect("DeployGuestosToAllCloudEngines should be a valid NNS function"); + .expect("UpdateGuestosVersionForSubnets should be a valid NNS function"); assert_eq!( valid.nns_function, - ValidNnsFunction::DeployGuestosToAllCloudEngines + ValidNnsFunction::UpdateGuestosVersionForSubnets ); assert_eq!( valid.nns_function.canister_and_function(), - (REGISTRY_CANISTER_ID, "deploy_guestos_to_all_cloud_engines"), + (REGISTRY_CANISTER_ID, "update_guestos_version_for_subnets"), ); } diff --git a/rs/nns/governance/unreleased_changelog.md b/rs/nns/governance/unreleased_changelog.md index 3381b2a9708c..e16e96a9dc7e 100644 --- a/rs/nns/governance/unreleased_changelog.md +++ b/rs/nns/governance/unreleased_changelog.md @@ -13,10 +13,9 @@ on the process that this file is part of, see proposes to set or unset the default subnet to which `SetupInitialDKG` management canister calls are routed when no subnet is specified explicitly in the request. -* Added a new `NnsFunction` variant `DeployGuestosToAllCloudEngines`, which - proposes to deploy a given (elected) GuestOS version to every CloudEngine - subnet at once. The set of affected subnets is resolved from the registry at - execution time rather than captured in the proposal payload. +* Added a new `NnsFunction` variant `UpdateGuestosVersionForSubnets`, which + proposes to deploy a given (elected) GuestOS version to an explicit list of + subnets at once. ## Changed diff --git a/rs/registry/canister/canister/canister.rs b/rs/registry/canister/canister/canister.rs index 9d6eec619d37..cb71f23848f6 100644 --- a/rs/registry/canister/canister/canister.rs +++ b/rs/registry/canister/canister/canister.rs @@ -47,7 +47,6 @@ use registry_canister::{ do_change_subnet_membership::ChangeSubnetMembershipPayload, do_create_subnet::{CreateSubnetPayload, NewSubnet}, do_delete_subnet::DeleteSubnetPayload, - do_deploy_guestos_to_all_cloud_engines::DeployGuestosToAllCloudEnginesPayload, do_deploy_guestos_to_all_subnet_nodes::DeployGuestosToAllSubnetNodesPayload, do_deploy_guestos_to_all_unassigned_nodes::DeployGuestosToAllUnassignedNodesPayload, do_migrate_node_operator_directly::MigrateNodeOperatorPayload, @@ -67,6 +66,7 @@ use registry_canister::{ do_update_elected_hostos_versions::{ ReviseElectedHostosVersionsPayload, UpdateElectedHostosVersionsPayload, }, + do_update_guestos_version_for_subnets::UpdateGuestosVersionForSubnetsPayload, do_update_node_operator_config::UpdateNodeOperatorConfigPayload, do_update_node_operator_config_directly::UpdateNodeOperatorConfigDirectlyPayload, do_update_nodes_hostos_version::{ @@ -564,15 +564,15 @@ fn deploy_guestos_to_all_subnet_nodes_(payload: DeployGuestosToAllSubnetNodesPay recertify_registry(); } -#[unsafe(export_name = "canister_update deploy_guestos_to_all_cloud_engines")] -fn deploy_guestos_to_all_cloud_engines() { - check_caller_is_governance_and_log("deploy_guestos_to_all_cloud_engines"); - over(candid_one, deploy_guestos_to_all_cloud_engines_); +#[unsafe(export_name = "canister_update update_guestos_version_for_subnets")] +fn update_guestos_version_for_subnets() { + check_caller_is_governance_and_log("update_guestos_version_for_subnets"); + over(candid_one, update_guestos_version_for_subnets_); } -#[candid_method(update, rename = "deploy_guestos_to_all_cloud_engines")] -fn deploy_guestos_to_all_cloud_engines_(payload: DeployGuestosToAllCloudEnginesPayload) { - registry_mut().do_deploy_guestos_to_all_cloud_engines(payload); +#[candid_method(update, rename = "update_guestos_version_for_subnets")] +fn update_guestos_version_for_subnets_(payload: UpdateGuestosVersionForSubnetsPayload) { + registry_mut().do_update_guestos_version_for_subnets(payload); recertify_registry(); } diff --git a/rs/registry/canister/canister/registry.did b/rs/registry/canister/canister/registry.did index f5d3c602906b..c5662556510f 100644 --- a/rs/registry/canister/canister/registry.did +++ b/rs/registry/canister/canister/registry.did @@ -155,10 +155,6 @@ type DataCenterRecord = record { owner : text; }; -type DeployGuestosToAllCloudEnginesPayload = record { - replica_version_id : text; -}; - type DeployGuestosToAllSubnetNodesPayload = record { subnet_id : principal; replica_version_id : text; @@ -445,6 +441,11 @@ type UpdateFirewallRulesPayload = record { rules : vec FirewallRule; }; +type UpdateGuestosVersionForSubnetsPayload = record { + subnet_ids : vec principal; + replica_version_id : text; +}; + type UpdateNodeDirectlyPayload = record { idkg_dealing_encryption_pk : opt blob; }; @@ -624,9 +625,6 @@ service : { complete_canister_migration : (CompleteCanisterMigrationPayload) -> (); create_subnet : (CreateSubnetPayload) -> (CreateSubnetResponse); delete_subnet : (DeleteSubnetPayload) -> (DeleteSubnetResponse); - deploy_guestos_to_all_cloud_engines : ( - DeployGuestosToAllCloudEnginesPayload - ) -> (); deploy_guestos_to_all_subnet_nodes : ( DeployGuestosToAllSubnetNodesPayload ) -> (); @@ -662,6 +660,7 @@ service : { update_elected_hostos_versions : (UpdateElectedHostosVersionsPayload) -> (); revise_elected_hostos_versions : (ReviseElectedHostosVersionsPayload) -> (); update_firewall_rules : (UpdateFirewallRulesPayload) -> (); + update_guestos_version_for_subnets : (UpdateGuestosVersionForSubnetsPayload) -> (); update_node_directly : (UpdateNodeDirectlyPayload) -> (); update_node_domain_directly : (UpdateNodeDomainDirectlyPayload) -> (UpdateNodeDomainDirectlyResponse); update_node_ipv4_config_directly : (UpdateNodeIPv4ConfigDirectlyPayload) -> ( diff --git a/rs/registry/canister/canister/registry_test.did b/rs/registry/canister/canister/registry_test.did index ebe875cf5d5a..70f5377501ce 100644 --- a/rs/registry/canister/canister/registry_test.did +++ b/rs/registry/canister/canister/registry_test.did @@ -155,10 +155,6 @@ type DataCenterRecord = record { owner : text; }; -type DeployGuestosToAllCloudEnginesPayload = record { - replica_version_id : text; -}; - type DeployGuestosToAllSubnetNodesPayload = record { subnet_id : principal; replica_version_id : text; @@ -445,6 +441,11 @@ type UpdateFirewallRulesPayload = record { rules : vec FirewallRule; }; +type UpdateGuestosVersionForSubnetsPayload = record { + subnet_ids : vec principal; + replica_version_id : text; +}; + type UpdateNodeDirectlyPayload = record { idkg_dealing_encryption_pk : opt blob; }; @@ -624,9 +625,6 @@ service : { complete_canister_migration : (CompleteCanisterMigrationPayload) -> (); create_subnet : (CreateSubnetPayload) -> (CreateSubnetResponse); delete_subnet : (DeleteSubnetPayload) -> (DeleteSubnetResponse); - deploy_guestos_to_all_cloud_engines : ( - DeployGuestosToAllCloudEnginesPayload - ) -> (); deploy_guestos_to_all_subnet_nodes : ( DeployGuestosToAllSubnetNodesPayload ) -> (); @@ -662,6 +660,7 @@ service : { update_elected_hostos_versions : (UpdateElectedHostosVersionsPayload) -> (); revise_elected_hostos_versions : (ReviseElectedHostosVersionsPayload) -> (); update_firewall_rules : (UpdateFirewallRulesPayload) -> (); + update_guestos_version_for_subnets : (UpdateGuestosVersionForSubnetsPayload) -> (); update_node_directly : (UpdateNodeDirectlyPayload) -> (); update_node_domain_directly : (UpdateNodeDomainDirectlyPayload) -> (UpdateNodeDomainDirectlyResponse); update_node_ipv4_config_directly : (UpdateNodeIPv4ConfigDirectlyPayload) -> ( diff --git a/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_cloud_engines.rs b/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_cloud_engines.rs deleted file mode 100644 index 79e83b5f7c59..000000000000 --- a/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_cloud_engines.rs +++ /dev/null @@ -1,236 +0,0 @@ -use crate::{ - common::LOG_PREFIX, - mutations::common::{check_replica_version_is_elected, get_subnet_ids_from_subnet_list}, - registry::Registry, -}; - -use candid::{CandidType, Deserialize}; -#[cfg(target_arch = "wasm32")] -use dfn_core::println; -use ic_registry_keys::make_subnet_record_key; -use ic_registry_subnet_type::SubnetType; -use ic_registry_transport::pb::v1::{RegistryMutation, registry_mutation}; -use prost::Message; -use serde::Serialize; - -impl Registry { - /// Deploys the given (elected) GuestOS version to every CloudEngine subnet. - /// - /// Unlike `do_deploy_guestos_to_all_subnet_nodes`, which targets a single - /// subnet, this enumerates the current `subnet_list` and updates the - /// `replica_version_id` of every subnet whose `subnet_type` is - /// `SubnetType::CloudEngine`. The set of affected subnets is therefore - /// resolved at execution time rather than captured in the payload, so the - /// proposal always acts on the CloudEngine fleet as it exists when it runs. - /// - /// All updates are applied as a single atomic registry mutation: either the - /// whole fleet moves to the new version, or nothing does. - pub fn do_deploy_guestos_to_all_cloud_engines( - &mut self, - payload: DeployGuestosToAllCloudEnginesPayload, - ) { - println!("{LOG_PREFIX}do_deploy_guestos_to_all_cloud_engines: {payload:?}"); - - check_replica_version_is_elected(self, &payload.replica_version_id); - - let cloud_engine_type = i32::from(SubnetType::CloudEngine); - let mut mutations = vec![]; - for subnet_id in get_subnet_ids_from_subnet_list(self.get_subnet_list_record()) { - let mut subnet_record = self.get_subnet_or_panic(subnet_id); - - // Only CloudEngine subnets are affected; everything else is left untouched. - if subnet_record.subnet_type != cloud_engine_type { - continue; - } - - // Skip subnets that are already on the requested version to avoid - // churning the registry with no-op updates. - if subnet_record.replica_version_id == payload.replica_version_id { - continue; - } - - subnet_record.replica_version_id = payload.replica_version_id.clone(); - mutations.push(RegistryMutation { - mutation_type: registry_mutation::Type::Update as i32, - key: make_subnet_record_key(subnet_id).into_bytes(), - value: subnet_record.encode_to_vec(), - }); - } - - println!( - "{LOG_PREFIX}do_deploy_guestos_to_all_cloud_engines: updating {} CloudEngine subnet(s) to version {}", - mutations.len(), - payload.replica_version_id, - ); - - // Check invariants before applying mutations. An empty mutation list is - // a no-op (e.g. when no CloudEngine subnets exist, or all are already on - // the requested version). - self.maybe_apply_mutation_internal(mutations) - } -} - -/// The argument of a command to update the replica version of every CloudEngine -/// subnet to a specific version. -/// -/// The subnets will be mutated only if the given version is, indeed, elected. -#[derive(Clone, Eq, PartialEq, Debug, CandidType, Deserialize, Serialize)] -pub struct DeployGuestosToAllCloudEnginesPayload { - /// The new Replica version to deploy to all CloudEngine subnets. - pub replica_version_id: String, -} - -#[cfg(test)] -mod tests { - use ic_base_types::{NodeId, SubnetId}; - use ic_protobuf::registry::crypto::v1::PublicKey; - use ic_protobuf::registry::node::v1::NodeRewardType; - use ic_protobuf::registry::replica_version::v1::ReplicaVersionRecord; - use ic_protobuf::registry::subnet::v1::CanisterCyclesCostSchedule; - use ic_registry_keys::make_replica_version_key; - use ic_registry_subnet_type::SubnetType; - use ic_registry_transport::insert; - use ic_test_utilities_types::ids::subnet_test_id; - use maplit::btreemap; - use prost::Message; - - use crate::common::test_helpers::{ - add_fake_subnet, get_invariant_compliant_subnet_record, invariant_compliant_registry, - prepare_registry_with_nodes, prepare_registry_with_nodes_and_reward_type, - }; - use crate::registry::Registry; - - use super::DeployGuestosToAllCloudEnginesPayload; - - const TARGET_VERSION: &str = "version"; - - /// Elects `TARGET_VERSION` so the deploy passes the elected-version check. - fn elect_target_version(registry: &mut Registry) { - registry.maybe_apply_mutation_internal(vec![insert( - make_replica_version_key(TARGET_VERSION), - ReplicaVersionRecord { - release_package_sha256_hex: "".into(), - release_package_urls: vec![], - guest_launch_measurements: None, - } - .encode_to_vec(), - )]); - } - - /// Adds a CloudEngine subnet backed by `node_id` and returns its id. The - /// node must have reward type `Type4` and the subnet uses the free cycles - /// cost schedule, both required by registry invariants for CloudEngines. - fn add_cloud_engine_subnet( - registry: &mut Registry, - node_id: NodeId, - dkg_pk: PublicKey, - subnet_index: u64, - ) -> SubnetId { - let mut subnet_record = get_invariant_compliant_subnet_record(vec![node_id]); - subnet_record.subnet_type = i32::from(SubnetType::CloudEngine); - subnet_record.canister_cycles_cost_schedule = i32::from(CanisterCyclesCostSchedule::Free); - add_subnet(registry, node_id, dkg_pk, subnet_index, subnet_record) - } - - /// Adds an application subnet backed by `node_id` and returns its id. - fn add_application_subnet( - registry: &mut Registry, - node_id: NodeId, - dkg_pk: PublicKey, - subnet_index: u64, - ) -> SubnetId { - let mut subnet_record = get_invariant_compliant_subnet_record(vec![node_id]); - subnet_record.subnet_type = i32::from(SubnetType::Application); - add_subnet(registry, node_id, dkg_pk, subnet_index, subnet_record) - } - - fn add_subnet( - registry: &mut Registry, - node_id: NodeId, - dkg_pk: PublicKey, - subnet_index: u64, - subnet_record: ic_protobuf::registry::subnet::v1::SubnetRecord, - ) -> SubnetId { - let mut subnet_list_record = registry.get_subnet_list_record(); - let subnet_id = subnet_test_id(subnet_index); - registry.maybe_apply_mutation_internal(add_fake_subnet( - subnet_id, - &mut subnet_list_record, - subnet_record, - &btreemap!(node_id => dkg_pk), - )); - subnet_id - } - - fn replica_version_of(registry: &Registry, subnet_id: SubnetId) -> String { - registry.get_subnet_or_panic(subnet_id).replica_version_id - } - - #[test] - #[should_panic(expected = "'version' is NOT elected")] - fn should_panic_if_version_not_elected() { - let mut registry = invariant_compliant_registry(0); - - registry.do_deploy_guestos_to_all_cloud_engines(DeployGuestosToAllCloudEnginesPayload { - replica_version_id: TARGET_VERSION.into(), - }); - } - - #[test] - fn should_upgrade_only_cloud_engine_subnets() { - let mut registry = invariant_compliant_registry(0); - elect_target_version(&mut registry); - - // CloudEngine subnets require nodes with reward type Type4; the - // application subnet must use a non-Type4 node. Distinct mutation-id - // ranges keep the generated node IPs/domains from colliding. - let (ce_req, ce_nodes) = - prepare_registry_with_nodes_and_reward_type(1, 2, NodeRewardType::Type4); - registry.maybe_apply_mutation_internal(ce_req.mutations); - let (app_req, app_nodes) = prepare_registry_with_nodes(10, 1); - registry.maybe_apply_mutation_internal(app_req.mutations); - - let mut ce_nodes = ce_nodes.into_iter(); - let (n1, pk1) = ce_nodes.next().unwrap(); - let (n2, pk2) = ce_nodes.next().unwrap(); - let (app_node, app_pk) = app_nodes.into_iter().next().unwrap(); - - let cloud_engine_a = add_cloud_engine_subnet(&mut registry, n1, pk1, 1001); - let cloud_engine_b = add_cloud_engine_subnet(&mut registry, n2, pk2, 1002); - let application = add_application_subnet(&mut registry, app_node, app_pk, 1003); - - registry.do_deploy_guestos_to_all_cloud_engines(DeployGuestosToAllCloudEnginesPayload { - replica_version_id: TARGET_VERSION.into(), - }); - - assert_eq!( - replica_version_of(®istry, cloud_engine_a), - TARGET_VERSION - ); - assert_eq!( - replica_version_of(®istry, cloud_engine_b), - TARGET_VERSION - ); - // The application subnet must be untouched. - assert_ne!(replica_version_of(®istry, application), TARGET_VERSION); - } - - #[test] - fn should_be_noop_when_no_cloud_engine_subnets_exist() { - let mut registry = invariant_compliant_registry(0); - elect_target_version(&mut registry); - - let (app_req, app_nodes) = prepare_registry_with_nodes(1, 1); - registry.maybe_apply_mutation_internal(app_req.mutations); - let (app_node, app_pk) = app_nodes.into_iter().next().unwrap(); - add_application_subnet(&mut registry, app_node, app_pk, 1001); - let version_before = registry.latest_version(); - - registry.do_deploy_guestos_to_all_cloud_engines(DeployGuestosToAllCloudEnginesPayload { - replica_version_id: TARGET_VERSION.into(), - }); - - // No CloudEngine subnets means no mutations, hence no version bump. - assert_eq!(registry.latest_version(), version_before); - } -} diff --git a/rs/registry/canister/src/mutations/do_update_guestos_version_for_subnets.rs b/rs/registry/canister/src/mutations/do_update_guestos_version_for_subnets.rs new file mode 100644 index 000000000000..2c72d650a2d8 --- /dev/null +++ b/rs/registry/canister/src/mutations/do_update_guestos_version_for_subnets.rs @@ -0,0 +1,246 @@ +use crate::{ + common::LOG_PREFIX, + mutations::common::{check_replica_version_is_elected, has_duplicates}, + registry::Registry, +}; + +use candid::{CandidType, Deserialize}; +#[cfg(target_arch = "wasm32")] +use dfn_core::println; +use ic_base_types::{PrincipalId, SubnetId}; +use ic_registry_keys::make_subnet_record_key; +use ic_registry_transport::pb::v1::{RegistryMutation, registry_mutation}; +use prost::Message; +use serde::Serialize; + +impl Registry { + /// Sets the `replica_version_id` of every subnet in `payload.subnet_ids` to + /// the given (elected) GuestOS version. + /// + /// Unlike `do_deploy_guestos_to_all_subnet_nodes`, which targets a single + /// subnet, this updates an explicit list of subnets in one atomic registry + /// mutation: either all listed subnets move to the new version, or none do. + /// Subnets that are already on the requested version are skipped to avoid + /// churning the registry with no-op updates. + pub fn do_update_guestos_version_for_subnets( + &mut self, + payload: UpdateGuestosVersionForSubnetsPayload, + ) { + println!("{LOG_PREFIX}do_update_guestos_version_for_subnets: {payload:?}"); + + check_replica_version_is_elected(self, &payload.replica_version_id); + + assert!( + !payload.subnet_ids.is_empty(), + "{LOG_PREFIX}do_update_guestos_version_for_subnets: subnet_ids must not be empty.", + ); + assert!( + !has_duplicates(&payload.subnet_ids), + "{LOG_PREFIX}do_update_guestos_version_for_subnets: subnet_ids must not contain duplicates.", + ); + + let mut mutations = vec![]; + for subnet_id in &payload.subnet_ids { + let subnet_id = SubnetId::from(*subnet_id); + let mut subnet_record = self + .get_subnet(subnet_id, self.latest_version()) + .unwrap_or_else(|err| { + panic!("{LOG_PREFIX}do_update_guestos_version_for_subnets: {err}") + }); + + if subnet_record.replica_version_id == payload.replica_version_id { + continue; + } + + subnet_record.replica_version_id = payload.replica_version_id.clone(); + mutations.push(RegistryMutation { + mutation_type: registry_mutation::Type::Update as i32, + key: make_subnet_record_key(subnet_id).into_bytes(), + value: subnet_record.encode_to_vec(), + }); + } + + println!( + "{LOG_PREFIX}do_update_guestos_version_for_subnets: updating {} of {} requested subnet(s) to version {}", + mutations.len(), + payload.subnet_ids.len(), + payload.replica_version_id, + ); + + // Check invariants before applying mutations. An empty mutation list is + // a no-op (e.g. when all listed subnets are already on the requested + // version). + self.maybe_apply_mutation_internal(mutations) + } +} + +/// The argument of a command to update the GuestOS (replica) version of a +/// specific list of subnets to a single version. +/// +/// The subnets will be mutated only if the given version is, indeed, elected. +#[derive(Clone, Eq, PartialEq, Debug, CandidType, Deserialize, Serialize)] +pub struct UpdateGuestosVersionForSubnetsPayload { + /// The subnets to update. + pub subnet_ids: Vec, // SubnetId See NNS-73 + /// The new GuestOS (replica) version to use. + pub replica_version_id: String, +} + +#[cfg(test)] +mod tests { + use ic_base_types::{NodeId, PrincipalId, SubnetId}; + use ic_protobuf::registry::crypto::v1::PublicKey; + use ic_protobuf::registry::replica_version::v1::ReplicaVersionRecord; + use ic_registry_keys::make_replica_version_key; + use ic_registry_transport::insert; + use ic_test_utilities_types::ids::subnet_test_id; + use maplit::btreemap; + use prost::Message; + + use crate::common::test_helpers::{ + add_fake_subnet, get_invariant_compliant_subnet_record, invariant_compliant_registry, + prepare_registry_with_nodes, + }; + use crate::registry::Registry; + + use super::UpdateGuestosVersionForSubnetsPayload; + + const TARGET_VERSION: &str = "version"; + + /// Elects `TARGET_VERSION` so the update passes the elected-version check. + fn elect_target_version(registry: &mut Registry) { + registry.maybe_apply_mutation_internal(vec![insert( + make_replica_version_key(TARGET_VERSION), + ReplicaVersionRecord { + release_package_sha256_hex: "".into(), + release_package_urls: vec![], + guest_launch_measurements: None, + } + .encode_to_vec(), + )]); + } + + /// Adds an application subnet backed by `node_id` and returns its id. + fn add_subnet( + registry: &mut Registry, + node_id: NodeId, + dkg_pk: PublicKey, + subnet_index: u64, + ) -> SubnetId { + let subnet_record = get_invariant_compliant_subnet_record(vec![node_id]); + let mut subnet_list_record = registry.get_subnet_list_record(); + let subnet_id = subnet_test_id(subnet_index); + registry.maybe_apply_mutation_internal(add_fake_subnet( + subnet_id, + &mut subnet_list_record, + subnet_record, + &btreemap!(node_id => dkg_pk), + )); + subnet_id + } + + fn replica_version_of(registry: &Registry, subnet_id: SubnetId) -> String { + registry.get_subnet_or_panic(subnet_id).replica_version_id + } + + /// Sets up a registry with three application subnets and elects the target + /// version. Returns the three subnet ids. + fn registry_with_three_subnets() -> (Registry, [SubnetId; 3]) { + let mut registry = invariant_compliant_registry(0); + elect_target_version(&mut registry); + + let (mutate_request, nodes) = prepare_registry_with_nodes(1, 3); + registry.maybe_apply_mutation_internal(mutate_request.mutations); + + let mut nodes = nodes.into_iter(); + let (n1, pk1) = nodes.next().unwrap(); + let (n2, pk2) = nodes.next().unwrap(); + let (n3, pk3) = nodes.next().unwrap(); + + let s1 = add_subnet(&mut registry, n1, pk1, 1001); + let s2 = add_subnet(&mut registry, n2, pk2, 1002); + let s3 = add_subnet(&mut registry, n3, pk3, 1003); + (registry, [s1, s2, s3]) + } + + #[test] + #[should_panic(expected = "'version' is NOT elected")] + fn should_panic_if_version_not_elected() { + let mut registry = invariant_compliant_registry(0); + + registry.do_update_guestos_version_for_subnets(UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![subnet_test_id(1001).get()], + replica_version_id: TARGET_VERSION.into(), + }); + } + + #[test] + #[should_panic(expected = "subnet_ids must not be empty")] + fn should_panic_on_empty_subnet_list() { + let mut registry = invariant_compliant_registry(0); + elect_target_version(&mut registry); + + registry.do_update_guestos_version_for_subnets(UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![], + replica_version_id: TARGET_VERSION.into(), + }); + } + + #[test] + #[should_panic(expected = "must not contain duplicates")] + fn should_panic_on_duplicate_subnets() { + let (mut registry, [s1, ..]) = registry_with_three_subnets(); + + registry.do_update_guestos_version_for_subnets(UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![s1.get(), s1.get()], + replica_version_id: TARGET_VERSION.into(), + }); + } + + #[test] + #[should_panic(expected = "not found in the registry")] + fn should_panic_on_unknown_subnet() { + let mut registry = invariant_compliant_registry(0); + elect_target_version(&mut registry); + + registry.do_update_guestos_version_for_subnets(UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![PrincipalId::new_subnet_test_id(12345)], + replica_version_id: TARGET_VERSION.into(), + }); + } + + #[test] + fn should_update_only_listed_subnets() { + let (mut registry, [s1, s2, s3]) = registry_with_three_subnets(); + + registry.do_update_guestos_version_for_subnets(UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![s1.get(), s2.get()], + replica_version_id: TARGET_VERSION.into(), + }); + + assert_eq!(replica_version_of(®istry, s1), TARGET_VERSION); + assert_eq!(replica_version_of(®istry, s2), TARGET_VERSION); + // The subnet that was not listed must be untouched. + assert_ne!(replica_version_of(®istry, s3), TARGET_VERSION); + } + + #[test] + fn should_be_noop_when_listed_subnets_already_on_version() { + let (mut registry, [s1, ..]) = registry_with_three_subnets(); + + // Bring s1 to the target version first. + registry.do_update_guestos_version_for_subnets(UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![s1.get()], + replica_version_id: TARGET_VERSION.into(), + }); + let version_before = registry.latest_version(); + + // Re-applying for the same subnet produces no mutation, hence no bump. + registry.do_update_guestos_version_for_subnets(UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![s1.get()], + replica_version_id: TARGET_VERSION.into(), + }); + + assert_eq!(registry.latest_version(), version_before); + } +} diff --git a/rs/registry/canister/src/mutations/mod.rs b/rs/registry/canister/src/mutations/mod.rs index f0ce12147e25..47b683c58296 100644 --- a/rs/registry/canister/src/mutations/mod.rs +++ b/rs/registry/canister/src/mutations/mod.rs @@ -10,7 +10,6 @@ pub mod do_change_subnet_membership; pub mod do_clear_provisional_whitelist; pub mod do_create_subnet; pub mod do_delete_subnet; -pub mod do_deploy_guestos_to_all_cloud_engines; pub mod do_deploy_guestos_to_all_subnet_nodes; pub mod do_deploy_guestos_to_all_unassigned_nodes; pub mod do_migrate_canisters; @@ -28,6 +27,7 @@ pub mod do_split_subnet; pub mod do_swap_node_in_subnet_directly; pub mod do_update_api_boundary_nodes_version; pub mod do_update_elected_hostos_versions; +pub mod do_update_guestos_version_for_subnets; pub mod do_update_node_directly; pub mod do_update_node_operator_config; pub mod do_update_node_operator_config_directly; diff --git a/rs/registry/canister/unreleased_changelog.md b/rs/registry/canister/unreleased_changelog.md index 6523f430505a..a6a7b0c35d61 100644 --- a/rs/registry/canister/unreleased_changelog.md +++ b/rs/registry/canister/unreleased_changelog.md @@ -21,11 +21,11 @@ on the process that this file is part of, see * Added `vcpu_type` to `GuestLaunchMeasurementMetadata` to record the virtual CPU type used for a guest launch measurement. * Added a new endpoint `get_subnet` to the registry canister, returning the subnet record of a given subnet. -* Added a new endpoint `deploy_guestos_to_all_cloud_engines` to the registry - canister, which sets the `replica_version_id` of every CloudEngine subnet to a - given (elected) GuestOS version in a single atomic mutation. The set of - affected subnets is resolved from the registry at execution time (all subnets - whose `subnet_type` is `CloudEngine`); other subnet types are left untouched. +* Added a new endpoint `update_guestos_version_for_subnets` to the registry + canister, which sets the `replica_version_id` of an explicit list of subnets + to a given (elected) GuestOS version in a single atomic mutation. Subnets not + in the list are left untouched, and subnets already on the requested version + are skipped. ## Changed From bc690e02d4b412593f415de94359b3e278650a42 Mon Sep 17 00:00:00 2001 From: Pietro Date: Tue, 9 Jun 2026 16:55:01 +0000 Subject: [PATCH 3/3] test(registry): PocketIC integration test for update_guestos_version_for_subnets Adds an integration test (modeled on delete_subnet.rs) covering the governance-only authorization and the end-to-end registry mutation through the canister candid boundary: - anonymous and unauthorized-principal ingress calls are rejected; - a non-governance canister call (via the universal canister) is rejected; - a governance call updates the listed subnet's replica_version_id; - an unelected version is rejected and leaves the subnet record unchanged. --- .../update_guestos_version_for_subnets.rs | 239 ++++++++++++++++++ 1 file changed, 239 insertions(+) create mode 100644 rs/registry/canister/tests/update_guestos_version_for_subnets.rs diff --git a/rs/registry/canister/tests/update_guestos_version_for_subnets.rs b/rs/registry/canister/tests/update_guestos_version_for_subnets.rs new file mode 100644 index 000000000000..13f8795e1701 --- /dev/null +++ b/rs/registry/canister/tests/update_guestos_version_for_subnets.rs @@ -0,0 +1,239 @@ +use candid::{Encode, Principal}; +use ic_base_types::{PrincipalId, SubnetId}; +use ic_nervous_system_integration_tests::pocket_ic_helpers::nns::registry::decode_registry_value; +use ic_nns_constants::{GOVERNANCE_CANISTER_ID, REGISTRY_CANISTER_ID}; +use ic_nns_test_utils::itest_helpers::{ + forward_call_via_universal_canister, set_up_registry_canister, set_up_universal_canister, + state_machine_test_on_nns_subnet, +}; +use ic_nns_test_utils::registry::invariant_compliant_mutation_as_atomic_req; +use ic_protobuf::registry::subnet::v1::{SubnetListRecord as SubnetListRecordPb, SubnetRecord}; +use ic_registry_keys::{make_subnet_list_record_key, make_subnet_record_key}; +use pocket_ic::PocketIcBuilder; +use pocket_ic::nonblocking::PocketIc; +use registry_canister::{ + init::RegistryCanisterInitPayloadBuilder, + mutations::{ + do_revise_elected_replica_versions::ReviseElectedGuestosVersionsPayload, + do_update_guestos_version_for_subnets::UpdateGuestosVersionForSubnetsPayload, + }, +}; + +mod common; + +use common::test_helpers::install_registry_canister_with_payload_builder; + +const MOCK_HASH: &str = "acdcacdcacdcacdcacdcacdcacdcacdcacdcacdcacdcacdcacdcacdcacdcacdc"; +const NEW_VERSION: &str = "version_43"; + +/// Installs an invariant-compliant registry (which already contains a single, +/// non-CloudEngine subnet) and returns the running `PocketIc` together with the +/// principal of an existing subnet that can be used as an update target. +async fn setup_with_existing_subnet() -> (PocketIc, Principal) { + let pocket_ic = PocketIcBuilder::new().with_nns_subnet().build_async().await; + + let mut builder = RegistryCanisterInitPayloadBuilder::new(); + builder.push_init_mutate_request(invariant_compliant_mutation_as_atomic_req(0)); + install_registry_canister_with_payload_builder(&pocket_ic, builder.build(), true).await; + + let subnet_list_record = + decode_registry_value::(&pocket_ic, make_subnet_list_record_key()) + .await; + let subnet_id = subnet_list_record + .subnets + .first() + .expect("expected the invariant-compliant registry to contain at least one subnet"); + let subnet_id = Principal::try_from(subnet_id.as_slice()).unwrap(); + + (pocket_ic, subnet_id) +} + +/// Elects `NEW_VERSION` by submitting a `revise_elected_replica_versions` call +/// as the governance canister. +async fn elect_new_version(pocket_ic: &PocketIc) { + let payload = ReviseElectedGuestosVersionsPayload { + replica_version_to_elect: Some(NEW_VERSION.into()), + release_package_sha256_hex: Some(MOCK_HASH.into()), + release_package_urls: vec!["http://release_package.tar.zst".into()], + guest_launch_measurements: None, + replica_versions_to_unelect: vec![], + }; + pocket_ic + .update_call( + REGISTRY_CANISTER_ID.get().0, + GOVERNANCE_CANISTER_ID.get().0, + "revise_elected_replica_versions", + Encode!(&payload).unwrap(), + ) + .await + .expect("failed to elect a new replica version"); +} + +fn subnet_replica_version(record: &SubnetRecord) -> &str { + &record.replica_version_id +} + +#[tokio::test] +async fn test_the_anonymous_user_cannot_update_guestos_version_for_subnets() { + let (pocket_ic, subnet_id) = setup_with_existing_subnet().await; + + let payload = UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![PrincipalId::from(subnet_id)], + replica_version_id: NEW_VERSION.into(), + }; + + // The anonymous end-user tries to update subnets via an ingress message, + // bypassing governance. This should be rejected by the authorization check. + let response = pocket_ic + .update_call( + REGISTRY_CANISTER_ID.get().0, + PrincipalId::new_anonymous().0, + "update_guestos_version_for_subnets", + Encode!(&payload).unwrap(), + ) + .await; + + assert!( + response.as_ref().is_err_and(|err| err + .reject_message + .contains("is not authorized to call this method: update_guestos_version_for_subnets")), + "Expected an authorization rejection, but got {response:?}" + ); +} + +#[tokio::test] +async fn test_an_unauthorized_principal_cannot_update_guestos_version_for_subnets() { + let (pocket_ic, subnet_id) = setup_with_existing_subnet().await; + + // A principal that is not the governance canister, calling via an ingress + // message. + let unauthorized_caller = PrincipalId::new_user_test_id(1); + assert_ne!(unauthorized_caller, GOVERNANCE_CANISTER_ID.get()); + + let payload = UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![PrincipalId::from(subnet_id)], + replica_version_id: NEW_VERSION.into(), + }; + + let response = pocket_ic + .update_call( + REGISTRY_CANISTER_ID.get().0, + unauthorized_caller.0, + "update_guestos_version_for_subnets", + Encode!(&payload).unwrap(), + ) + .await; + + assert!( + response.as_ref().is_err_and(|err| err + .reject_message + .contains("is not authorized to call this method: update_guestos_version_for_subnets")), + "Expected an authorization rejection, but got {response:?}" + ); +} + +#[test] +fn test_a_canister_other_than_governance_cannot_update_guestos_version_for_subnets() { + state_machine_test_on_nns_subnet(|runtime| async move { + // An attacker canister tries to update subnets via an inter-canister + // call. Going through a real canister (rather than an ingress message) + // ensures the access control cannot be bypassed by, e.g., only guarding + // ingress messages in `inspect_message`. + let attacker_canister = set_up_universal_canister(&runtime).await; + assert_ne!( + attacker_canister.canister_id(), + ic_nns_constants::GOVERNANCE_CANISTER_ID + ); + + let registry = set_up_registry_canister( + &runtime, + RegistryCanisterInitPayloadBuilder::new() + .push_init_mutate_request(invariant_compliant_mutation_as_atomic_req(0)) + .build(), + ) + .await; + + let payload = UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![PrincipalId::new_subnet_test_id(999)], + replica_version_id: NEW_VERSION.into(), + }; + + // The attacker canister's call should have no effect. + assert!( + !forward_call_via_universal_canister( + &attacker_canister, + ®istry, + "update_guestos_version_for_subnets", + Encode!(&payload).unwrap() + ) + .await + ); + + Ok(()) + }); +} + +#[tokio::test] +async fn test_governance_can_update_guestos_version_for_subnets() { + let (pocket_ic, subnet_id) = setup_with_existing_subnet().await; + let subnet_key = make_subnet_record_key(SubnetId::from(PrincipalId::from(subnet_id))); + + // The subnet must not already be on the new version, otherwise the update + // would be a no-op and this test would not prove anything. + let before = decode_registry_value::(&pocket_ic, &subnet_key).await; + assert_ne!(subnet_replica_version(&before), NEW_VERSION); + + elect_new_version(&pocket_ic).await; + + let payload = UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![PrincipalId::from(subnet_id)], + replica_version_id: NEW_VERSION.into(), + }; + pocket_ic + .update_call( + REGISTRY_CANISTER_ID.get().0, + GOVERNANCE_CANISTER_ID.get().0, + "update_guestos_version_for_subnets", + Encode!(&payload).unwrap(), + ) + .await + .expect("governance update_guestos_version_for_subnets call was unexpectedly rejected"); + + let after = decode_registry_value::(&pocket_ic, &subnet_key).await; + assert_eq!(subnet_replica_version(&after), NEW_VERSION); +} + +#[tokio::test] +async fn test_unelected_version_is_rejected() { + let (pocket_ic, subnet_id) = setup_with_existing_subnet().await; + let subnet_key = make_subnet_record_key(SubnetId::from(PrincipalId::from(subnet_id))); + let before = decode_registry_value::(&pocket_ic, &subnet_key).await; + + let payload = UpdateGuestosVersionForSubnetsPayload { + subnet_ids: vec![PrincipalId::from(subnet_id)], + replica_version_id: "unelected".into(), + }; + + let response = pocket_ic + .update_call( + REGISTRY_CANISTER_ID.get().0, + GOVERNANCE_CANISTER_ID.get().0, + "update_guestos_version_for_subnets", + Encode!(&payload).unwrap(), + ) + .await; + + assert!( + response + .as_ref() + .is_err_and(|err| err.reject_message.contains("is NOT elected")), + "Expected a rejection because the version is not elected, but got {response:?}" + ); + + // The subnet record must be unchanged. + let after = decode_registry_value::(&pocket_ic, &subnet_key).await; + assert_eq!( + subnet_replica_version(&after), + subnet_replica_version(&before) + ); +}