From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EB97326A1BD for ; Mon, 19 May 2025 13:51:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747662688; cv=none; b=bEA3DbdzjHoRUN6vj9H8jSsQDUoUlEcAnLgnbe7KnuPj1DGuoF2PIXYJZTJItEShBh7sPoIWeRhgJYCT0DLF91ZC7n5tb2MdHvr61pP83EVF0+3/3mVl5B66qE9mAY2yRE0mz1EXBkLOYaSbhGT0IWIg1kCCu/UJupnWhSSr1HY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747662688; c=relaxed/simple; bh=s36C5w6gZmB5LM+LDfKvOdcuXoFgz5zGJd1PuU8HZCI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=IRPAcsQIEm/d1IuQ2W6ko60BcXO14VdJZTdd/E9pvTUkz293cS1ACxKemVXIqE1/OAdFfGJDTTE8eldXxXv6yv8PKhJ6A1iFGlCFcrEHH11lrkPFoJoVG3eJEqzg1cfGWUCohOJA0fyJNOSWfBcLzozJ59QqcuSnQzDv+4EsTk0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=B303HGmR; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="B303HGmR" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C1CADC4CEE9; Mon, 19 May 2025 13:51:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1747662687; bh=s36C5w6gZmB5LM+LDfKvOdcuXoFgz5zGJd1PuU8HZCI=; h=Date:Reply-To:Subject:To:Cc:References:From:In-Reply-To:From; b=B303HGmRq11A2q9sqt9Vlm8X/jOGxz+ObFuy6SDIMRKvlIUk+WW8tyi7fIFxJGh8Z I8xYGjcfTR2YmUFMQsnisuXms5tHOnWE0Xmuh+0/hsLhUIlqB76VTQ5kGcVmSjbzjn vz0c0jYu4VbtAz3tkmREIpjKyIN8aIF1n0NERxtQjLA0DYQWRtt8Q6/PTsde7iugof 4qC3O2v15UCd/7riEuQ6pMkmB43BBfP8QLRqNKrS4Qi5Owqk3+dFzECcR7xGbRZFxk uUgZLdRWTE5rN3XN0ZqEX97oCZuahhquTivSnyw9AfDR8gSc6Hn1bXL5o5FbthqMWn sNkWxdsAq0SJA== Message-ID: <7601ab2b-d541-4b58-990b-4942a07c6437@kernel.org> Date: Mon, 19 May 2025 15:51:25 +0200 Precedence: bulk X-Mailing-List: kdevops@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Reply-To: da.gomez@kernel.org Subject: Re: [PATCH v1 1/2] bootlinux: Modernize install-deps/main.yml To: Chuck Lever , kdevops@lists.linux.dev Cc: Chuck Lever References: <20250518141838.176772-1-cel@kernel.org> <23720f8e-6f0f-4107-a6d5-c7cc2b9a1bdc@kernel.org> <8685ab2b-7169-4818-afb5-e7eb6cc11beb@kernel.org> Content-Language: en-US From: Daniel Gomez Organization: kernel.org In-Reply-To: <8685ab2b-7169-4818-afb5-e7eb6cc11beb@kernel.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 >>> >>> 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 >>> --- >>>   .../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.