From 3fe447931b76b6a0d64092e3917851e55b00045c Mon Sep 17 00:00:00 2001 From: Kiran Pawar Date: Mon, 14 Sep 2026 13:32:53 +0000 Subject: [PATCH] scheduler: clear snapshot_id from replica request_spec A share created from a snapshot carries snapshot_id, which the capacity filter and weigher use to apply reserved_snapshot_percentage instead of reserved_percentage, and which the host-affinity weigher uses to steer scheduling toward the snapshot's source host. Neither behaviour is appropriate for replica scheduling: replicas are intentionally placed on a different backend, so reserved_snapshot_percentage (meant to keep snapshot-derived shares near their parent) is wrong, and affinity toward the original snapshot host is actively harmful. Change-Id: I9ba703dc38c6d9630174dad7ab4faf6270360d3a --- manila/share/api.py | 13 +++++++++---- manila/tests/share/test_api.py | 33 +++++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/manila/share/api.py b/manila/share/api.py index 7ed9bfefbd..d5b76255ea 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -786,7 +786,7 @@ def create_share_instance_and_get_request_spec( az_request_multiple_subnet_support_map=None, mount_point_name=None, share_instance_id=None, encryption_key_ref=None, qos_type_id=None, - share_instance_metadata=None): + share_instance_metadata=None, is_replica=False): availability_zone_id = None if availability_zone: @@ -832,7 +832,11 @@ def create_share_instance_and_get_request_spec( 'share_group_id': share['share_group_id'], 'source_share_group_snapshot_member_id': share[ 'source_share_group_snapshot_member_id'], - 'snapshot_id': share['snapshot_id'], + # Replicas are placed on a different backend than the source + # share, so the snapshot origin is irrelevant for scheduling: + # capacity and affinity filters must not use snapshot_percentage + # or steer toward the snapshot host. + 'snapshot_id': None if is_replica else share['snapshot_id'], 'replication_type': share['replication_type'], } share_instance_properties = { @@ -859,7 +863,7 @@ def create_share_instance_and_get_request_spec( 'share_instance_properties': share_instance_properties, 'share_proto': share['share_proto'], 'share_id': share['id'], - 'snapshot_id': share['snapshot_id'], + 'snapshot_id': None if is_replica else share['snapshot_id'], 'snapshot_host': snapshot_host, 'share_type': share_type, 'share_group': share_group, @@ -1016,7 +1020,8 @@ def create_share_replica(self, context, share, availability_zone=None, az_request_multiple_subnet_support_map), qos_type_id=qos_type_id, share_instance_metadata=metadata, - mount_point_name=active_replica.get('mount_point_name')) + mount_point_name=active_replica.get('mount_point_name'), + is_replica=True) ) QUOTAS.commit( context, reservations, project_id=share['project_id'], diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index b572030425..a2a9e2ee7b 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -1353,6 +1353,27 @@ def test_create_share_instance_from_snapshot(self): self.api.share_rpcapi.create_share_instance_and_get_request_spec .called) + def test_create_share_instance_replica_clears_snapshot_id(self): + """Replica request_spec must have snapshot_id=None. + + A share created from a snapshot carries snapshot_id. When a replica + of that share is scheduled, the snapshot origin must not influence + capacity or affinity decisions because the replica goes to a different + backend where reserved_snapshot_percentage and host-affinity toward + the snapshot host are irrelevant. + """ + snapshot, share, _, _ = self._setup_create_from_snapshot_mocks() + self.assertIsNotNone(share['snapshot_id']) + + request_spec, share_instance = ( + self.api.create_share_instance_and_get_request_spec( + self.context, share, is_replica=True) + ) + + self.assertIsNone(request_spec['snapshot_id']) + self.assertIsNone( + request_spec['share_properties']['snapshot_id']) + def test_create_instance_share_group_snapshot_member(self): fake_req_spec = { 'share_properties': 'fake_share_properties', @@ -4694,7 +4715,8 @@ def test_create_share_replica_azs_with_subnets(self, availability_zones, cast_rules_to_readonly=cast_rules_to_readonly, qos_type_id=None, share_instance_metadata=None, - mount_point_name=None)) + mount_point_name=None, + is_replica=True)) db_api.share_replica_update.assert_called_once() mock_snapshot_get_all_call.assert_called_once() mock_sched_rpcapi_call.assert_called_once() @@ -4838,7 +4860,8 @@ def test_create_share_replica(self, has_snapshots, extra_specs, cast_rules_to_readonly=cast_rules_to_readonly, qos_type_id=None, share_instance_metadata=None, - mount_point_name=None)) + mount_point_name=None, + is_replica=True)) def test_create_share_replica_mount_point_name_inheritance(self): extra_specs = {'replication_type': constants.REPLICATION_TYPE_DR} @@ -4883,7 +4906,8 @@ def test_create_share_replica_mount_point_name_inheritance(self): cast_rules_to_readonly=False, qos_type_id=None, share_instance_metadata=None, - mount_point_name='fake_mpn') + mount_point_name='fake_mpn', + is_replica=True) def test_delete_last_active_replica(self): fake_replica = fakes.fake_replica( @@ -5125,7 +5149,8 @@ def test_create_share_replica_error_on_quota_commit(self): cast_rules_to_readonly=False, qos_type_id=None, share_instance_metadata=None, - mount_point_name=None)) + mount_point_name=None, + is_replica=True)) def test_migration_complete(self):