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

Fix!: Change uppercase CVP role vars to lower case #2504

Merged

Conversation

ClausHolbechArista
Copy link
Contributor

Change Summary

Change uppercase CVP role vars to lower case

Potentially breaking in rare cases where custom logic relies on the registered vars or content of files.

Component(s) name

Roles:
arista.avd.eos_config_deploy_cvp
arista.avd.cvp_configlet_upload

Modules:
arista.avd.inventory_to_container
arista.avd.configlet_build_config

Proposed changes

Change registered vars in above roles/modules from uppercase to lowercase, to conform with Ansible linting requirements.

CVP_CONFIGLETS -> cvp_configlets
CVP_CONFIGLETS_STATUS -> cvp_configlets_status
CVP_CONTAINERS -> cvp_containers
CVP_CONTAINER_RESULTS -> cvp_container_results
CVP_DEVICES -> cvp_devices
CVP_DEVICES_RESULTS -> cvp_devices_results
CVP_FACTS -> cvp_facts
CVP_TOPOLOGY -> cvp_topology
CVP_VARS -> cvp_vars

How to test

All existing integration tests and molecule have been updated.

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)

@ClausHolbechArista ClausHolbechArista requested a review from a team as a code owner February 8, 2023 19:30
@github-actions github-actions bot added module: configlet_build_config issue related to configlet_build_config module module: inventory_to_container issue related to inventory_to_container module role: cvp_configlet_upload issue related to cvp_configlet_upload role role: eos_config_deploy_cvp issue related to eos_config_deploy_cvp role state: CI Updated CI scenario have been updated in the PR type: documentation Improvements or additions to documentation labels Feb 8, 2023
@ClausHolbechArista ClausHolbechArista added this to the v4.0.0-dev3 milestone Feb 8, 2023
Copy link
Contributor

@gmuloc gmuloc left a comment

Choose a reason for hiding this comment

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

LGTM - only suggestion for the release notes - though maybe it should be in the porting guide?

ansible_collections/arista/avd/docs/release-notes/4.x.x.md Outdated Show resolved Hide resolved
Comment on lines 31 to 33
Modules:
arista.avd.inventory_to_container
arista.avd.configlet_build_config
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Modules:
arista.avd.inventory_to_container
arista.avd.configlet_build_config
Modules:
* arista.avd.inventory_to_container
* arista.avd.configlet_build_config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented the fixes. I don't think we should have this in the porting guide, since we have no way of knowing how the vars are consumed by custom logic.

ClausHolbechArista and others added 2 commits February 9, 2023 18:54
Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
@ClausHolbechArista ClausHolbechArista requested a review from a team February 9, 2023 20:34
Copy link
Member

@carlbuchmann carlbuchmann left a comment

Choose a reason for hiding this comment

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

LGTM!

@carlbuchmann carlbuchmann merged commit d60317f into aristanetworks:devel Feb 10, 2023
carlbuchmann pushed a commit to carlbuchmann/avd that referenced this pull request Feb 16, 2023
Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: configlet_build_config issue related to configlet_build_config module module: inventory_to_container issue related to inventory_to_container module rn: Fix! role: cvp_configlet_upload issue related to cvp_configlet_upload role role: eos_config_deploy_cvp issue related to eos_config_deploy_cvp role state: CI Updated CI scenario have been updated in the PR type: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants