Closed JMLX42 closed 3 years ago
@JMLX42 I found this guide: Want to trigger linting to your Ansible deployment on every Pull Request? explaining how to setup the ansible-lint GitHub Action.
Is this what we want?
If so I can take care of this issue and setup ansible lint
Let me know if I can start working on this and if you agree with the bullet points above.
Let me know if I can start working on this and if you agree with the bullet points above.
I do.
Does that mean that PR get automatically rejected if ansible-lint fails in the CI? I guess that's the whole point of the CI/Github actions... that's what we want for sure.
Does that mean that PR get automatically rejected if ansible-lint fails in the CI? I guess that's the whole point of the CI/Github actions... that's what we want for sure.
I think so and with the pre-commit hook the commit itself will be rejected.
ansible-lint went through a lot of changes since the latest release (v4.1.0.post0 from Mar 13, 2019) so I suggest we use the current master and fix the version to the latest commit SHA until the next release is out.
I suggest we use the current master and fix the version to the latest commit SHA until the next release is out.
Agreed.
Currently the ansible files of the project do not pass the linting.
Ansible-lint can be executed locally with the following command:
shopt -s globstar
ansible-lint -v --force-color **/*.yml
Which produces the following output:
@JMLX42 If that is OK with you I will fix thoses linting errors in a second PR before continuing the setup of the GitHub action.
@JMLX42 How would you prefer long lines to be split?
test: "{{ test \
| test \
| test }}"
test: >-
{{ test
| test
| test
}}
test: >-
{{
test
| test
| test
}}
Personnally I prefer 3.
Edit: actually the backslashes are not needed in 1. 1 is fine as well for small blocks.
If that is OK with you I will fix thoses linting errors in a second PR
@jerome-caucat Make it so.
How would you prefer long lines to be split?
If we're talking about shell
tasks then yes, number 3 is best I guess.
@JMLX42 I'm going to have to modify the long lines in provisioning/roles/laprimaire.monitoring/defaults/main/prometheus.yml but I don't know how to test my modifications to make sure I don't break anything.
I thought it could be connected to https://monitoring.infra.laprimaire.org.test/alerting/list but I have no alert rules.
How could I test this file?
@jerome-caucat this file was loosely inspired from https://github.com/cloudalchemy/ansible-prometheus/blob/master/defaults/main.yml
Here is the .ansible-lint
the corresponding project uses: https://github.com/cloudalchemy/ansible-prometheus/blob/master/.ansible-lint
@JMLX42 We can indeed ignore the linting error [204] Lines should be no longer than 160 chars
, this would greatly reduce the number of lines to change and the risk of breaking things.
Normally I would prefer to be strict on the linting, but I guess long lines are not such a big deal.
So I will use the following .ansible-lint
and fix the remaining errors:
---
skip_list:
- '204'
Let me know I you would actually prefer not to ignore long lines.