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

added pre-commit configuration for ansible-lint #93

Merged
merged 11 commits into from
Sep 25, 2023

Conversation

jce-redhat
Copy link
Collaborator

  • updated .ansible-lint
  • added .yamllint
  • added missing collection used by linux/temp_sudo.yml

no lint errors were fixed in this commit, only the scaffolding for using pre-commit was put in place.

@jce-redhat
Copy link
Collaborator Author

also, the github action ansible/[email protected] has not been changed, although it will be reading the new changes to .ansible-lint

@jce-redhat jce-redhat force-pushed the pre-commit branch 3 times, most recently from 4f03581 to b6f5daa Compare September 18, 2023 21:45
@l3acon
Copy link
Collaborator

l3acon commented Sep 19, 2023

So this is looking awesome to me.

I had some trouble around requirements and collections being found, pre-commit seemed to want them in ~/.cache/ansible-compat/ac09d1/collections/ansible_collections/ so I am curious if there is a better way. Perhaps we could provide some guidelines for installing collection dependencies?

@jce-redhat
Copy link
Collaborator Author

jce-redhat commented Sep 19, 2023

I had some trouble around requirements and collections being found, pre-commit seemed to want them in ~/.cache/ansible-compat/ac09d1/collections/ansible_collections/ so I am curious if there is a better way. Perhaps we could provide some guidelines for installing collection dependencies?

if any collections referenced aren't installed in the collections path, ansible-lint will try to install whatever is in collections/requirements.yml into the ansible-lint cache itself. since this can take some time, i disabled that in the .ansible-lint config file by setting offline: true. since i already had things in my local cache, it didn't affect me when i initially pushed this up. i'll switch that to false and update the PR, that should make local development easier.

note that setting offline: false means that for local development, the environment variable ANSIBLE_GALAXY_SERVER_AH_TOKEN must contain a valid token for pulling content from automation hub, since some of the collections come from there. this will probably be required for the github action that runs ansible-lint as well. i don't have permissions on the repo to check what secrets are set, so if anything needs to change there i may need @willtome to help.

@jce-redhat
Copy link
Collaborator Author

just tested with offline: false in .ansible-lint. twice in a row ansible-lint failed to install the collections with the following transient error:

ERROR! Failed to resolve the requested dependencies map. Could not satisfy the following requirements:
* ansible.controller:4.4.0 (direct request)

the third time it succeeded without any changes. this might be why i switched to offline: true in the first place but i can't remember.

so, the tradeoffs for using offline mode or not:

  • with offline: true, devs will be responsible for populating the local ansible-lint collection cache themselves.
  • with offline: false, the ansible-lint pre-commit hook should install collections in the local cache, and also in the github action runtime. the ANSIBLE_GALAXY_SERVER_AH_TOKEN environment variable will need to be populated with a valid token in both cases. if there are transient issues during github action execution, the github build action will need to be re-run manually. same for a local commit, although if the cache is already populated i don't think this will occur often.

i'm leaning towards offline: false if everyone else is good with it.

- updated .ansible-lint
- added .yamllint
- added missing collection used by linux/temp_sudo.yml

no lint errors were fixed in this commit, only the scaffolding
for using pre-commit was put in place.
- ansible-lint-action action is deprecated, switching to ansible-lint
  action at https://github.com/marketplace/actions/run-ansible-lint
- pinned ansible-lint version to match the same one in
  .pre-commit-config.yaml
- named build action
- removed unneeded call to actions/checkout@v3
- updated ansible.cfg galaxy server configuration per current
  documentation
@willtome willtome merged commit 2ee334f into ansible:main Sep 25, 2023
1 check passed
@jce-redhat jce-redhat deleted the pre-commit branch August 20, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants