Background
Follow-up to #2192 (foundation) and PR #2193 (pytest + Zuul infrastructure). Part of Tier 3 (#2199). Closes the SONiC sync pipeline tests:
osism/tasks/conductor/sonic/sync.py (sync_sonic) is the entry-point Celery task that orchestrates SONiC config generation, NetBox export, file export, and cache management.
osism/tasks/conductor/sonic/exporter.py (save_config_to_netbox, export_config_to_file) writes the generated configuration to NetBox local context and to the SONiC export directory on disk.
These two modules share enough state (caches, device, generated config) that one combined test module is the smallest reasonable unit.
Scope
Add two test modules:
tests/unit/tasks/conductor/sonic/test_sync.py — for osism/tasks/conductor/sonic/sync.py
tests/unit/tasks/conductor/sonic/test_exporter.py — for osism/tasks/conductor/sonic/exporter.py
Test targets
sync_sonic(device_name=None, task_id=None, show_diff=True) — sync.py:25
Patch the heavy external dependencies at their import site inside sync.py:
osism.tasks.conductor.sonic.sync.utils.nb
...get_nb_device_query_list_sonic
...find_interconnected_devices
...calculate_minimum_as_for_group
...generate_sonic_config
...save_config_to_netbox
...export_config_to_file
...clear_interface_cache, ...clear_all_caches, ...clear_vip_addresses_cache
..._load_metalbox_devices_cache, ...load_vip_addresses_cache
...get_interface_cache_stats
...utils.push_task_output, ...utils.finish_task_output
Do not test find_interconnected_devices or generate_sonic_config here — they have their own issues. Stub them.
Cache lifecycle
- Caches cleared at start (
clear_interface_cache, clear_all_caches) and at end (clear_interface_cache, clear_all_caches, clear_vip_addresses_cache) — verify the call order
_load_metalbox_devices_cache and load_vip_addresses_cache called once per invocation
Single-device path (device_name="sw-1")
- Device exists, role slug ∈
DEFAULT_SONIC_ROLES → device added, processed
- Device exists, role slug ∉ allowed list → warning logged, returns
{}
- Device exists,
role is None → warning logged, returns {}
nb.dcim.devices.get returns None → error logged, returns {}
nb.dcim.devices.get raises → error logged, returns {}
- Single device is a spine → all spine/superspine devices fetched for group detection (verify
nb.dcim.devices.filter called via nb_device_query_list)
- Single device is a leaf → uses devices list as-is (no extra fetch)
Multi-device path (device_name=None)
nb_device_query_list produces devices; only those with allowed roles are kept
- Devices without
role skipped silently
Per-device processing
- HWSKU missing → device skipped, debug log
- HWSKU not in
SUPPORTED_HWSKUS → warning logged, device skipped
- HWSKU valid →
generate_sonic_config(device, hwsku, device_as_mapping, config_version) called and result stored under device.name
config_version taken from device.custom_fields["sonic_parameters"]["config_version"] if present
- AS mapping built from
calculate_minimum_as_for_group for each spine group
- AS mapping passed into
generate_sonic_config
- Group with
min_as=None → not propagated to device_as_mapping
Diff handling (show_diff=True)
save_config_to_netbox(..., return_diff=True) returns (True, diff_text) and task_id set → diff streamed via push_task_output
- Returns
(True, None) (first-time configuration) → "First-time configuration created" message pushed
- Returns
(False, None) → no diff output, but export_config_to_file still called
show_diff=False → save_config_to_netbox(device, sonic_config) called without return_diff
Task output
task_id set → push_task_output("Processing device: ...") per device, plus finish_task_output(task_id, rc=0) at end
task_id None → no push_task_output calls
Cache stats
get_interface_cache_stats non-empty → debug log
- Empty/
None stats → no log
save_config_to_netbox(device, config, return_diff=False) — exporter.py:15
Patch osism.tasks.conductor.sonic.exporter.utils.nb.
device.local_context_data None → first-time path, device.save() called, returns True (or (True, None) with return_diff)
device.local_context_data {"sonic_config": same} → DeepDiff empty, returns False / (False, None), device.save() not called
device.local_context_data differs → diff generated via difflib.unified_diff, journal entry created via nb.extras.journal_entries.create, device.save() called, returns True / (True, diff_text)
- Journal create raises → error logged, but device save still happens, return value still indicates change
device.save() (or any other op) raises → error logged, returns False / (False, None)
- Diff returned in tuple form when
return_diff=True; bool form otherwise
export_config_to_file(device, config) — exporter.py:106
Patch osism.tasks.conductor.sonic.exporter.settings, os.makedirs, os.path.exists, os.path.islink, os.symlink, os.remove, and builtins.open (mock_open). Patch get_device_hostname.
Filename selection
SONIC_EXPORT_IDENTIFIER="hostname" → uses get_device_hostname(device)
SONIC_EXPORT_IDENTIFIER="serial-number" and device.serial="ABC123" → uses serial
serial-number mode but device.serial empty/missing → warning logged, falls back to hostname
- Filename is
f"{prefix}{identifier}{suffix}", joined under export_dir
Diff handling
- File missing → write happens, returns
True
- File exists, JSON identical → no write, returns
False
- File exists, JSON differs → write happens, returns
True
- File exists but unreadable / invalid JSON → warning logged, write happens, returns
True
Symlink handling (only when serial-number + device.serial truthy)
- Hostname filepath does not exist →
os.symlink called
- Hostname filepath exists as a regular file →
os.remove called first, then os.symlink
- Hostname filepath is a dangling symlink (
os.path.exists=False, os.path.islink=True) → os.remove called first
os.symlink raises → error logged, but function still returns True (the config was written successfully)
identifier_type="hostname" → no symlink attempt
identifier_type="serial-number" but device.serial falsy → no symlink attempt, debug log
Error handling
- Outer exception (e.g.
os.makedirs fails) → error logged, returns False
Mocking hints
- Build a small reusable device factory:
SimpleNamespace(name="sw-1", id=1, serial="ABC", role=SimpleNamespace(slug="leaf"), local_context_data=None, custom_fields={"sonic_parameters": {"hwsku": "Accton-AS7326-56X"}}, config_context={}).
- Stub
generate_sonic_config to return a minimal config like {"PORT": {"Ethernet0": {}}} so the len(sonic_config["PORT"]) log doesn't crash.
- For
_load_metalbox_devices_cache / clear_all_caches patches: just verify call counts. Don't try to exercise the real cache.
- For
export_config_to_file, prefer a tmp_path fixture for real filesystem if it simplifies the diff/symlink tests; otherwise patch os.* and open.
- For
save_config_to_netbox, the DeepDiff call works on plain dicts — no extra mocking needed for the equality path.
Definition of Done
Dependencies
Background
Follow-up to #2192 (foundation) and PR #2193 (pytest + Zuul infrastructure). Part of Tier 3 (#2199). Closes the SONiC sync pipeline tests:
osism/tasks/conductor/sonic/sync.py(sync_sonic) is the entry-point Celery task that orchestrates SONiC config generation, NetBox export, file export, and cache management.osism/tasks/conductor/sonic/exporter.py(save_config_to_netbox,export_config_to_file) writes the generated configuration to NetBox local context and to the SONiC export directory on disk.These two modules share enough state (caches, device, generated config) that one combined test module is the smallest reasonable unit.
Scope
Add two test modules:
tests/unit/tasks/conductor/sonic/test_sync.py— forosism/tasks/conductor/sonic/sync.pytests/unit/tasks/conductor/sonic/test_exporter.py— forosism/tasks/conductor/sonic/exporter.pyTest targets
sync_sonic(device_name=None, task_id=None, show_diff=True)—sync.py:25Patch the heavy external dependencies at their import site inside
sync.py:osism.tasks.conductor.sonic.sync.utils.nb...get_nb_device_query_list_sonic...find_interconnected_devices...calculate_minimum_as_for_group...generate_sonic_config...save_config_to_netbox...export_config_to_file...clear_interface_cache,...clear_all_caches,...clear_vip_addresses_cache..._load_metalbox_devices_cache,...load_vip_addresses_cache...get_interface_cache_stats...utils.push_task_output,...utils.finish_task_outputDo not test
find_interconnected_devicesorgenerate_sonic_confighere — they have their own issues. Stub them.Cache lifecycle
clear_interface_cache,clear_all_caches) and at end (clear_interface_cache,clear_all_caches,clear_vip_addresses_cache) — verify the call order_load_metalbox_devices_cacheandload_vip_addresses_cachecalled once per invocationSingle-device path (
device_name="sw-1")DEFAULT_SONIC_ROLES→ device added, processed{}roleisNone→ warning logged, returns{}nb.dcim.devices.getreturnsNone→ error logged, returns{}nb.dcim.devices.getraises → error logged, returns{}nb.dcim.devices.filtercalled vianb_device_query_list)Multi-device path (
device_name=None)nb_device_query_listproduces devices; only those with allowed roles are keptroleskipped silentlyPer-device processing
SUPPORTED_HWSKUS→ warning logged, device skippedgenerate_sonic_config(device, hwsku, device_as_mapping, config_version)called and result stored underdevice.nameconfig_versiontaken fromdevice.custom_fields["sonic_parameters"]["config_version"]if presentcalculate_minimum_as_for_groupfor each spine groupgenerate_sonic_configmin_as=None→ not propagated todevice_as_mappingDiff handling (
show_diff=True)save_config_to_netbox(..., return_diff=True)returns(True, diff_text)andtask_idset → diff streamed viapush_task_output(True, None)(first-time configuration) → "First-time configuration created" message pushed(False, None)→ no diff output, butexport_config_to_filestill calledshow_diff=False→save_config_to_netbox(device, sonic_config)called withoutreturn_diffTask output
task_idset →push_task_output("Processing device: ...")per device, plusfinish_task_output(task_id, rc=0)at endtask_idNone→ nopush_task_outputcallsCache stats
get_interface_cache_statsnon-empty → debug logNonestats → no logsave_config_to_netbox(device, config, return_diff=False)—exporter.py:15Patch
osism.tasks.conductor.sonic.exporter.utils.nb.device.local_context_dataNone→ first-time path,device.save()called, returnsTrue(or(True, None)withreturn_diff)device.local_context_data{"sonic_config": same}→DeepDiffempty, returnsFalse/(False, None),device.save()not calleddevice.local_context_datadiffers → diff generated viadifflib.unified_diff, journal entry created vianb.extras.journal_entries.create,device.save()called, returnsTrue/(True, diff_text)device.save()(or any other op) raises → error logged, returnsFalse/(False, None)return_diff=True; bool form otherwiseexport_config_to_file(device, config)—exporter.py:106Patch
osism.tasks.conductor.sonic.exporter.settings,os.makedirs,os.path.exists,os.path.islink,os.symlink,os.remove, andbuiltins.open(mock_open). Patchget_device_hostname.Filename selection
SONIC_EXPORT_IDENTIFIER="hostname"→ usesget_device_hostname(device)SONIC_EXPORT_IDENTIFIER="serial-number"anddevice.serial="ABC123"→ uses serialserial-numbermode butdevice.serialempty/missing → warning logged, falls back to hostnamef"{prefix}{identifier}{suffix}", joined underexport_dirDiff handling
TrueFalseTrueTrueSymlink handling (only when
serial-number+device.serialtruthy)os.symlinkcalledos.removecalled first, thenos.symlinkos.path.exists=False, os.path.islink=True) →os.removecalled firstos.symlinkraises → error logged, but function still returnsTrue(the config was written successfully)identifier_type="hostname"→ no symlink attemptidentifier_type="serial-number"butdevice.serialfalsy → no symlink attempt, debug logError handling
os.makedirsfails) → error logged, returnsFalseMocking hints
SimpleNamespace(name="sw-1", id=1, serial="ABC", role=SimpleNamespace(slug="leaf"), local_context_data=None, custom_fields={"sonic_parameters": {"hwsku": "Accton-AS7326-56X"}}, config_context={}).generate_sonic_configto return a minimal config like{"PORT": {"Ethernet0": {}}}so thelen(sonic_config["PORT"])log doesn't crash._load_metalbox_devices_cache/clear_all_cachespatches: just verify call counts. Don't try to exercise the real cache.export_config_to_file, prefer atmp_pathfixture for real filesystem if it simplifies the diff/symlink tests; otherwise patchos.*andopen.save_config_to_netbox, theDeepDiffcall works on plain dicts — no extra mocking needed for the equality path.Definition of Done
tests/unit/tasks/conductor/sonic/test_sync.pyandtests/unit/tasks/conductor/sonic/test_exporter.pycreatedpytest --cov=osism.tasks.conductor.sonic.syncandpytest --cov=osism.tasks.conductor.sonic.exporter≥ 90 %pipenv run pytest tests/unit/tasks/conductor/sonic/test_sync.py tests/unit/tasks/conductor/sonic/test_exporter.pypasses locallyflake8,mypy,python-blackremain greenpython-osism-unit-testspassesDependencies