Feat(eos_designs): Add act_cv_instance to target a dev CV instance - #7312
Feat(eos_designs): Add act_cv_instance to target a dev CV instance#7312natedoot wants to merge 7 commits into
Conversation
|
Review docs on Read the Docs To test this pull request: # Create virtual environment for this testing below the current directory
python -m venv test-avd-pr-7312
# Activate the virtual environment
source test-avd-pr-7312/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/natedoot/avd.git@feat/digital-twin-cv-instance#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
python -m ansible.cli.galaxy collection install git+https://github.com/natedoot/avd.git#/ansible_collections/arista/avd/,feat/digital-twin-cv-instance --force
cd test-avd-pr-7312
# Run your playbook using `python -m ansible.cli.playbook path/to/playbook.yml ...`You can also test this PR using AVD playground:
|
github-actions
Bot
added
role: eos_cli_config_gen
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds ChangesDigital Twin ACT CloudVision settings
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FabricActCvSettings
participant DaemonTerminattr
participant TerminAttr
FabricActCvSettings->>DaemonTerminattr: provide ACT CloudVision settings
DaemonTerminattr->>DaemonTerminattr: replace standard cv_settings
DaemonTerminattr->>TerminAttr: generate ACT-specific connection settings
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ansible_collections/arista/avd/extensions/molecule/digital_twin/intended/configs/digital-twin-cv-instance-1.cfg`:
- Around line 5-6: Regenerate the Digital Twin expected artifacts so the
TerminAttr command in
ansible_collections/arista/avd/extensions/molecule/digital_twin/intended/configs/digital-twin-cv-instance-1.cfg
uses the intended 10.20.30.40:9910 CV address and removes the stale
127.0.0.1:9911 reference. Keep the existing structured config values in
ansible_collections/arista/avd/extensions/molecule/digital_twin/digital_twin/intended/structured_configs/digital-twin-cv-instance-1.yml
at lines 4-6 and 31-33 unchanged, and leave the matching documentation entries
in
ansible_collections/arista/avd/extensions/molecule/digital_twin/documentation/fabric/DIGITAL_TWIN-documentation.md
lines 18-25 unchanged.
In
`@ansible_collections/arista/avd/extensions/molecule/digital_twin/intended/structured_configs/digital-twin-cv-instance-1.yml`:
- Around line 31-33: Add the expected alternate Digital Twin CloudVision
instance under the fixture's metadata.digital_twin.cv_instance structure,
alongside the existing is_deployed and fabric_name metadata. Use the overridden
daemon_terminattr.cvaddrs value so this intended structured configuration
validates metadata propagation.
In `@python-avd/pyavd/_eos_designs/structured_config/base/daemon_terminattr.py`:
- Around line 69-75: Normalize the shared act_cv_instance value before assigning
it to TerminAttr cvaddrs, converting URL-form inputs to the accepted host/port
format or rejecting them consistently. Apply this to both daemon_terminattr.py
ranges 69-75 and 89-91, preserving get_cv_addrs behavior for non-ACT clusters;
update the digital-twin-enforce-eapi-1.yml range 15 fixture if needed to cover
the normalized or rejected URL input.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 986d783b-6071-482a-a105-807ee6fd580c
📒 Files selected for processing (21)
ansible_collections/arista/avd/extensions/molecule/digital_twin/digital_twin/documentation/fabric/DIGITAL_TWIN-documentation.mdansible_collections/arista/avd/extensions/molecule/digital_twin/digital_twin/documentation/fabric/DIGITAL_TWIN-topology.ymlansible_collections/arista/avd/extensions/molecule/digital_twin/digital_twin/intended/configs/digital-twin-cv-instance-1.cfgansible_collections/arista/avd/extensions/molecule/digital_twin/digital_twin/intended/structured_configs/digital-twin-cv-instance-1.ymlansible_collections/arista/avd/extensions/molecule/digital_twin/digital_twin/intended/structured_configs/digital-twin-enforce-eapi-1.ymlansible_collections/arista/avd/extensions/molecule/digital_twin/documentation/fabric/DIGITAL_TWIN-documentation.mdansible_collections/arista/avd/extensions/molecule/digital_twin/intended/configs/digital-twin-cv-instance-1.cfgansible_collections/arista/avd/extensions/molecule/digital_twin/intended/structured_configs/digital-twin-cv-instance-1.ymlansible_collections/arista/avd/extensions/molecule/digital_twin/inventory/host_vars/digital-twin-cv-instance-1.ymlansible_collections/arista/avd/extensions/molecule/digital_twin/inventory/host_vars/digital-twin-enforce-eapi-1.ymlansible_collections/arista/avd/extensions/molecule/digital_twin/inventory/hosts.ymlansible_collections/arista/avd/roles/eos_cli_config_gen/docs/tables/metadata.mdansible_collections/arista/avd/roles/eos_designs/docs/tables/digital-twin-configuration.mdpython-avd/pyavd/_eos_cli_config_gen/schema/__init__.pypython-avd/pyavd/_eos_cli_config_gen/schema/eos_cli_config_gen.schema.ymlpython-avd/pyavd/_eos_cli_config_gen/schema/schema_fragments/metadata.schema.ymlpython-avd/pyavd/_eos_designs/schema/__init__.pypython-avd/pyavd/_eos_designs/schema/eos_designs.schema.ymlpython-avd/pyavd/_eos_designs/schema/schema_fragments/digital_twin.schema.ymlpython-avd/pyavd/_eos_designs/structured_config/base/daemon_terminattr.pypython-avd/pyavd/_eos_designs/structured_config/metadata/digital_twin.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devel #7312 +/- ##
=======================================
Coverage 92.87% 92.88%
=======================================
Files 783 783
Lines 43363 43366 +3
Branches 10541 10542 +1
=======================================
+ Hits 40273 40279 +6
+ Misses 1886 1885 -1
+ Partials 1204 1202 -2
🚀 New features to boost your workflow:
|
ClausHolbechArista
left a comment
ClausHolbechArista
left a comment
There was a problem hiding this comment.
cv_instance is a deprecated or maybe even removed key by now. We should not reintroduce this model. Please reimplement this with a pattern that resembles the cv_settings schema. Maybe it is even a full act_cv_settings: under the digital twin, where we just read that instead of the global cv_settings.
Thanks for the feedback Claus. I want to confirm the intended structure. A couple of questions:
Option A — sibling to fabric under digital_twin: Option B — under digital_twin.fabric alongside the other act_* keys:
Using $ref would inherit the full cv_settings key tree automatically: This keeps both schemas in sync, but also pulls in keys that may not be relevant in the digital twin context (e.g., set_source_interfaces, terminattr sub-settings). Alternatively, a trimmed subset would cherry-pick only the relevant sub-keys:
|
…on instance for Digital Twin Allow users to specify a different CloudVision instance for Digital Twin deployment via `digital_twin.fabric.act_cv_instance`. In ACT Digital Twin mode, this overrides `daemon_terminattr.cvaddrs` so the digital twin connects to the dev/staging CV instance instead of production. Fixes aristanetworks#6623
e7ec542 to
a014acd
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…cv_settings Addresses maintainer feedback on PR aristanetworks#7312. Replaces the flat `act_cv_instance` string with a structured `act_cv_settings` block under `digital_twin.fabric` that inherits the full `cv_settings` schema via `$ref`. When set in ACT Digital Twin mode, `act_cv_settings` fully replaces `cv_settings` for `daemon_terminattr` generation, giving users control over clusters, auth, VRF, and all terminattr settings.
github-actions
Bot
removed
the
role: eos_cli_config_gen
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python-avd/pyavd/_eos_designs/structured_config/base/daemon_terminattr.py`:
- Around line 37-40: Update the cv_settings selection in the daemon_terminattr
configuration method so ACT Digital Twin mode chooses
digital_twin.fabric.act_cv_settings before the guard that returns when global
cv_settings is unset. Preserve the existing global cv_settings fallback for
non-ACT configurations, and add a test covering an ACT-only configuration that
generates daemon_terminattr without top-level cv_settings.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d14205d1-14cb-420a-9699-8b2a8df60ea5
📒 Files selected for processing (7)
ansible_collections/arista/avd/extensions/molecule/digital_twin/digital_twin/intended/structured_configs/digital-twin-cv-instance-1.ymlansible_collections/arista/avd/extensions/molecule/digital_twin/inventory/host_vars/digital-twin-cv-instance-1.ymlansible_collections/arista/avd/roles/eos_designs/docs/tables/cloudvision-settings.mdpython-avd/pyavd/_eos_designs/schema/__init__.pypython-avd/pyavd/_eos_designs/schema/eos_designs.schema.ymlpython-avd/pyavd/_eos_designs/schema/schema_fragments/digital_twin.schema.ymlpython-avd/pyavd/_eos_designs/structured_config/base/daemon_terminattr.py
💤 Files with no reviewable changes (1)
- ansible_collections/arista/avd/extensions/molecule/digital_twin/digital_twin/intended/structured_configs/digital-twin-cv-instance-1.yml
Move act_cv_settings override before the cv_settings early-return guard so that act_cv_settings works even when global cv_settings is not set. Add missing device documentation files for digital-twin-cv-instance-1.
|
What I ended up changing:
|
|
Perfect. Thank you for this. I agree on everything you have done to address my comment. One more thing though, instead of act_cv_settings, this shold probably just be |
I was thinking the same. I'll adjust and resubmit once ready for review. |
…bric Per maintainer feedback, cv_settings is platform-agnostic and does not need the act_ prefix. Only settings specific to a digital twin platform (e.g., act_os_version, act_node_type) use the platform prefix. Also includes upstream doc formatting updates (False -> false in IPv6 routing summary tables).
|
|
All set - here's what changed: Renamed Inherits the full Behavior (daemon_terminattr.py): When in Digital Twin mode and User-facing config: |
2 participants
Allow users to specify a different CloudVision instance for Digital Twin deployment via
digital_twin.fabric.act_cv_instance. In ACT Digital Twin mode, this overridesdaemon_terminattr.cvaddrsso the digital twin connects to the dev/staging CV instance instead of production.Fixes #6623
Change Summary
Add act_cv_instance option to digital_twin.fabric allowing users to target a specific CloudVision instance for Digital Twin deployment, separate from production.
Related Issue(s)
Fixes aristanetworks/avd#6623
Component(s) name
arista.avd.eos_designs
Proposed changes
Organizations running separate dev/staging and production CloudVision instances need the ability to point their Digital Twin to a non-production CV instance. This PR adds a new digital_twin.fabric.act_cv_instance key that:
Data model:
eos_designs input
digital_twin:
fabric:
act_cv_instance: # e.g. "10.20.30.40:9910" or "cv-dev.example.com:443"
eos_cli_config_gen structured config output (metadata)
metadata:
digital_twin:
cv_instance:
The field is optional. When unset, behavior is unchanged — daemon_terminattr.cvaddrs uses the production addresses from cv_settings as before.
How to test
A dedicated molecule test host digital-twin-cv-instance-1 was added to the digital_twin scenario with:
Verified behavior:
Additionally, act_cv_instance was added to the existing digital-twin-enforce-eapi-1 host to verify metadata-only passthrough (no cv_settings / no daemon_terminattr).
All 8460 molecule scenario tests pass. Compiled schemas regenerated via pre-commit run schemas.
Repository Checklist
Summary by CodeRabbit