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

[Daniel Gomez <da.gomez@kernel.org>]

On 19/05/2025 15.11, Chuck Lever wrote:
> 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.

Cool.

> 
> 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.
> 
> 

My understanding is that import_tasks allows you to not having to use 
tags inside the import_tasks YAML file. For example:

- name: Import tasks
   import_tasks: tasks/deps/debian/main.yml
   tags: [ 'tag1', 'tag2', 'tag3']

Tags are evaluated at the "Import tasks" task. We don't need to specify 
them in debian/main.yml. So, in tasks/deps/debian/main.yml, we can write:

- name: Task1
   debug:
     msg: "Task1"

- name: Task2
   debug:
     msg: "Task2"

- name: Task3
   debug:
     msg: "Task3"

However, if we want debian/main.yml to run task1 with tag1, and so on. 
We need the include_task module. And write the tags at both levels. So, 
for simple sub-roles we can stick with import_tasks.

For this case, I'd stick with import_tasks module and remove any tag 
inside. And apply the necessary tags at the import_task task level.

I hope I'm not missing anything. I can't remember if there was a special 
case where tags were described at task level but no tags were supplied 
with --tags.