Skip to content

Move ansible lint config to standard config file and fix few warnings#273

Draft
mpagot wants to merge 1 commit intoSUSE:mainfrom
mpagot:ansible_warnings
Draft

Move ansible lint config to standard config file and fix few warnings#273
mpagot wants to merge 1 commit intoSUSE:mainfrom
mpagot:ansible_warnings

Conversation

@mpagot
Copy link
Collaborator

@mpagot mpagot commented Sep 12, 2024

Move them from the Makefile to the specific lint config file. In this way they are applied also when not running the lint ad make target, but directly calling ansible-lint.

Ticket: https://jira.suse.com/browse/TEAM-7184

Verification

Azure

sle-15-SP6-Qesap-Azure-Byos-x86_64-BuildLATEST_AZURE_SLE15_6_BYOS-qesap_azure_saptune_test

sle-15-SP6-HanaSr-Azure-Byos-x86_64-Build15-SP6_2025-06-24T02:03:18Z-hanasr_azure_test_sapconf_sbd az_Standard_E4s_v3

GCP

sle-15-SP5-Qesap-Gcp-Byos-x86_64-BuildLATEST_GCE_SLE15_5_BYOS-qesap_gcp_sbd_test

sle-15-SP6-Qesap-Gcp-Payg-x86_64-BuildLATEST_GCE_SLE15_6_PAYG-qesap_gcp_angi_test

AWS

sle-15-SP6-Qesap-Aws-Byos-x86_64-BuildLATEST_AWS_SLE15_6_BYOS-qesap_aws_angi_test

sle-15-SP6-Qesap-Aws-Byos-x86_64-BuildLATEST_AWS_SLE15_6_BYOS-qesap_aws_ibsmirror_peering_test

Copy link
Collaborator

@jankohoutek jankohoutek left a comment

Choose a reason for hiding this comment

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

LGTM

@mpagot mpagot force-pushed the ansible_warnings branch 5 times, most recently from 4714506 to 2d988ec Compare June 25, 2025 21:28
- ansible/playbooks/registration_role.yaml
- ansible/playbooks/vars/hana_media.yaml
- ansible/playbooks/vars/hana_vars.yaml
warn_list:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I decided to ignore this one for the moment. Problem is qe-sap-deployment has a role folder in ansible/role and maybe it should be in ansible/playbook/role. As soon as we not fix this folder structure, we cannot use named roles and we need to use path.

rules:
line-length: disable No newline at end of file
line-length: disable
comments:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There was an error message running ansible-lint listing them. ansible-lint also internally run yamllint, but Ansible likes its rules. The ansible-lint warning was suggesting to remove this file or tune it to match the Ansible rules


- name: Download PTF files recursively with wget
command: "wget --no-directories --recursive --reject 'index.html*' --user={{ ptf_user }} --password={{ ptf_password }} --no-parent {{ ptf_url }}"
- name: Download PTF files recursively with wget # noqa: command-instead-of-module
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

here using wget was intentional. # noqa: is the way to add local exeptions

- name: Execute hana system replication role
ansible.builtin.include_role:
role: ../roles/sap_ha_install_hana_hsr
name: ../roles/sap_ha_install_hana_hsr
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The change in the .ansiblelint demote this from fatal error to warning. The github workflow does not fails but the message is there and github annotate the file. It is good as we will not forget about it.

Copy link
Collaborator

@BillAnastasiadis BillAnastasiadis left a comment

Choose a reason for hiding this comment

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

LGTM

@mpagot mpagot marked this pull request as draft June 26, 2025 12:00
@mpagot mpagot force-pushed the ansible_warnings branch 3 times, most recently from 211f080 to be6cf07 Compare June 30, 2025 12:04
Copy link
Collaborator

@alvarocarvajald alvarocarvajald left a comment

Choose a reason for hiding this comment

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

LGTM

Remove no more used playbook, fix ansible-lint version used in the
github workflow, tune the yamllint to be compliant with Ansible
(remove warning about it reported by ansible-lint),
add global exception about role-name[path] as it require project folder
structure change, and last but not least fix some warnings.
@mpagot mpagot force-pushed the ansible_warnings branch from be6cf07 to 3cdcd03 Compare July 1, 2025 16:16
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.

4 participants