Feat(cv_deploy): Expose the details of the Workspace build validation errors #4629
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-4629
# Activate the virtual environment
source test-avd-pr-4629/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/alexeygorbunov/avd.git@cv_deploy_ci#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 --force
# Optional: Install AVD examples
cd test-avd-pr-4629
ansible-playbook arista.avd.install_examples |
github-actions
bot
added
the
state: CI Updated
ansible_collections/arista/avd/molecule/cv_deploy/invalid_config.yml
Outdated
Show resolved
Hide resolved
github-actions
bot
added
type: documentation
alexeygorbunov
changed the title
+ Expose WS build processing times
| build_warnings: "{{ cv_workspace_build_warnings.enabled | arista.avd.default(true) }}" | ||
| build_warnings_suppress_patterns: "{{ cv_workspace_build_warnings.suppress_patterns | arista.avd.default([]) }}" | ||
| build_warnings_suppress_portfast: "{{ cv_workspace_build_warnings.suppress_portfast | arista.avd.default(false) }}" |
There was a problem hiding this comment.
you don't need the default filter here since you already set them in the role defaults.
There was a problem hiding this comment.
fixed, but had to fallback to plain-level variables now (vs dict), otherwise defining only one key (like 'enabled') when calling role would make other dict attributes undefined
was:
cv_workspace_build_warnings:
enabled: true
suppress_patterns: []
suppress_portfast: false
now:
cv_workspace_build_warnings_enabled: true
cv_workspace_build_warnings_suppress_patterns: []
cv_workspace_build_warnings_suppress_portfast: false
There was a problem hiding this comment.
Otherwise go back to dict and just remove it from default/main.yml and add back the arista.avd.default filters here. Up to you.
There was a problem hiding this comment.
We can keep current flatten approach (used by most of the other cv_deploy's vars anyway) as long as you are OK with it
|
|
Moving to draft until comments have been addressed. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
| @@ -12,46 +12,50 @@ | |||
| structured_dir: "{{ playbook_dir }}/intended/structured_configs/test_configs" | |||
| intended_tag_device: avd-ci-leaf1 | |||
| intended_tags: "{{ lookup('file', structured_dir ~ '/' ~ intended_tag_device ~ '.yml')| from_yaml }}" | |||
| test_id: "cc-false" | |||
There was a problem hiding this comment.
Should we also add tags with this name on all the tasks related to this test? To make it easier to just run some of them?
There was a problem hiding this comment.
Added. See commit 'PR comments - tag cv_deploy molecule play tasks for flexible scope control'
| ! | ||
| ntp server vrf MGMT 1.pool.ntp.org iburst local-interface Management1 | ||
| ! | ||
| end |
There was a problem hiding this comment.
Please make sure all config files have a closing new-line. It seems like some do and some dont.
There was a problem hiding this comment.
Fixed in commit 'PR comments - end-of-file-fixer for avd-ci-core1.cfg'.
Easy to miss as pre-commit excludes these molecule files
| @@ -0,0 +1,2 @@ | |||
| hostname: avd-ci-leaf2 | |||
There was a problem hiding this comment.
We can remove these files now, since they only contain the default values.
There was a problem hiding this comment.
Fixed in commit PR comments - Delete structure_config files containing only default settings
| build_id: str, | ||
| time: datetime | None = None, | ||
| timeout: float = DEFAULT_API_TIMEOUT, | ||
| ) -> WorkspaceBuildDetailsStreamResponse: |
There was a problem hiding this comment.
Please try to normalise the output a bit, so we don't return raw response objects. The resonses object is an async generator, so we need to process it inside this function to catch errors correctly. You could do return [response.to_dict(Casing.SNAKE) async for response in reponses]
There was a problem hiding this comment.
Fixed in 'PR Comments - normalize return of get_workspace_build_details'. Tested in CI environment with cv_deploy molecule scenario
Change Summary
Explicitly expose the details of the:
Related Issue(s)
Component(s) name
arista.avd.cv_deployProposed changes
cv_deploy_results.workspace.build_idan ID of the latest build attempt. Needed as WS in Pending state maybe reused and may have multiple Build attempts (with each attempt producing different set of errors)cv_deploy_results.workspace.build_results:portfaston switchport)How to test
cv_deploymolecule test feeding in proper Access Token for CI CVaaS tenantcv_deploy_resultsoutputs (cv_deploy_results.workspace.build_idandcv_deploy_results.workspace.build_results) of theinvalid_configstep. Compare with other steps (which don't have incorrect DC/image and therefore don't trigger WS Build failures)Example with default warning suppression settings (no suppression of warnings enabled):
Checklist
User Checklist
Repository Checklist