Skip to content

Commit 1ea1d9c

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 08969bc commit 1ea1d9c

3 files changed

Lines changed: 370 additions & 4 deletions

File tree

aap_gateway_api/management/commands/_migrate_service_data/resource_migration.py

Lines changed: 97 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,100 @@ 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+
@staticmethod
257+
def _build_bulk_update_item(resource_ansible_id: str, updated_service_resource: Dict[str, Any]) -> Dict[str, Any]:
258+
"""Build a single bulk-update payload item from the reconciled service resource fields."""
259+
bulk_item: Dict[str, Any] = {"ansible_id": resource_ansible_id}
260+
if "service_id" in updated_service_resource:
261+
bulk_item["service_id"] = updated_service_resource["service_id"]
262+
if "is_partially_migrated" in updated_service_resource:
263+
bulk_item["is_partially_migrated"] = updated_service_resource["is_partially_migrated"]
264+
if "ansible_id" in updated_service_resource:
265+
bulk_item["new_ansible_id"] = str(updated_service_resource["ansible_id"])
266+
if "resource_data" in updated_service_resource:
267+
bulk_item["resource_data"] = updated_service_resource["resource_data"]
268+
return bulk_item
269+
270+
def _send_bulk_update(self, bulk_update_items: List[Dict[str, Any]]) -> int:
271+
"""Send bulk update to upstream and return the number of successfully updated items.
272+
273+
Per-item errors are logged as warnings. Items whose upstream update failed
274+
will reappear on the next query (the filter naturally retries them).
275+
"""
276+
resp = self.client.bulk_update_resources(bulk_update_items)
277+
if resp.status_code != 200:
278+
self._log(
279+
f"Bulk resource update returned HTTP {resp.status_code}. Items will be retried on next page fetch. Response: {resp.text[:500]}",
280+
logging.WARNING,
281+
)
282+
return 0
283+
284+
resp_data = resp.json()
285+
if resp_data.get("errors"):
286+
for err in resp_data["errors"]:
287+
self._log(
288+
f"Warning: bulk-update failed for {err.get('ansible_id')}: {err.get('error')}",
289+
logging.WARNING,
290+
)
291+
return resp_data.get("updated", 0)
292+
293+
def _process_resource_page_batch(self, results: List[Dict[str, Any]], resource_context: Dict[str, Any]) -> int:
294+
"""
295+
Process and migrate a batch of resource items from a single API page.
296+
297+
Follows the same pattern as role assignments: validates all items,
298+
performs bulk local writes, then sends a single bulk HTTP call to
299+
update upstream. Per-item failures are logged as warnings rather than
300+
aborting the entire page — failed items will reappear on the next
301+
iteration since their service_id/is_partially_migrated was not updated.
302+
303+
Args:
304+
results: List of resource items from the upstream service API page
305+
resource_context: Static data about the resource type
306+
307+
Returns:
308+
Number of items successfully processed in this batch
309+
"""
310+
resource_type = resource_context["type"]
311+
bulk_update_items = []
312+
create_operations = []
313+
314+
for upstream_resource_item in results:
315+
resource_ansible_id = upstream_resource_item["ansible_id"]
316+
317+
if "resource_data" not in upstream_resource_item:
318+
raise RuntimeError(
319+
f"Resource {resource_ansible_id} is missing 'resource_data'. Ensure all services are running a version of DAB that supports extra_fields."
320+
)
321+
322+
upstream_resource = upstream_resource_item
323+
validated_resource_data = self._deserialize_and_validate_resource_data(upstream_resource, resource_context["type_serializer"])
324+
325+
if resource_context["type_name"] == SHARED_USER_RESOURCE_TYPE:
326+
upstream_resource = self._sync_user_superuser_flag(upstream_resource, validated_resource_data)
327+
validated_resource_data = self._deserialize_and_validate_resource_data(upstream_resource, resource_context["type_serializer"])
328+
329+
resource_creation_kwargs, updated_service_resource = self._initialize_resource_sync_payloads(upstream_resource)
330+
create_gateway_resource = self._reconcile_existing_resource(upstream_resource, resource_context, validated_resource_data, updated_service_resource)
331+
332+
if create_gateway_resource:
333+
create_operations.append((resource_type, upstream_resource["resource_data"], resource_creation_kwargs))
334+
335+
bulk_update_items.append(self._build_bulk_update_item(resource_ansible_id, updated_service_resource))
336+
337+
# Create gateway resources locally. If a resource already exists (e.g. from a
338+
# previous interrupted run), the reconcile logic above will have set
339+
# create_gateway_resource=False, so duplicates are safe.
340+
with transaction.atomic():
341+
for rt, resource_data, creation_kwargs in create_operations:
342+
Resource.create_resource(rt, resource_data, **creation_kwargs)
343+
344+
# Send bulk update to upstream service.
345+
if bulk_update_items:
346+
return self._send_bulk_update(bulk_update_items)
347+
348+
return len(results)
349+
256350
def migrate_resource(self, resource_type_name: str) -> None:
257351
"""
258352
Migrate all resources of a specific type from upstream service to Gateway.
@@ -328,7 +422,6 @@ def migrate_resource(self, resource_type_name: str) -> None:
328422
self._log("No more items remaining to migrate.", logging.INFO)
329423
break
330424

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)
425+
batch_size = self._process_resource_page_batch(results, resource_context)
426+
resource_processed += batch_size
427+
self._log_progress(progress_label, resource_processed, resource_total)

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

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,270 @@ 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+
batch_org_id = str(uuid.uuid4())
222+
new_org_id = str(uuid.uuid4())
223+
results = [
224+
{
225+
"ansible_id": batch_org_id,
226+
"name": "BatchOrg1",
227+
"resource_type": "shared.organization",
228+
"resource_data": {"name": "BatchOrg1"},
229+
},
230+
{
231+
"ansible_id": new_org_id,
232+
"name": "NewOrg",
233+
"resource_type": "shared.organization",
234+
"resource_data": {"name": "NewOrg"},
235+
},
236+
]
237+
238+
count = cmd._process_resource_page_batch(results, resource_context)
239+
# Returns the "updated" count from the bulk response
240+
assert count == 2
241+
242+
mock_client.bulk_update_resources.assert_called_once()
243+
bulk_items = mock_client.bulk_update_resources.call_args[0][0]
244+
assert len(bulk_items) == 2
245+
assert all("ansible_id" in item for item in bulk_items)
246+
# The first item (BatchOrg1 exists) triggers reconcile which sets ansible_id and resource_data
247+
merged_item = bulk_items[0]
248+
assert "new_ansible_id" in merged_item
249+
assert "resource_data" in merged_item
250+
# The second item (NewOrg is new) only gets service_id
251+
new_item = bulk_items[1]
252+
assert "service_id" in new_item
253+
254+
255+
@pytest.mark.django_db
256+
def test_process_resource_page_batch_with_partially_migrated():
257+
"""Test that is_partially_migrated is included in bulk payload when set."""
258+
import uuid
259+
from unittest.mock import patch as mock_patch
260+
261+
from ansible_base.resource_registry.models import ResourceType
262+
263+
cmd = MigrateCommand()
264+
cmd.stdout = StringIO()
265+
cmd.stderr = StringIO()
266+
cmd.upstream_service_id = str(uuid.uuid4())
267+
268+
mock_client = Mock()
269+
mock_client.service.service_cluster.service_type.name = "awx"
270+
mock_resp = Mock()
271+
mock_resp.status_code = 200
272+
mock_resp.json.return_value = {"updated": 1, "errors": []}
273+
mock_client.bulk_update_resources.return_value = mock_resp
274+
cmd.client = mock_client
275+
276+
org_resource_type = ResourceType.objects.get(name="shared.organization")
277+
resource_context = {
278+
"type": org_resource_type,
279+
"type_name": "shared.organization",
280+
"type_serializer": org_resource_type.serializer_class,
281+
"type_name_field": org_resource_type.get_resource_config().name_field,
282+
"unique_fields": ["name"],
283+
"LocalResourceModel": Organization,
284+
}
285+
286+
results = [
287+
{
288+
"ansible_id": str(uuid.uuid4()),
289+
"name": "PartialOrg",
290+
"resource_type": "shared.organization",
291+
"resource_data": {"name": "PartialOrg"},
292+
},
293+
]
294+
295+
# Mock _reconcile_existing_resource to inject is_partially_migrated
296+
def mock_reconcile(upstream_resource, ctx, validated_data, updated_service_resource):
297+
updated_service_resource["is_partially_migrated"] = True
298+
return True
299+
300+
with mock_patch.object(cmd, "_reconcile_existing_resource", side_effect=mock_reconcile):
301+
count = cmd._process_resource_page_batch(results, resource_context)
302+
303+
assert count == 1
304+
bulk_items = mock_client.bulk_update_resources.call_args[0][0]
305+
assert bulk_items[0]["is_partially_migrated"] is True
306+
307+
308+
@pytest.mark.django_db
309+
def test_process_resource_page_batch_graceful_on_bulk_failure():
310+
"""Test that bulk update HTTP failure is non-fatal and returns 0 (items will be retried)."""
311+
import uuid
312+
313+
from ansible_base.resource_registry.models import ResourceType
314+
315+
cmd = MigrateCommand()
316+
cmd.stdout = StringIO()
317+
cmd.stderr = StringIO()
318+
cmd.upstream_service_id = str(uuid.uuid4())
319+
320+
mock_client = Mock()
321+
mock_client.service.service_cluster.service_type.name = "awx"
322+
mock_resp = Mock()
323+
mock_resp.status_code = 500
324+
mock_resp.text = "Internal Server Error"
325+
mock_client.bulk_update_resources.return_value = mock_resp
326+
cmd.client = mock_client
327+
328+
org_resource_type = ResourceType.objects.get(name="shared.organization")
329+
resource_context = {
330+
"type": org_resource_type,
331+
"type_name": "shared.organization",
332+
"type_serializer": org_resource_type.serializer_class,
333+
"type_name_field": org_resource_type.get_resource_config().name_field,
334+
"unique_fields": ["name"],
335+
"LocalResourceModel": Organization,
336+
}
337+
338+
results = [
339+
{
340+
"ansible_id": str(uuid.uuid4()),
341+
"name": "RetryOrg",
342+
"resource_type": "shared.organization",
343+
"resource_data": {"name": "RetryOrg"},
344+
},
345+
]
346+
347+
# Bulk failure is non-fatal: returns 0 and logs warning (items retry next iteration)
348+
count = cmd._process_resource_page_batch(results, resource_context)
349+
assert count == 0
350+
351+
# Local gateway resource was still created (will be reconciled on retry)
352+
assert Organization.objects.filter(name="RetryOrg").exists()
353+
354+
355+
@pytest.mark.django_db
356+
def test_process_resource_page_batch_partial_errors():
357+
"""Test that per-item errors from bulk update are logged as warnings."""
358+
import uuid
359+
360+
from ansible_base.resource_registry.models import ResourceType
361+
362+
cmd = MigrateCommand()
363+
cmd.stdout = StringIO()
364+
cmd.stderr = StringIO()
365+
cmd.upstream_service_id = str(uuid.uuid4())
366+
367+
mock_client = Mock()
368+
mock_client.service.service_cluster.service_type.name = "awx"
369+
mock_resp = Mock()
370+
mock_resp.status_code = 200
371+
mock_resp.json.return_value = {
372+
"updated": 1,
373+
"errors": [{"ansible_id": "some-id", "error": "Resource not found."}],
374+
}
375+
mock_client.bulk_update_resources.return_value = mock_resp
376+
cmd.client = mock_client
377+
378+
Organization.objects.create(name="PartialErrOrg")
379+
380+
org_resource_type = ResourceType.objects.get(name="shared.organization")
381+
resource_context = {
382+
"type": org_resource_type,
383+
"type_name": "shared.organization",
384+
"type_serializer": org_resource_type.serializer_class,
385+
"type_name_field": org_resource_type.get_resource_config().name_field,
386+
"unique_fields": ["name"],
387+
"LocalResourceModel": Organization,
388+
}
389+
390+
results = [
391+
{
392+
"ansible_id": str(uuid.uuid4()),
393+
"name": "PartialErrOrg",
394+
"resource_type": "shared.organization",
395+
"resource_data": {"name": "PartialErrOrg"},
396+
},
397+
{
398+
"ansible_id": str(uuid.uuid4()),
399+
"name": "NewPartialOrg",
400+
"resource_type": "shared.organization",
401+
"resource_data": {"name": "NewPartialOrg"},
402+
},
403+
]
404+
405+
count = cmd._process_resource_page_batch(results, resource_context)
406+
# Only 1 updated (the other had an error on upstream side)
407+
assert count == 1
408+
# Warning was logged for the failed item
409+
output = cmd.stderr.getvalue()
410+
assert "bulk-update failed" in output
411+
412+
413+
def test_build_bulk_update_item_all_fields():
414+
"""Test that _build_bulk_update_item includes all present fields."""
415+
import uuid
416+
417+
cmd = MigrateCommand()
418+
ansible_id = str(uuid.uuid4())
419+
new_ansible_id = uuid.uuid4()
420+
updated_service_resource = {
421+
"service_id": "svc-123",
422+
"is_partially_migrated": True,
423+
"ansible_id": new_ansible_id,
424+
"resource_data": {"username": "test"},
425+
}
426+
427+
result = cmd._build_bulk_update_item(ansible_id, updated_service_resource)
428+
assert result["ansible_id"] == ansible_id
429+
assert result["service_id"] == "svc-123"
430+
assert result["is_partially_migrated"] is True
431+
assert result["new_ansible_id"] == str(new_ansible_id)
432+
assert result["resource_data"] == {"username": "test"}
433+
434+
435+
def test_build_bulk_update_item_minimal():
436+
"""Test that _build_bulk_update_item only includes ansible_id when no updates."""
437+
cmd = MigrateCommand()
438+
result = cmd._build_bulk_update_item("some-id", {})
439+
assert result == {"ansible_id": "some-id"}
440+
441+
178442
@pytest.mark.django_db
179443
def test_reconcile_existing_resource_matching_ansible_id_same_data():
180444
"""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)