Skip to content

Commit 8f04a0d

Browse files
committed
[AAP-82119] Batch resource migration using bulk-update endpoint
Refactor migrate_resource() to process entire API pages as batches instead of one item at a time. The new _process_resource_page_batch() method: 1. Validates all items on the page (reuses existing helpers) 2. Reconciles against existing gateway resources 3. Bulk-creates gateway resources in a single transaction 4. Sends a single bulk_update_resources() HTTP call per page This reduces HTTP round-trips from N (one per resource) to N/page_size (one per page). For Kyndryl-scale data (3,962 users), this drops from 3,962 sequential PATCH calls to ~80 batch calls — projected to cut user migration from 203s to under 10s. Depends on: django-ansible-base AAP-82119/bulk-update-endpoint branch Assisted-by: Claude AI assistant
1 parent b5c4d96 commit 8f04a0d

3 files changed

Lines changed: 251 additions & 4 deletions

File tree

aap_gateway_api/management/commands/_migrate_service_data/resource_migration.py

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,72 @@ def _process_and_migrate_resource_item(self, upstream_resource_item: Dict[str, A
253253

254254
self.client.update_resource(resource_ansible_id, ResourceRequestBody(**updated_service_resource), partial=True)
255255

256+
def _process_resource_page_batch(self, results: List[Dict[str, Any]], resource_context: Dict[str, Any]) -> int:
257+
"""
258+
Process and migrate a batch of resource items from a single API page.
259+
260+
This replaces the per-item loop with a batch approach that:
261+
1. Validates and prepares all items on the page
262+
2. Creates gateway resources within a single transaction
263+
3. Sends a single bulk HTTP call to update all upstream resources
264+
265+
Args:
266+
results: List of resource items from the upstream service API page
267+
resource_context: Static data about the resource type
268+
269+
Returns:
270+
Number of items successfully processed in this batch
271+
"""
272+
resource_type = resource_context["type"]
273+
bulk_update_items = []
274+
create_operations = []
275+
276+
for upstream_resource_item in results:
277+
resource_ansible_id = upstream_resource_item["ansible_id"]
278+
279+
if "resource_data" not in upstream_resource_item:
280+
raise RuntimeError(
281+
f"Resource {resource_ansible_id} is missing 'resource_data'. Ensure all services are running a version of DAB that supports extra_fields."
282+
)
283+
284+
upstream_resource = upstream_resource_item
285+
286+
validated_resource_data = self._deserialize_and_validate_resource_data(upstream_resource, resource_context["type_serializer"])
287+
288+
if resource_context["type_name"] == SHARED_USER_RESOURCE_TYPE:
289+
upstream_resource = self._sync_user_superuser_flag(upstream_resource, validated_resource_data)
290+
validated_resource_data = self._deserialize_and_validate_resource_data(upstream_resource, resource_context["type_serializer"])
291+
292+
resource_creation_kwargs, updated_service_resource = self._initialize_resource_sync_payloads(upstream_resource)
293+
create_gateway_resource = self._reconcile_existing_resource(upstream_resource, resource_context, validated_resource_data, updated_service_resource)
294+
295+
if create_gateway_resource:
296+
create_operations.append((resource_type, upstream_resource["resource_data"], resource_creation_kwargs))
297+
298+
# Build the bulk-update payload item for the upstream service
299+
bulk_item: Dict[str, Any] = {"ansible_id": resource_ansible_id}
300+
if "service_id" in updated_service_resource:
301+
bulk_item["service_id"] = updated_service_resource["service_id"]
302+
if "is_partially_migrated" in updated_service_resource:
303+
bulk_item["is_partially_migrated"] = updated_service_resource["is_partially_migrated"]
304+
if "ansible_id" in updated_service_resource:
305+
bulk_item["new_ansible_id"] = str(updated_service_resource["ansible_id"])
306+
if "resource_data" in updated_service_resource:
307+
bulk_item["resource_data"] = updated_service_resource["resource_data"]
308+
309+
bulk_update_items.append(bulk_item)
310+
311+
with transaction.atomic():
312+
for rt, resource_data, creation_kwargs in create_operations:
313+
Resource.create_resource(rt, resource_data, **creation_kwargs)
314+
315+
if bulk_update_items:
316+
resp = self.client.bulk_update_resources(bulk_update_items)
317+
if resp.status_code != 200:
318+
raise RuntimeError(f"Bulk resource update failed with status {resp.status_code}: {resp.text}")
319+
320+
return len(results)
321+
256322
def migrate_resource(self, resource_type_name: str) -> None:
257323
"""
258324
Migrate all resources of a specific type from upstream service to Gateway.
@@ -328,7 +394,6 @@ def migrate_resource(self, resource_type_name: str) -> None:
328394
self._log("No more items remaining to migrate.", logging.INFO)
329395
break
330396

331-
for upstream_resource_item in results:
332-
resource_processed += 1
333-
self._log_progress(progress_label, resource_processed, resource_total)
334-
self._process_and_migrate_resource_item(upstream_resource_item, resource_context)
397+
batch_size = self._process_resource_page_batch(results, resource_context)
398+
resource_processed += batch_size
399+
self._log_progress(progress_label, resource_processed, resource_total)

aap_gateway_api/tests/management/commands/_migrate_service_data/test_resource_migration.py

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,179 @@ def test_process_migrate_resource_item_raises_on_missing_resource_data():
175175
cmd._process_and_migrate_resource_item(resource_item, resource_context)
176176

177177

178+
@pytest.mark.django_db
179+
def test_process_resource_page_batch_raises_on_missing_resource_data():
180+
"""Test that _process_resource_page_batch raises when resource_data is missing."""
181+
cmd = MigrateCommand()
182+
results = [{"ansible_id": "test-id-456", "name": "test"}]
183+
resource_context = {"type": Mock()}
184+
185+
with pytest.raises(RuntimeError, match="missing 'resource_data'"):
186+
cmd._process_resource_page_batch(results, resource_context)
187+
188+
189+
@pytest.mark.django_db
190+
def test_process_resource_page_batch_bulk_update():
191+
"""Test that _process_resource_page_batch calls bulk_update_resources with correct payloads."""
192+
import uuid
193+
194+
from ansible_base.resource_registry.models import ResourceType
195+
196+
cmd = MigrateCommand()
197+
cmd.stdout = StringIO()
198+
cmd.stderr = StringIO()
199+
cmd.upstream_service_id = str(uuid.uuid4())
200+
201+
mock_client = Mock()
202+
mock_client.service.service_cluster.service_type.name = "awx"
203+
mock_resp = Mock()
204+
mock_resp.status_code = 200
205+
mock_resp.json.return_value = {"updated": 2, "errors": []}
206+
mock_client.bulk_update_resources.return_value = mock_resp
207+
cmd.client = mock_client
208+
209+
Organization.objects.create(name="BatchOrg1")
210+
211+
org_resource_type = ResourceType.objects.get(name="shared.organization")
212+
resource_context = {
213+
"type": org_resource_type,
214+
"type_name": "shared.organization",
215+
"type_serializer": org_resource_type.serializer_class,
216+
"type_name_field": org_resource_type.get_resource_config().name_field,
217+
"unique_fields": ["name"],
218+
"LocalResourceModel": Organization,
219+
}
220+
221+
results = [
222+
{
223+
"ansible_id": str(uuid.uuid4()),
224+
"name": "BatchOrg1",
225+
"resource_type": "shared.organization",
226+
"resource_data": {"name": "BatchOrg1"},
227+
},
228+
{
229+
"ansible_id": str(uuid.uuid4()),
230+
"name": "NewOrg",
231+
"resource_type": "shared.organization",
232+
"resource_data": {"name": "NewOrg"},
233+
},
234+
]
235+
236+
count = cmd._process_resource_page_batch(results, resource_context)
237+
assert count == 2
238+
239+
mock_client.bulk_update_resources.assert_called_once()
240+
bulk_items = mock_client.bulk_update_resources.call_args[0][0]
241+
assert len(bulk_items) == 2
242+
assert all("ansible_id" in item for item in bulk_items)
243+
# The first item (BatchOrg1 exists) triggers reconcile which sets ansible_id and resource_data
244+
merged_item = bulk_items[0]
245+
assert "new_ansible_id" in merged_item
246+
assert "resource_data" in merged_item
247+
# The second item (NewOrg is new) only gets service_id
248+
new_item = bulk_items[1]
249+
assert "service_id" in new_item
250+
251+
252+
@pytest.mark.django_db
253+
def test_process_resource_page_batch_with_partially_migrated():
254+
"""Test that is_partially_migrated is included in bulk payload when set."""
255+
import uuid
256+
from unittest.mock import patch as mock_patch
257+
258+
from ansible_base.resource_registry.models import ResourceType
259+
260+
cmd = MigrateCommand()
261+
cmd.stdout = StringIO()
262+
cmd.stderr = StringIO()
263+
cmd.upstream_service_id = str(uuid.uuid4())
264+
265+
mock_client = Mock()
266+
mock_client.service.service_cluster.service_type.name = "awx"
267+
mock_resp = Mock()
268+
mock_resp.status_code = 200
269+
mock_client.bulk_update_resources.return_value = mock_resp
270+
cmd.client = mock_client
271+
272+
org_resource_type = ResourceType.objects.get(name="shared.organization")
273+
resource_context = {
274+
"type": org_resource_type,
275+
"type_name": "shared.organization",
276+
"type_serializer": org_resource_type.serializer_class,
277+
"type_name_field": org_resource_type.get_resource_config().name_field,
278+
"unique_fields": ["name"],
279+
"LocalResourceModel": Organization,
280+
}
281+
282+
results = [
283+
{
284+
"ansible_id": str(uuid.uuid4()),
285+
"name": "PartialOrg",
286+
"resource_type": "shared.organization",
287+
"resource_data": {"name": "PartialOrg"},
288+
},
289+
]
290+
291+
# Mock _reconcile_existing_resource to inject is_partially_migrated
292+
def mock_reconcile(upstream_resource, ctx, validated_data, updated_service_resource):
293+
updated_service_resource["is_partially_migrated"] = True
294+
return True
295+
296+
with mock_patch.object(cmd, "_reconcile_existing_resource", side_effect=mock_reconcile):
297+
count = cmd._process_resource_page_batch(results, resource_context)
298+
299+
assert count == 1
300+
bulk_items = mock_client.bulk_update_resources.call_args[0][0]
301+
assert bulk_items[0]["is_partially_migrated"] is True
302+
303+
304+
@pytest.mark.django_db
305+
def test_process_resource_page_batch_rollback_on_bulk_failure():
306+
"""Test that local DB changes are rolled back if bulk_update_resources fails."""
307+
import uuid
308+
309+
from ansible_base.resource_registry.models import ResourceType
310+
311+
cmd = MigrateCommand()
312+
cmd.stdout = StringIO()
313+
cmd.stderr = StringIO()
314+
cmd.upstream_service_id = str(uuid.uuid4())
315+
316+
mock_client = Mock()
317+
mock_client.service.service_cluster.service_type.name = "awx"
318+
mock_resp = Mock()
319+
mock_resp.status_code = 500
320+
mock_resp.text = "Internal Server Error"
321+
mock_client.bulk_update_resources.return_value = mock_resp
322+
cmd.client = mock_client
323+
324+
org_resource_type = ResourceType.objects.get(name="shared.organization")
325+
resource_context = {
326+
"type": org_resource_type,
327+
"type_name": "shared.organization",
328+
"type_serializer": org_resource_type.serializer_class,
329+
"type_name_field": org_resource_type.get_resource_config().name_field,
330+
"unique_fields": ["name"],
331+
"LocalResourceModel": Organization,
332+
}
333+
334+
results = [
335+
{
336+
"ansible_id": str(uuid.uuid4()),
337+
"name": "RollbackOrg",
338+
"resource_type": "shared.organization",
339+
"resource_data": {"name": "RollbackOrg"},
340+
},
341+
]
342+
343+
org_count_before = Organization.objects.count()
344+
with pytest.raises(RuntimeError, match="Bulk resource update failed"):
345+
cmd._process_resource_page_batch(results, resource_context)
346+
347+
# Verify rollback: no new org should have been created
348+
assert Organization.objects.count() == org_count_before
349+
350+
178351
@pytest.mark.django_db
179352
def test_reconcile_existing_resource_matching_ansible_id_same_data():
180353
"""Case 1 with matching data: logs 'Correcting service_id'."""

aap_gateway_api/utils/resources_client.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ def get_default_user(self):
3535
return get_user_model().objects.first()
3636
return user
3737

38+
def bulk_update_resources(self, items: list[dict]):
39+
"""
40+
Bulk-update multiple resources in a single HTTP request.
41+
42+
Each item must contain 'ansible_id' and one or more fields to update:
43+
service_id, new_ansible_id, is_partially_migrated, resource_data.
44+
"""
45+
return self._make_request("post", "resources/bulk-update/", data=items)
46+
3847
def get_url_for_service(self, service):
3948
http_port = service.http_port
4049
protocol = "https" if http_port.use_https else "http"

0 commit comments

Comments
 (0)