Skip to content

Commit eb8641f

Browse files
MinJuTurclaude
andauthored
fix(BA-3806): use intersection operator for scaling group conflict detection (#7892)
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 36e9b63 commit eb8641f

File tree

3 files changed

+57
-2
lines changed

3 files changed

+57
-2
lines changed

changes/3806.fix.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix incorrect set operator for scaling group conflict validation in `modify_domain_node`

src/ai/backend/manager/services/domain/service.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,10 @@ async def modify_domain_node(
118118
self, action: ModifyDomainNodeAction
119119
) -> ModifyDomainNodeActionResult:
120120
if action.sgroups_to_add is not None and action.sgroups_to_remove is not None:
121-
if union := action.sgroups_to_add | action.sgroups_to_remove:
121+
if conflict := action.sgroups_to_add & action.sgroups_to_remove:
122122
raise InvalidAPIParameters(
123123
"Should be no scaling group names included in both `sgroups_to_add` and `sgroups_to_remove` "
124-
f"(sg:{union})."
124+
f"(sg:{conflict})."
125125
)
126126

127127
if action.user_info.role == UserRole.SUPERADMIN:

tests/unit/manager/services/test_domain.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,3 +1001,57 @@ async def test_modify_domain_node_with_scaling_groups_to_remove(
10011001

10021002
assert result.domain_data is not None
10031003
mock_admin_repository.modify_domain_node_with_permissions_force.assert_called_once()
1004+
1005+
async def test_modify_domain_node_with_disjoint_scaling_groups_succeeds(
1006+
self,
1007+
service: DomainService,
1008+
mock_admin_repository: MagicMock,
1009+
superadmin_user: UserInfo,
1010+
sample_domain_data: DomainData,
1011+
) -> None:
1012+
"""Modify domain node with disjoint add/remove scaling groups should succeed."""
1013+
mock_admin_repository.modify_domain_node_with_permissions_force = AsyncMock(
1014+
return_value=sample_domain_data
1015+
)
1016+
1017+
action = ModifyDomainNodeAction(
1018+
user_info=superadmin_user,
1019+
updater=Updater(
1020+
spec=DomainNodeUpdaterSpec(),
1021+
pk_value="test-domain",
1022+
),
1023+
sgroups_to_add={"sg1", "sg2"},
1024+
sgroups_to_remove={"sg3", "sg4"}, # No overlap
1025+
)
1026+
1027+
result = await service.modify_domain_node(action)
1028+
1029+
assert result.domain_data is not None
1030+
mock_admin_repository.modify_domain_node_with_permissions_force.assert_called_once()
1031+
1032+
async def test_modify_domain_node_with_empty_scaling_groups_succeeds(
1033+
self,
1034+
service: DomainService,
1035+
mock_admin_repository: MagicMock,
1036+
superadmin_user: UserInfo,
1037+
sample_domain_data: DomainData,
1038+
) -> None:
1039+
"""Modify domain node with both empty scaling groups should succeed."""
1040+
mock_admin_repository.modify_domain_node_with_permissions_force = AsyncMock(
1041+
return_value=sample_domain_data
1042+
)
1043+
1044+
action = ModifyDomainNodeAction(
1045+
user_info=superadmin_user,
1046+
updater=Updater(
1047+
spec=DomainNodeUpdaterSpec(),
1048+
pk_value="test-domain",
1049+
),
1050+
sgroups_to_add=set(),
1051+
sgroups_to_remove=set(),
1052+
)
1053+
1054+
result = await service.modify_domain_node(action)
1055+
1056+
assert result.domain_data is not None
1057+
mock_admin_repository.modify_domain_node_with_permissions_force.assert_called_once()

0 commit comments

Comments
 (0)