| Message ID | 20250113-jag-bringup_fixes-v1-5-fb28030b1f26@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kdevops: Various fixes | expand |
On Mon, Jan 13, 2025 at 12:53:03PM +0100, Joel Granados wrote: > Add build-deps tag to enable-user and install-deps tasks in libvirt_user > role so they can be used from other roles. With no tags, these tasks are > skipped. > > Signed-off-by: Joel Granados <joel.granados@kernel.org> > --- > playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml | 6 +++++- > playbooks/roles/libvirt_user/tasks/enable-user/main.yml | 4 ++++ > playbooks/roles/libvirt_user/tasks/install-deps/main.yml | 7 +++++++ > scripts/guestfs.Makefile | 2 +- > 4 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > index 1ad7f17..5797bd6 100644 > --- a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > +++ b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > @@ -9,12 +9,14 @@ > when: > - not libvirt_session > - not only_verify_user|bool > + tags: [ 'build-deps' ] > > - name: Check if apparmor_status exists > stat: > path: /usr/sbin/apparmor_status > register: apparmor_file_stat_result > when: 'only_verify_user|bool' > + tags: [ 'build-deps' ] > > - name: Verify if AppArmor is disabled when applicable > become: yes > @@ -26,6 +28,7 @@ > when: > - 'only_verify_user|bool' > - 'apparmor_file_stat_result.stat.exists' > + tags: [ 'build-deps' ] > > - name: Verifies user's effective group allows to run libvirt/kvm without being root > shell: groups | grep {{ item }} > @@ -41,9 +44,10 @@ > when: > - not libvirt_session > - not only_verify_user|bool > + tags: [ 'build-deps' ] > > - name: Ensure our user is part of the libvirt/kvm groups > - tags: [ 'journal' ] > + tags: [ 'journal', 'build-deps' ] > become: yes > become_flags: 'su - -c' > become_method: sudo > diff --git a/playbooks/roles/libvirt_user/tasks/enable-user/main.yml b/playbooks/roles/libvirt_user/tasks/enable-user/main.yml > index 04f8e31..364d625 100644 > --- a/playbooks/roles/libvirt_user/tasks/enable-user/main.yml > +++ b/playbooks/roles/libvirt_user/tasks/enable-user/main.yml > @@ -2,19 +2,23 @@ > - name: Debian-specific set up > ansible.builtin.include_tasks: install-deps/debian/main.yml > when: ansible_os_family == 'Debian' > + tags: [ 'build-deps' ] Enabling the user while we do bringup will not succeed if we did not run this before, force the user to log out and log in again. Why do we need enable-user to be run with build-deps? > > - name: SuSE-specific set up > ansible.builtin.include_tasks: install-deps/suse/main.yml > when: ansible_os_family == 'Suse' > + tags: [ 'build-deps' ] > > - name: Red Hat-specific set up > ansible.builtin.include_tasks: install-deps/redhat/main.yml > when: > - ansible_os_family == 'RedHat' > - ansible_facts['distribution'] != "Fedora" > + tags: [ 'build-deps' ] > > - name: Fedora-specific set up > ansible.builtin.include_tasks: install-deps/fedora/main.yml > when: > - ansible_os_family == 'RedHat' > - ansible_facts['distribution'] == "Fedora" > + tags: [ 'build-deps' ] > diff --git a/playbooks/roles/libvirt_user/tasks/install-deps/main.yml b/playbooks/roles/libvirt_user/tasks/install-deps/main.yml > index c652ab2..c2f58c8 100644 > --- a/playbooks/roles/libvirt_user/tasks/install-deps/main.yml > +++ b/playbooks/roles/libvirt_user/tasks/install-deps/main.yml > @@ -11,11 +11,18 @@ > - name: Distribution specific setup > import_tasks: debian/main.yml > when: ansible_facts['os_family']|lower == 'debian' > + tags: [ 'build-deps' ] > + We've had a recent issue with tags import* and include* modules. The import_tasks + tags here will do tag inheritance. So, all the tasks under install-deps will run when build-deps tag is passed. On the other hand, I think this will conflict with libvirt setup (kdevops_install_libvirt target)? > - import_tasks: suse/main.yml > when: ansible_facts['os_family']|lower == 'suse' > + tags: [ 'build-deps' ] > + > - import_tasks: redhat/main.yml > when: > - ansible_facts['os_family']|lower == 'redhat' > - ansible_facts['distribution']|lower != "fedora" > + tags: [ 'build-deps' ] > + > - import_tasks: fedora/main.yml > when: ansible_facts['distribution']|lower == "fedora" > + tags: [ 'build-deps' ] > diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile > index 52397d5..dbfb372 100644 > --- a/scripts/guestfs.Makefile > +++ b/scripts/guestfs.Makefile > @@ -78,7 +78,7 @@ bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) > playbooks/bringup_guestfs.yml \ > -e 'ansible_python_interpreter=/usr/bin/python3' \ > --extra-vars=@./extra_vars.yaml \ > - --tags config-check,network > + --tags config-check,network,build-deps Since this is a build-deps, I suggest adding a new target to bringup_guestfs` > $(Q)$(TOPDIR)/scripts/bringup_guestfs.sh > $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ > --inventory localhost, \ > > -- > 2.44.2 > >
On Thu, Jan 30, 2025 at 10:15:14AM +0100, Daniel Gomez wrote: > On Mon, Jan 13, 2025 at 12:53:03PM +0100, Joel Granados wrote: > > Add build-deps tag to enable-user and install-deps tasks in libvirt_user > > role so they can be used from other roles. With no tags, these tasks are > > skipped. > > > > Signed-off-by: Joel Granados <joel.granados@kernel.org> > > --- > > playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml | 6 +++++- > > playbooks/roles/libvirt_user/tasks/enable-user/main.yml | 4 ++++ > > playbooks/roles/libvirt_user/tasks/install-deps/main.yml | 7 +++++++ > > scripts/guestfs.Makefile | 2 +- > > 4 files changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > > index 1ad7f17..5797bd6 100644 > > --- a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > > +++ b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > > @@ -9,12 +9,14 @@ > > when: > > - not libvirt_session > > - not only_verify_user|bool > > + tags: [ 'build-deps' ] > > > > - name: Check if apparmor_status exists > > stat: > > path: /usr/sbin/apparmor_status > > register: apparmor_file_stat_result > > when: 'only_verify_user|bool' > > + tags: [ 'build-deps' ] > > > > - name: Verify if AppArmor is disabled when applicable > > become: yes > > @@ -26,6 +28,7 @@ > > when: > > - 'only_verify_user|bool' > > - 'apparmor_file_stat_result.stat.exists' > > + tags: [ 'build-deps' ] > > > > - name: Verifies user's effective group allows to run libvirt/kvm without being root > > shell: groups | grep {{ item }} > > @@ -41,9 +44,10 @@ > > when: > > - not libvirt_session > > - not only_verify_user|bool > > + tags: [ 'build-deps' ] > > > > - name: Ensure our user is part of the libvirt/kvm groups > > - tags: [ 'journal' ] > > + tags: [ 'journal', 'build-deps' ] > > become: yes > > become_flags: 'su - -c' > > become_method: sudo > > diff --git a/playbooks/roles/libvirt_user/tasks/enable-user/main.yml b/playbooks/roles/libvirt_user/tasks/enable-user/main.yml > > index 04f8e31..364d625 100644 > > --- a/playbooks/roles/libvirt_user/tasks/enable-user/main.yml > > +++ b/playbooks/roles/libvirt_user/tasks/enable-user/main.yml > > @@ -2,19 +2,23 @@ > > - name: Debian-specific set up > > ansible.builtin.include_tasks: install-deps/debian/main.yml > > when: ansible_os_family == 'Debian' > > + tags: [ 'build-deps' ] > > Enabling the user while we do bringup will not succeed if we did not run > this before, force the user to log out and log in again. Why do we need > enable-user to be run with build-deps? In order to run it from other roles. If you do not have it taged, then it is skipped. We can use another tag, but it needs to be tagged, in order for it to be called from another role. Notice that I added build-deps tags in the bringup_guestfs call. It does not mean that all the build-deps are going to be run. It means that all the tage included in the --tag argument will be called and the rest ignored. its a way to run the subset of a role (unless I misunderstood). > > > > > - name: SuSE-specific set up > > ansible.builtin.include_tasks: install-deps/suse/main.yml > > when: ansible_os_family == 'Suse' > > + tags: [ 'build-deps' ] > > > > - name: Red Hat-specific set up > > ansible.builtin.include_tasks: install-deps/redhat/main.yml > > when: > > - ansible_os_family == 'RedHat' > > - ansible_facts['distribution'] != "Fedora" > > + tags: [ 'build-deps' ] > > > > - name: Fedora-specific set up > > ansible.builtin.include_tasks: install-deps/fedora/main.yml > > when: > > - ansible_os_family == 'RedHat' > > - ansible_facts['distribution'] == "Fedora" > > + tags: [ 'build-deps' ] > > diff --git a/playbooks/roles/libvirt_user/tasks/install-deps/main.yml b/playbooks/roles/libvirt_user/tasks/install-deps/main.yml > > index c652ab2..c2f58c8 100644 > > --- a/playbooks/roles/libvirt_user/tasks/install-deps/main.yml > > +++ b/playbooks/roles/libvirt_user/tasks/install-deps/main.yml > > @@ -11,11 +11,18 @@ > > - name: Distribution specific setup > > import_tasks: debian/main.yml > > when: ansible_facts['os_family']|lower == 'debian' > > + tags: [ 'build-deps' ] > > + > > We've had a recent issue with tags import* and include* modules. The > import_tasks + tags here will do tag inheritance. So, all the tasks > under install-deps will run when build-deps tag is passed. > > On the other hand, I think this will conflict with libvirt setup > (kdevops_install_libvirt target)? Maybe. TBH, I did not check the dependencies on this. This can be dropped and we can mainline only the patches that make sense. > > > - import_tasks: suse/main.yml > > when: ansible_facts['os_family']|lower == 'suse' > > + tags: [ 'build-deps' ] > > + > > - import_tasks: redhat/main.yml > > when: > > - ansible_facts['os_family']|lower == 'redhat' > > - ansible_facts['distribution']|lower != "fedora" > > + tags: [ 'build-deps' ] > > + > > - import_tasks: fedora/main.yml > > when: ansible_facts['distribution']|lower == "fedora" > > + tags: [ 'build-deps' ] > > diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile > > index 52397d5..dbfb372 100644 > > --- a/scripts/guestfs.Makefile > > +++ b/scripts/guestfs.Makefile > > @@ -78,7 +78,7 @@ bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) > > playbooks/bringup_guestfs.yml \ > > -e 'ansible_python_interpreter=/usr/bin/python3' \ > > --extra-vars=@./extra_vars.yaml \ > > - --tags config-check,network > > + --tags config-check,network,build-deps > > Since this is a build-deps, I suggest adding a new target to bringup_guestfs` > > > $(Q)$(TOPDIR)/scripts/bringup_guestfs.sh > > $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ > > --inventory localhost, \ > > > > -- > > 2.44.2 > > > >
On Fri, Jan 31, 2025 at 05:57:36PM +0100, Joel Granados wrote: > On Thu, Jan 30, 2025 at 10:15:14AM +0100, Daniel Gomez wrote: > > On Mon, Jan 13, 2025 at 12:53:03PM +0100, Joel Granados wrote: ... > > > - not only_verify_user|bool > > > + tags: [ 'build-deps' ] > > > > > > - name: Ensure our user is part of the libvirt/kvm groups > > > - tags: [ 'journal' ] > > > + tags: [ 'journal', 'build-deps' ] > > > become: yes > > > become_flags: 'su - -c' > > > become_method: sudo > > > diff --git a/playbooks/roles/libvirt_user/tasks/enable-user/main.yml b/playbooks/roles/libvirt_user/tasks/enable-user/main.yml > > > index 04f8e31..364d625 100644 > > > --- a/playbooks/roles/libvirt_user/tasks/enable-user/main.yml > > > +++ b/playbooks/roles/libvirt_user/tasks/enable-user/main.yml > > > @@ -2,19 +2,23 @@ > > > - name: Debian-specific set up > > > ansible.builtin.include_tasks: install-deps/debian/main.yml > > > when: ansible_os_family == 'Debian' > > > + tags: [ 'build-deps' ] > > > > Enabling the user while we do bringup will not succeed if we did not run > > this before, force the user to log out and log in again. Why do we need > > enable-user to be run with build-deps? > In order to run it from other roles. If you do not have it taged, then it is > skipped. We can use another tag, but it needs to be tagged, in order for it to > be called from another role. > > Notice that I added build-deps tags in the bringup_guestfs call. It does not > mean that all the build-deps are going to be run. It means that all the tage > included in the --tag argument will be called and the rest ignored. its a way to > run the subset of a role (unless I misunderstood). I'll leave this out of my V2 for now. We can come back to it if it makes sense. Best
diff --git a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml index 1ad7f17..5797bd6 100644 --- a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml +++ b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml @@ -9,12 +9,14 @@ when: - not libvirt_session - not only_verify_user|bool + tags: [ 'build-deps' ] - name: Check if apparmor_status exists stat: path: /usr/sbin/apparmor_status register: apparmor_file_stat_result when: 'only_verify_user|bool' + tags: [ 'build-deps' ] - name: Verify if AppArmor is disabled when applicable become: yes @@ -26,6 +28,7 @@ when: - 'only_verify_user|bool' - 'apparmor_file_stat_result.stat.exists' + tags: [ 'build-deps' ] - name: Verifies user's effective group allows to run libvirt/kvm without being root shell: groups | grep {{ item }} @@ -41,9 +44,10 @@ when: - not libvirt_session - not only_verify_user|bool + tags: [ 'build-deps' ] - name: Ensure our user is part of the libvirt/kvm groups - tags: [ 'journal' ] + tags: [ 'journal', 'build-deps' ] become: yes become_flags: 'su - -c' become_method: sudo diff --git a/playbooks/roles/libvirt_user/tasks/enable-user/main.yml b/playbooks/roles/libvirt_user/tasks/enable-user/main.yml index 04f8e31..364d625 100644 --- a/playbooks/roles/libvirt_user/tasks/enable-user/main.yml +++ b/playbooks/roles/libvirt_user/tasks/enable-user/main.yml @@ -2,19 +2,23 @@ - name: Debian-specific set up ansible.builtin.include_tasks: install-deps/debian/main.yml when: ansible_os_family == 'Debian' + tags: [ 'build-deps' ] - name: SuSE-specific set up ansible.builtin.include_tasks: install-deps/suse/main.yml when: ansible_os_family == 'Suse' + tags: [ 'build-deps' ] - name: Red Hat-specific set up ansible.builtin.include_tasks: install-deps/redhat/main.yml when: - ansible_os_family == 'RedHat' - ansible_facts['distribution'] != "Fedora" + tags: [ 'build-deps' ] - name: Fedora-specific set up ansible.builtin.include_tasks: install-deps/fedora/main.yml when: - ansible_os_family == 'RedHat' - ansible_facts['distribution'] == "Fedora" + tags: [ 'build-deps' ] diff --git a/playbooks/roles/libvirt_user/tasks/install-deps/main.yml b/playbooks/roles/libvirt_user/tasks/install-deps/main.yml index c652ab2..c2f58c8 100644 --- a/playbooks/roles/libvirt_user/tasks/install-deps/main.yml +++ b/playbooks/roles/libvirt_user/tasks/install-deps/main.yml @@ -11,11 +11,18 @@ - name: Distribution specific setup import_tasks: debian/main.yml when: ansible_facts['os_family']|lower == 'debian' + tags: [ 'build-deps' ] + - import_tasks: suse/main.yml when: ansible_facts['os_family']|lower == 'suse' + tags: [ 'build-deps' ] + - import_tasks: redhat/main.yml when: - ansible_facts['os_family']|lower == 'redhat' - ansible_facts['distribution']|lower != "fedora" + tags: [ 'build-deps' ] + - import_tasks: fedora/main.yml when: ansible_facts['distribution']|lower == "fedora" + tags: [ 'build-deps' ] diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile index 52397d5..dbfb372 100644 --- a/scripts/guestfs.Makefile +++ b/scripts/guestfs.Makefile @@ -78,7 +78,7 @@ bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) playbooks/bringup_guestfs.yml \ -e 'ansible_python_interpreter=/usr/bin/python3' \ --extra-vars=@./extra_vars.yaml \ - --tags config-check,network + --tags config-check,network,build-deps $(Q)$(TOPDIR)/scripts/bringup_guestfs.sh $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ --inventory localhost, \
Add build-deps tag to enable-user and install-deps tasks in libvirt_user role so they can be used from other roles. With no tags, these tasks are skipped. Signed-off-by: Joel Granados <joel.granados@kernel.org> --- playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml | 6 +++++- playbooks/roles/libvirt_user/tasks/enable-user/main.yml | 4 ++++ playbooks/roles/libvirt_user/tasks/install-deps/main.yml | 7 +++++++ scripts/guestfs.Makefile | 2 +- 4 files changed, 17 insertions(+), 2 deletions(-)