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

Add ansible-lint CI #1368

Open
wants to merge 19 commits into
base: stackhpc/2024.1
Choose a base branch
from
Open

Add ansible-lint CI #1368

wants to merge 19 commits into from

Conversation

Alex-Welsh
Copy link
Contributor

@Alex-Welsh Alex-Welsh commented Nov 6, 2024 •
edited
Loading

Adds an ansible-lint CI job.

The job itself is quite simple, however there are a lot of changes to get to pass. Many were fixed automatically by the linter (e.g. formatting). Others were fixed by hand (e.g. Task names). Some rules are skipped because they are not relevant

@Alex-Welsh Alex-Welsh force-pushed the 2024.1-ansible-lint-alex branch 6 times, most recently from 6679b59 to 05ff525 Compare November 7, 2024 12:01
@Alex-Welsh
Add ansible-lint CI
0be7a56 
This commit includes a properly configured ansible-lint CI job and a
large amount of changes to the existing playbooks so that the new CI
passes. Some fixes were applied automatically with the --fix argument,
many were made manually. There is some risk that the changes have
altered the behaviour of the playbooks.
@Alex-Welsh Alex-Welsh force-pushed the 2024.1-ansible-lint-alex branch from 05ff525 to 0be7a56 Compare November 7, 2024 12:03
@Alex-Welsh Alex-Welsh marked this pull request as ready for review November 7, 2024 12:05
@Alex-Welsh Alex-Welsh requested a review from a team as a code owner November 7, 2024 12:05
@Alex-Welsh Alex-Welsh changed the title WIP: Add ansible-lint CI Add ansible-lint CI Nov 7, 2024
@Alex-Welsh Alex-Welsh mentioned this pull request Nov 7, 2024
Closed
@Alex-Welsh Alex-Welsh force-pushed the 2024.1-ansible-lint-alex branch from 61edbbc to 321acad Compare November 8, 2024 09:15
@Alex-Welsh Alex-Welsh mentioned this pull request Nov 12, 2024
Merged
MoteHue
MoteHue previously requested changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@MoteHue MoteHue left a comment

Choose a reason for hiding this comment

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

Looking good! Got a bunch of nitpicks, but linting is nitpicky anyway ;)

.ansible-lint-ignore Outdated Show resolved Hide resolved
etc/kayobe/ansible/cephadm-commands-post.yml Outdated Show resolved Hide resolved
etc/kayobe/ansible/cephadm-commands-pre.yml Outdated Show resolved Hide resolved
etc/kayobe/ansible/cephadm.yml Outdated Show resolved Hide resolved
etc/kayobe/ansible/fix-houston.yml Show resolved Hide resolved
etc/kayobe/ansible/stackhpc-openstack-tests.yml Outdated Show resolved Hide resolved
etc/kayobe/ansible/vault-deploy-overcloud.yml Outdated Show resolved Hide resolved
etc/kayobe/ansible/vault-deploy-overcloud.yml Outdated Show resolved Hide resolved
etc/kayobe/ansible/wazuh-manager.yml Show resolved Hide resolved
etc/kayobe/ansible/wazuh-manager.yml Outdated Show resolved Hide resolved
@Alex-Welsh Alex-Welsh added stackhpc-ci Automated action performed by stackhpc-ci Caracal Targets the Caracal OpenStack release labels Nov 15, 2024
@Alex-Welsh Alex-Welsh force-pushed the 2024.1-ansible-lint-alex branch from ff3fd29 to c81275f Compare December 2, 2024 10:52
seunghun1ee
seunghun1ee previously approved these changes Dec 3, 2024
View reviewed changes
Copy link
Member

@seunghun1ee seunghun1ee left a comment

Choose a reason for hiding this comment

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

If we're choosing not to worry about ansible-lint warnings, all looks good to me.

etc/kayobe/ansible/wazuh-manager.yml Show resolved Hide resolved
@Alex-Welsh Alex-Welsh added workflows Workflow files have been modified Caracal Targets the Caracal OpenStack release and removed stackhpc-ci Automated action performed by stackhpc-ci Caracal Targets the Caracal OpenStack release labels Dec 5, 2024
@Alex-Welsh Alex-Welsh force-pushed the 2024.1-ansible-lint-alex branch from 213d3c1 to 8c0039d Compare December 9, 2024 11:34
@Alex-Welsh Alex-Welsh requested a review from priteau December 16, 2024 11:39
priteau
priteau requested changes Dec 20, 2024
View reviewed changes
Copy link
Member

@priteau priteau left a comment

Choose a reason for hiding this comment

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

There is one change that is worth double checking (see inline comment). Otherwise the changes look correct, although I am not too keen on some of the style decisions made by this linter.

etc/kayobe/ansible/pulp-artifact-upload.yml
path: /tmp/updated_artifacts.txt
line: "{{ remote_pulp_url }}/pulp/content/{{ pulp_base_path }}/\
latest/{{ found_files.files[0].path | basename }}"
line: "{{ remote_pulp_url }}/pulp/content/{{ pulp_base_path }}/latest/{{ found_files.files[0].path | basename }}"
when: latest_distribution_details.changed
Copy link
Member

Choose a reason for hiding this comment

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

There is a mix of when and listen in the handlers section now. Is this correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@priteau priteau priteau requested changes

@seunghun1ee seunghun1ee seunghun1ee approved these changes

@jovial jovial Awaiting requested review from jovial

@MoteHue MoteHue Awaiting requested review from MoteHue

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
Caracal Targets the Caracal OpenStack release size: l workflows Workflow files have been modified
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Alex-Welsh @priteau @jovial @MoteHue @seunghun1ee @markgoddard