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.

Sign up for GitHub

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

Jump to bottom

Refactor(cv_container_v3): Adding image bundle dictionary to the output. #627

Merged
merged 9 commits into from
Jul 13, 2023
Merged

Refactor(cv_container_v3): Adding image bundle dictionary to the output. #627

merged 9 commits into from
Jul 13, 2023

Conversation

Vibhu-gslab
Copy link
Collaborator

Change Summary

Adding integration tests for cv_conatiner_v3

Related Issue(s)

Fixes #

Component(s) name

arista.cvp.cv_conatiner_v3

Proposed changes

How to test

molecule converge -s cv_container_v3

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. (check the box if not applicable)
  • I have updated molecule CI testing accordingly. (check the box if not applicable)

sugetha24
sugetha24 reviewed Jun 14, 2023
View reviewed changes
ansible_collections/arista/cvp/molecule/cv_container_v3/converge.yml Outdated
@@ -0,0 +1,42 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we follow the same format as the other molecule tests?
move these tests into a separate playbook and import it in converge.yml?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

sugetha24
sugetha24 reviewed Jun 14, 2023
View reviewed changes
ansible_collections/arista/cvp/molecule/cv_container_v3/converge.yml Outdated
parentContainerName: Tenant

tasks:
- name: "Build Container topology on {{inventory_hostname}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add tests for the following?

  1. Create a 3 tier topology
  2. Delete the above topology
  3. Create a 3 tier topology and attach configlet to one of the containers and remove it
  4. Create a 3 tier topology and attach image bundles to one of the containers and remove it
  5. Play around with apply_mode: loose and apply_mode: strict.
    Refer to cv_container_v3 for more info on how apply_mode works.
    NOTE: apply_mode works for both configlets and imageBundle, so we need to test both

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

@github-actions github-actions bot requested a review from sugetha24 June 15, 2023 18:33
@Vibhu-gslab Vibhu-gslab marked this pull request as draft June 16, 2023 10:34
@github-actions github-actions bot added module: cv_container_v3 Issue related to cv_container module in v3 module_utils: cv_client cv_client implementation issue labels Jun 16, 2023
@Vibhu-gslab Vibhu-gslab marked this pull request as ready for review June 16, 2023 12:55
@Vibhu-gslab Vibhu-gslab marked this pull request as draft June 19, 2023 06:28
@Vibhu-gslab Vibhu-gslab marked this pull request as ready for review June 20, 2023 07:02
@Vibhu-gslab Vibhu-gslab changed the title Test: Adding integration tests for cv_conatiner_v3 Refactor(cv_container_v3): Adding image bundle dictionary to the output. Jun 20, 2023
sugetha24
sugetha24 reviewed Jun 20, 2023
View reviewed changes
ansible_collections/arista/cvp/molecule/cv_container_v3/test_cv_container_v3_configlet.yaml Outdated
TEAM01_LEAFS:
parentContainerName: TEAM01_DC
configlets:
- ATD-INFRA
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use our own configlet instead? I don't want to rely on ATD configlets

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we can use any configlet, whichever is available on your lab.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

sugetha24
sugetha24 reviewed Jun 20, 2023
View reviewed changes
ansible_collections/arista/cvp/molecule/cv_container_v3/test_cv_container_v3_configlet.yaml
# TC2 - Create a 3 tier topology and attach configlet to one of the containers and remove it.
# Testing:
# 1. Adding configlet
# 2. Change in configlet with apply_mode: loose
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a test to change configlet with apply_mode: strict as well? This should match the configlets on CVP with what's supplied in configlet keyword making adds and removes as necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added!

@github-actions github-actions bot requested a review from sugetha24 June 21, 2023 07:32
sugetha24
sugetha24 reviewed Jun 22, 2023
View reviewed changes
ansible_collections/arista/cvp/molecule/cv_container_v3/test_cv_container_v3_configlet.yaml
topology: '{{TOPOLOGY_3_TIER}}'
register: CONFIGLET_ADD

- name: Testing Added Configlet
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get configlet facts from CVP and check if it's applied on the container we want?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added!

@github-actions github-actions bot requested a review from sugetha24 June 23, 2023 07:05
@sugetha24 sugetha24 mentioned this pull request Jun 26, 2023
Closed
9 tasks
noredistribution
noredistribution approved these changes Jul 1, 2023
View reviewed changes
Copy link
Contributor

@noredistribution noredistribution left a comment

Choose a reason for hiding this comment

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

LGTM! worked nicely on AtD!

@github-actions github-actions bot added the state: conflict PR has conflict to resolve label Jul 5, 2023
@github-actions
Copy link

github-actions bot commented Jul 5, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added type: Code quality Issue related to code quality in repository type: dependencies Issue related to repository content type: documentation Improvements or additions to documentation labels Jul 5, 2023
@github-actions
Copy link

github-actions bot commented Jul 5, 2023

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the state: conflict PR has conflict to resolve label Jul 5, 2023
@sugetha24 sugetha24 merged commit e1842c0 into aristanetworks:devel Jul 13, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guillaumeVilar guillaumeVilar Awaiting requested review from guillaumeVilar

@sugetha24 sugetha24 Awaiting requested review from sugetha24

@noredistribution noredistribution Awaiting requested review from noredistribution

Assignees

@Vibhu-gslab Vibhu-gslab

Labels
module: cv_container_v3 Issue related to cv_container module in v3 module_utils: cv_client cv_client implementation issue type: Code quality Issue related to code quality in repository type: dependencies Issue related to repository content type: documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Vibhu-gslab @sugetha24 @noredistribution