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

Introduce ansible-lint to format ansible files #2666

Merged
merged 1 commit into from
Mar 23, 2024

Conversation

idorax
Copy link
Contributor

@idorax idorax commented Feb 2, 2024

Pull Request Checklist

  • implement the feature

Fixes: #2664

@idorax idorax requested review from martinhoyer, thrix, happz and psss and removed request for happz, thrix, lukaszachy, psss and janhavlin February 2, 2024 15:03
@idorax idorax force-pushed the dev-huanli-2664-20240202-ansible-lint branch 2 times, most recently from 59ec0db to 31d6a90 Compare February 2, 2024 15:09
@idorax idorax changed the title Introduce ansible-lint to format Ansible files Introduce ansible-lint to format ansible files Feb 2, 2024
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@thrix
Copy link
Collaborator

thrix commented Feb 2, 2024

/packit test -i full

@idorax idorax force-pushed the dev-huanli-2664-20240202-ansible-lint branch from 6b57e8a to 843a4d4 Compare February 5, 2024 15:02
idorax added a commit that referenced this pull request Feb 5, 2024
This is proposed by Martin. For details, please refer to:
#2666 (comment)

Signed-off-by: Vector Li <huanli@redhat.com>
@idorax
Copy link
Contributor Author

idorax commented Feb 6, 2024

In patch eee8f619, file examples/redis/ansible/tasks/redis.yml is excluded because of a failure which doesn't comply with rule schema. e.g.

$ pre-commit run --all-files ansible-lint
Ansible-lint.............................................................Failed
- hook id: ansible-lint
- exit code: 2
...<snip>...
INFO     Executing syntax check on playbook tests/run/worktree/data/ansible/playbook.yml (0.61s)
WARNING  Ignored exception from VariableNamingRule.<bound method VariableNamingRule.matchyaml of var-naming: All variables should be named using only lowercase and underscores.> while processing examples/redis/ansible/setup_server.yml (playbook): list indices must be integers or slices, not str
WARNING  Listing 1 violation(s) that are fatal
�]8;id=251359;https://ansible.readthedocs.io/projects/lint/rules/schema/�\schema[playbook]�]8;;\: $[0].vars [{'version': '{{ ansible_distribution_version }}'}] is not of type 'object'
examples/redis/ansible/setup_server.yml:1  Returned errors will not include exact line numbers, but they will mention
the schema name being used as a tag, like ``schema[playbook]``,
``schema[tasks]``.

This rule is not skippable and stops further processing of the file.

If incorrect schema was picked, you might want to either:

* move the file to standard location, so its file is detected correctly.
* use ``kinds:`` option in linter config to help it pick correct file type.


Read �]8;id=797634;https://ansible.readthedocs.io/projects/lint/configuring/#ignoring-rules-for-entire-files�\documentation�]8;;�\ for instructions on how to ignore specific rule violations.

               Rule Violation Summary                
 count tag              profile rule associated tags 
     1 �]8;id=577568;https://ansible.readthedocs.io/projects/lint/rules/schema�\schema[playbook]�]8;;\ basic   core                 

Failed: 1 failure(s), 0 warning(s) on 22 files. Profile 'production' was required, but 'min' profile passed.

And I had a try to find possible solution to fix the failure, e.g. ansible-lint/issues/2230, but I failed. @martinhoyer, @thrix and @happz, if you have idea to fix the failure, please let me know, thanks!

@martinhoyer
Copy link
Collaborator

@idorax I suspect it's just a matter of removing the dash.

   vars:
-    - version: "{{ ansible_distribution_version }}"
+    version: "{{ ansible_distribution_version }}"

@idorax idorax force-pushed the dev-huanli-2664-20240202-ansible-lint branch from eee8f61 to e8a1d35 Compare February 6, 2024 11:43
@idorax
Copy link
Contributor Author

idorax commented Feb 6, 2024

@idorax I suspect it's just a matter of removing the dash.

   vars:
-    - version: "{{ ansible_distribution_version }}"
+    version: "{{ ansible_distribution_version }}"

Thanks, it works fine now.

@psss psss self-requested a review February 27, 2024 10:57
idorax added a commit that referenced this pull request Feb 29, 2024
This is proposed by Martin. For details, please refer to:
#2666 (comment)

Signed-off-by: Vector Li <huanli@redhat.com>
@idorax idorax force-pushed the dev-huanli-2664-20240202-ansible-lint branch from e89bf32 to 29e6fc5 Compare February 29, 2024 03:07
@thrix thrix requested review from thrix and martinhoyer March 19, 2024 10:30
@lukaszachy lukaszachy added this to the 1.32 milestone Mar 19, 2024
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@happz happz added the tmt tests Improvements or additions to test coverage of tmt itself label Mar 19, 2024
Copy link
Collaborator

@martinhoyer martinhoyer left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

.pre-commit-config.yaml Outdated Show resolved Hide resolved
idorax added a commit that referenced this pull request Mar 19, 2024
This is proposed by Martin. For details, please refer to:
#2666 (comment)

Signed-off-by: Vector Li <huanli@redhat.com>
@idorax idorax force-pushed the dev-huanli-2664-20240202-ansible-lint branch from 29e6fc5 to 01db3cb Compare March 19, 2024 10:41
@lukaszachy lukaszachy added the full test Pull request is ready for the full test execution label Mar 19, 2024
@lukaszachy
Copy link
Collaborator

Package manager "None" is not supported. --> trying to rerun

@idorax
Copy link
Contributor Author

idorax commented Mar 20, 2024

Package manager "None" is not supported. --> trying to rerun

Hi @lukaszachy, is there anything I can do?

BTW, the failure from testing-farm:fedora-39-x86_64:provision is because of /tests/test/check/avc. e.g.

...<snip>...
    execute
        queued execute task #1: default-0 on default-0
        
        execute task #1: default-0 on default-0
        how: tmt
        exit-first: false
            test: /avc/nasty
                cmd:
                    set -x
                    systemctl status auditd || /bin/true
                    sudo bash -c "passwd --help &> /root/passwd.log; \
                                  ls -alZ /root/passwd.log; \
                                  rm -f /root/passwd.log" || /bin/true
                00:00:00 pass /avc/nasty (on default-0) [1/2]
                00:00:00     fail avc (after-test check)
            test: /avc/harmless
                cmd: /bin/true

Maximum test time '5m' exceeded.
Adjust the test 'duration' attribute if necessary.
https://tmt.readthedocs.io/en/stable/spec/tests.html#duration

@martinhoyer martinhoyer mentioned this pull request Mar 21, 2024
1 task
idorax added a commit that referenced this pull request Mar 21, 2024
This is proposed by Martin. For details, please refer to:
#2666 (comment)

Signed-off-by: Vector Li <huanli@redhat.com>
@idorax idorax force-pushed the dev-huanli-2664-20240202-ansible-lint branch from 01db3cb to fd7a72b Compare March 21, 2024 09:52
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Looks good, I just run into one question/bug during the review. It was present before but I guess we could/should fix it while we're doing the changes here.

tmt/steps/prepare/feature/epel-disable.yaml Outdated Show resolved Hide resolved
@idorax idorax requested a review from psss March 22, 2024 14:20
Add ansible-lint to the pre-commit-config file, and fix
failures/warnings for ansible files as well.

Signed-off-by: Vector Li <huanli@redhat.com>
@psss psss force-pushed the dev-huanli-2664-20240202-ansible-lint branch from 63b1c04 to 4ad9ce6 Compare March 22, 2024 20:30
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Thanks, now looks good!

@psss psss merged commit 4ad9ce6 into main Mar 23, 2024
19 of 20 checks passed
@psss psss deleted the dev-huanli-2664-20240202-ansible-lint branch March 23, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full test Pull request is ready for the full test execution tmt tests Improvements or additions to test coverage of tmt itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce ansible-lint and prettier to format Ansible files
6 participants