Add unit tests for SONiC config_generator orchestrator and service ca…

[berendt]

…ches

Covers natural_sort_key, generate_sonic_config (base-config loading, deepcopy, primary-IP / AS routing, OOB management, breakout merge, config_version normalization), the metalbox / NTP / DNS caches, and the log-server / SNMP helpers. Helpers from companion sub-issues are patched at their import sites so this file exercises only the orchestrator's own glue.

Also hardens generate_sonic_config so the top-level scaffold keys it and its helpers index into directly are always present, even when /etc/sonic/config_db.json is absent or fails to load.

Closes #2221

AI-assisted: Claude Code

[sourcery-ai]

Hey - I've left some high level feedback:

• The list of top-level scaffold keys is duplicated between generate_sonic_config and the _make_base_config test helper; consider extracting this list into a single constant or helper used by both to avoid drift when keys change.
• The test_config_generator_orchestrator.py file has grown very large and spans multiple distinct concerns (orchestrator, metalbox cache, NTP, DNS, log, SNMP); consider splitting it into smaller test modules per concern to keep future changes more manageable.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The list of top-level scaffold keys is duplicated between `generate_sonic_config` and the `_make_base_config` test helper; consider extracting this list into a single constant or helper used by both to avoid drift when keys change.
- The `test_config_generator_orchestrator.py` file has grown very large and spans multiple distinct concerns (orchestrator, metalbox cache, NTP, DNS, log, SNMP); consider splitting it into smaller test modules per concern to keep future changes more manageable.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[ideaship]

[config_generator.py:319] Dead guard introduced by this PR

The if "VERSIONS" not in config: guard at line 319 is now dead: adding "VERSIONS" to TOP_LEVEL_SCAFFOLD_KEYS guarantees it is present via setdefault at line 193. Remove lines 319–320 only — the inner guard at line 321 (if "DATABASE" not in config["VERSIONS"]:) is still live because the scaffold only creates an empty dict, not the nested key.

[ideaship]

TOP_LEVEL_SCAFFOLD_KEYS is missing sections the orchestrator writes into directly: BGP_NEIGHBOR_AF (e.g. line 1020), BGP_NEIGHBOR (line 1202), BGP_GLOBALS_AF_NETWORK (line 1819), BGP_GLOBALS_AF (line 2014), BGP_GLOBALS_ROUTE_ADVERTISE (line 2035), VXLAN_TUNNEL, VXLAN_EVPN_NVO, VXLAN_TUNNEL_MAP (lines 2067–2086). With a missing or sparse config_db.json, any device with connected ports, Loopback0 routes, or VRFs with VNI will raise KeyError at runtime. These eight keys need to be added to the tuple.

[ideaship]

_get_ntp_servers is not called from production code — _add_ntp_configuration reaches the metalbox IP via _get_metalbox_ip_for_device directly, never through this function. The associated _ntp_servers_cache global and clear_ntp_cache() are also unreachable. These tests exercise an orphaned private function. Recommend either removing _get_ntp_servers, _ntp_servers_cache, and clear_ntp_cache() if the NetBox-crawl NTP path is obsolete, or wiring the function back into _add_ntp_configuration if it is intended behaviour.