Re: [PATCH v1 1/2] bootlinux: Modernize install-deps/main.yml

[Chuck Lever <cel@kernel.org>]

On 5/19/25 5:13 AM, Daniel Gomez wrote:
> On 18/05/2025 16.18, cel@kernel.org wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>>
>> This subrole is run using include_tasks, but install-deps/main.yml
>> then imports the distribution-specific steps. It's better to avoid
>> mixing import_tasks and include_tasks.
> 
> I agree.
> 
> Ansible explains tag inheritance recommendation here:
> https://docs.ansible.com/ansible/latest/playbook_guide/
> playbooks_tags.html#tag-inheritance-for-includes-blocks-and-the-apply-
> keyword
> 
> Can you add that to the commit message?

Yes.


>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>   .../bootlinux/tasks/install-deps/main.yml     | 21 ++++++++++++-------
>>   1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/playbooks/roles/bootlinux/tasks/install-deps/main.yml b/
>> playbooks/roles/bootlinux/tasks/install-deps/main.yml
>> index c4c16d20509c..2931f9fe06e3 100644
>> --- a/playbooks/roles/bootlinux/tasks/install-deps/main.yml
>> +++ b/playbooks/roles/bootlinux/tasks/install-deps/main.yml
>> @@ -1,8 +1,15 @@
>>   ---
>> -- name: oscheck distribution ospecific setup
>> -  import_tasks: debian/main.yml
>> -  when: ansible_facts['os_family']|lower == 'debian'
>> -- import_tasks: suse/main.yml
>> -  when: ansible_facts['os_family']|lower == 'suse'
>> -- import_tasks: redhat/main.yml
>> -  when: ansible_facts['os_family']|lower == 'redhat'
>> +- name: Debian-specific setup
>> +  ansible.builtin.include_tasks: debian/main.yml
>> +  when:
>> +    - ansible_os_family == "Debian"
> 
> In general, I think Ansible import is preferred for simplicity. The
> include_tasks module forces us to be explicit with the tags at
> 'include_*' task and at every included task level. Here, no tags are
> applied but debian/main.yml uses the linux tag. So if the playbook is
> run without specifying a tag (i.e. through linux Makefile target), I
> believe the file will be included but no tasks will be executed. Is that
> correct?
> 

The "linux" tag is not set by any invocation of this role. I have a
patch that removes the tag completely. I can include that patch when
I repost.

I went back and forth about whether to stick with import_tasks or
change to include_tasks, and I can't remember why I went with
include_tasks. So right at the moment, I'm not stuck on either one or
the other.


-- 
Chuck Lever