| Message ID | 20250128173233.1084701-1-cel@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] devconfig: install-deps is skipped in some configurations | expand |
On Tue, Jan 28, 2025 at 12:32:33PM -0500, cel@kernel.org wrote: > From: Chuck Lever <chuck.lever@oracle.com> > > A recent commit replaced import_tasks with include_tasks in a few > steps in the devconfig playbook. > > The Ansible "import_tasks" documentation says: > > > Tags are not interpreted for this action, they are applied to the > > imported tasks They seem to not recommend mixing tags with imports and includes [1] due to tag inheritance differences between them. [1] https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_tags.html#tags If I'm reading correctly, include_tasks require the tag to be present at both levels, the include_tasks 'task' and at every task we want to be executed. This is not needed when import_tasks is used as all imported 'tasks' inherit the top level tag and tag annotation is not required. We tend to be explicit with tags so, we probably want to stick with include_tasks, right? > > Revert some of the import_tasks conversions to ensure those steps > are run according to the command-line tag settings. > > For the original problem that I tripped on, add tags to ensure those > sub-steps get run as needed. > > Reported-by: Joel Granados <joel.granados@kernel.org> > Closes: https://lore.kernel.org/kdevops/20250128112453.rawxqvpffxwupvco@AALNPWDAGOMEZ1.aal.scsc.local/T/#t > Fixes: e1a41cd9d4ee ("devconfig: Replace import_tasks with include_tasks") This is the wrong commit hash. It points to a forked repository. Should be ec9a34e (ec9a34eb6b8eb150822b264747ebd053dc1d8663) instead. > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > playbooks/roles/devconfig/tasks/main.yml | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/playbooks/roles/devconfig/tasks/main.yml b/playbooks/roles/devconfig/tasks/main.yml > index 51b890375215..84c5fa1f1cf8 100644 > --- a/playbooks/roles/devconfig/tasks/main.yml > +++ b/playbooks/roles/devconfig/tasks/main.yml > @@ -32,9 +32,10 @@ > # Distro specific > - name: Install dependencies > ansible.builtin.include_tasks: install-deps/main.yml > + tags: ['vars', 'vars_simple'] Keeping include_tasks enforces us to explicitly tag this include_tasks task with the right tag, but also every task in install-deps. journal-server target is executed with vars_simple,journal tags, so the include will now be evaluated true. Then, install/deps/main.yml has a debian setup task to include_tasks in debian/main.yml and properly using vars_simple tag. Then, debian/main.yml will only execute tasks properly tagged with vars_simple or journal. So, "Install systemd-journal-remote" is run and rest of the tasks are skipped. > > - name: Configure custom repositories and install packages > - ansible.builtin.include_tasks: config-custom-repos-and-packages/main.yml > + ansible.builtin.import_tasks: config-custom-repos-and-packages/main.yml > when: > - ansible_facts['os_family']|lower == 'redhat' If my reading above is correct, this import will be skipped when tags are applied to the ansible-playbook command. At this moment, only devconfig, sysctl-tunings targets will trigger this path. > > @@ -447,7 +448,7 @@ > tags: [ 'console' ] > > - name: Update your boot GRUB file if necessary > - ansible.builtin.include_tasks: update-grub/main.yml > + ansible.builtin.import_tasks: update-grub/main.yml > when: > - grub2_config_file.stat.exists > - devconfig_enable_console|bool Same here, only triggered with devconfig and sysctl-tunings. > @@ -637,7 +638,7 @@ > tags: [ 'sysctl' ] > > - name: Rev the kernel to the latest distribution kotd > - ansible.builtin.include_tasks: kotd-rev-kernel/main.yml > + ansible.builtin.import_tasks: kotd-rev-kernel/main.yml > when: > - devconfig_enable_kotd|bool > tags: [ 'kotd' ] This will only be executed for kotd-dev, kotd-baseline and kotd targets. > -- > 2.48.1 > I can't verify the other paths being reverted but they look good to me. Reviewed-by: Daniel Gomez <da.gomez@samsung.com> Also, the changes here fix the issue in kdevops CI (tested locally).
On 1/29/25 5:40 AM, Daniel Gomez wrote: > On Tue, Jan 28, 2025 at 12:32:33PM -0500, cel@kernel.org wrote: >> From: Chuck Lever <chuck.lever@oracle.com> >> >> A recent commit replaced import_tasks with include_tasks in a few >> steps in the devconfig playbook. >> >> The Ansible "import_tasks" documentation says: >> >>> Tags are not interpreted for this action, they are applied to the >>> imported tasks > > They seem to not recommend mixing tags with imports and includes [1] due > to tag inheritance differences between them. > > [1] https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_tags.html#tags > > If I'm reading correctly, include_tasks require the tag to be present > at both levels, the include_tasks 'task' and at every task we want to > be executed. > > This is not needed when import_tasks is used as all imported 'tasks' > inherit the top level tag and tag annotation is not required. > > We tend to be explicit with tags so, we probably want to stick with > include_tasks, right? > >> >> Revert some of the import_tasks conversions to ensure those steps >> are run according to the command-line tag settings. >> >> For the original problem that I tripped on, add tags to ensure those >> sub-steps get run as needed. >> >> Reported-by: Joel Granados <joel.granados@kernel.org> >> Closes: https://lore.kernel.org/kdevops/20250128112453.rawxqvpffxwupvco@AALNPWDAGOMEZ1.aal.scsc.local/T/#t >> Fixes: e1a41cd9d4ee ("devconfig: Replace import_tasks with include_tasks") > > This is the wrong commit hash. It points to a forked repository. > > Should be ec9a34e (ec9a34eb6b8eb150822b264747ebd053dc1d8663) instead. > >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >> --- >> playbooks/roles/devconfig/tasks/main.yml | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/playbooks/roles/devconfig/tasks/main.yml b/playbooks/roles/devconfig/tasks/main.yml >> index 51b890375215..84c5fa1f1cf8 100644 >> --- a/playbooks/roles/devconfig/tasks/main.yml >> +++ b/playbooks/roles/devconfig/tasks/main.yml >> @@ -32,9 +32,10 @@ >> # Distro specific >> - name: Install dependencies >> ansible.builtin.include_tasks: install-deps/main.yml >> + tags: ['vars', 'vars_simple'] > > Keeping include_tasks enforces us to explicitly tag this include_tasks > task with the right tag, but also every task in install-deps. > > journal-server target is executed with vars_simple,journal tags, so the > include will now be evaluated true. Then, install/deps/main.yml has a > debian setup task to include_tasks in debian/main.yml and properly using > vars_simple tag. Then, debian/main.yml will only execute tasks properly > tagged with vars_simple or journal. So, "Install systemd-journal-remote" > is run and rest of the tasks are skipped. > >> >> - name: Configure custom repositories and install packages >> - ansible.builtin.include_tasks: config-custom-repos-and-packages/main.yml >> + ansible.builtin.import_tasks: config-custom-repos-and-packages/main.yml >> when: >> - ansible_facts['os_family']|lower == 'redhat' > > If my reading above is correct, this import will be skipped when tags > are applied to the ansible-playbook command. At this moment, only > devconfig, sysctl-tunings targets will trigger this path. > >> >> @@ -447,7 +448,7 @@ >> tags: [ 'console' ] >> >> - name: Update your boot GRUB file if necessary >> - ansible.builtin.include_tasks: update-grub/main.yml >> + ansible.builtin.import_tasks: update-grub/main.yml >> when: >> - grub2_config_file.stat.exists >> - devconfig_enable_console|bool > > Same here, only triggered with devconfig and sysctl-tunings. > >> @@ -637,7 +638,7 @@ >> tags: [ 'sysctl' ] >> >> - name: Rev the kernel to the latest distribution kotd >> - ansible.builtin.include_tasks: kotd-rev-kernel/main.yml >> + ansible.builtin.import_tasks: kotd-rev-kernel/main.yml >> when: >> - devconfig_enable_kotd|bool >> tags: [ 'kotd' ] > > This will only be executed for kotd-dev, kotd-baseline and kotd targets. > >> -- >> 2.48.1 >> > > I can't verify the other paths being reverted but they look good to me. > > Reviewed-by: Daniel Gomez <da.gomez@samsung.com> > > Also, the changes here fix the issue in kdevops CI (tested locally). Thank you for confirming that. May I add Tested-by: Daniel ... as well? I will correct the Fixes: tag before committing.
diff --git a/playbooks/roles/devconfig/tasks/main.yml b/playbooks/roles/devconfig/tasks/main.yml index 51b890375215..84c5fa1f1cf8 100644 --- a/playbooks/roles/devconfig/tasks/main.yml +++ b/playbooks/roles/devconfig/tasks/main.yml @@ -32,9 +32,10 @@ # Distro specific - name: Install dependencies ansible.builtin.include_tasks: install-deps/main.yml + tags: ['vars', 'vars_simple'] - name: Configure custom repositories and install packages - ansible.builtin.include_tasks: config-custom-repos-and-packages/main.yml + ansible.builtin.import_tasks: config-custom-repos-and-packages/main.yml when: - ansible_facts['os_family']|lower == 'redhat' @@ -447,7 +448,7 @@ tags: [ 'console' ] - name: Update your boot GRUB file if necessary - ansible.builtin.include_tasks: update-grub/main.yml + ansible.builtin.import_tasks: update-grub/main.yml when: - grub2_config_file.stat.exists - devconfig_enable_console|bool @@ -637,7 +638,7 @@ tags: [ 'sysctl' ] - name: Rev the kernel to the latest distribution kotd - ansible.builtin.include_tasks: kotd-rev-kernel/main.yml + ansible.builtin.import_tasks: kotd-rev-kernel/main.yml when: - devconfig_enable_kotd|bool tags: [ 'kotd' ]