Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI(cv_deploy): Enhance logic and readability of cv_deploy molecule scenario #5034

Open
wants to merge 6 commits into
base: devel
Choose a base branch
from

Conversation

alexeygorbunov
Copy link
Contributor

@alexeygorbunov alexeygorbunov commented Feb 12, 2025

Change Summary

Enhance logic and readability of cv_deploy molecule scenario

Related Issue(s)

Component(s) name

arista.avd.cv_deploy

Proposed changes

  • Enhance naming convention of different playbook and CV elements

Examples of the new WS names:

avd-cv-deploy_global-scenario-cleanup_n4w7_cleanup
avd-cv-deploy_force-ws-true_run-cc-false_qqul_cleanup
avd-cv-deploy_force-ws-true_run-cc-false_qqul_converge
  • Introduce tagging to control execution of specific tests
  • Change approach of assigning cv_deploy - related variables

How to test

Run cv_deploy scenario in CI

Checklist

User Checklist

  • N/A

Repository Checklist

  • My code has been rebased from devel before I start
  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation and documentation have been updated accordingly.
  • I have updated molecule CI testing accordingly. (check the box if not applicable)

@alexeygorbunov alexeygorbunov requested review from a team as code owners February 12, 2025 22:55
@github-actions github-actions bot added the state: CI Updated CI scenario have been updated in the PR label Feb 12, 2025
Copy link

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-5034
# Activate the virtual environment
source test-avd-pr-5034/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/alexeygorbunov/avd.git@cv_deploy_ci_molecule_enhancement#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/alexeygorbunov/avd.git#/ansible_collections/arista/avd/,cv_deploy_ci_molecule_enhancement --force
# Optional: Install AVD examples
cd test-avd-pr-5034
ansible-playbook arista.avd.install_examples

@alexeygorbunov alexeygorbunov marked this pull request as draft February 12, 2025 23:27
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor question on r but I am fine either way.

run_once: true
ansible.builtin.set_fact:
r: "{{ lookup('password', '/dev/null chars=ascii_lowercase,digits length=4') }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be best to set this as a play var all the way at the top? So we have a single ID per execution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree! Before this unique r was the only way to distinguish results of the different tests. Now all tests have unique names and having them share the same execution ID makes more sense.
I tried to implement it natively with molecule but faced issues here and there. Ended up implementing via externally-fed env which will be generated as a part of the CI pipeline.

New naming convention example (from last manual run):

avd_cv-deploy_scenario-cleanup_tg1b_cleanup
avd_cv-deploy_force-ws-true_run-cc-false_tg1b_cleanup
avd_cv-deploy_force-ws-true_run-cc-false_tg1b_converge
avd_cv-deploy_inactive-device_force-ws-false_tg1b_cleanup
avd_cv-deploy_inactive-device_force-ws-false_tg1b_converge
avd_cv-deploy_inactive-device_force-ws-true_tg1b_cleanup
avd_cv-deploy_inactive-device_force-ws-true_tg1b_converge
avd-cv-deploy_submit-ws-false_tg1b_cleanup
avd-cv-deploy_submit-ws-false_tg1b_converge
avd_cv-deploy_force-ws-true_run-cc-true_tg1b_cleanup
avd_cv-deploy_force-ws-true_run-cc-true_tg1b_converge

Same consistency is implemented in other scenarios that rely on cv_deploy

@ClausHolbechArista ClausHolbechArista requested a review from a team February 14, 2025 12:08
@ClausHolbechArista ClausHolbechArista added the one approval This PR has one approval and is only missing one more. label Feb 14, 2025
@carlbuchmann carlbuchmann self-requested a review February 14, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
one approval This PR has one approval and is only missing one more. state: CI Updated CI scenario have been updated in the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants