| Message ID | 20250113-jag-bringup_fixes-v1-7-fb28030b1f26@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kdevops: Various fixes | expand |
On Mon, Jan 13, 2025 at 12:53:05PM +0100, Joel Granados wrote: > This patch does two things: > * Remove the become_flags from sudo tasks in bringup_guestfs and > libvirt_user roles > * Call the playbook with the -K argument so the user enters the > sudo password just once after the playbook call > > Signed-off-by: Joel Granados <joel.granados@kernel.org> > --- > playbooks/roles/bringup_guestfs/tasks/main.yml | 6 ------ > playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml | 1 - > scripts/guestfs.Makefile | 2 +- > 3 files changed, 1 insertion(+), 8 deletions(-) > > diff --git a/playbooks/roles/bringup_guestfs/tasks/main.yml b/playbooks/roles/bringup_guestfs/tasks/main.yml > index 07e0fc4..95631aa 100644 > --- a/playbooks/roles/bringup_guestfs/tasks/main.yml > +++ b/playbooks/roles/bringup_guestfs/tasks/main.yml > @@ -59,7 +59,6 @@ > > - name: Check dnsmasq service status > become: yes > - become_flags: 'su - -c' > become_method: sudo > command: systemctl is-enabled dnsmasq > register: dnsmasq_enabled > @@ -72,7 +71,6 @@ > > - name: Check if dnsmasq service is active > become: yes > - become_flags: 'su - -c' > become_method: sudo > command: systemctl is-active dnsmasq > register: dnsmasq_active > @@ -94,7 +92,6 @@ > > - name: Check if libvirt default network is running > become: yes > - become_flags: 'su - -c' > become_method: sudo > shell: virsh net-list | grep -q default > register: libvirt_default_net > @@ -106,7 +103,6 @@ > - name: Start the libvirt default network if not running > command: virsh net-start default > become: yes > - become_flags: 'su - -c' > become_method: sudo > when: > - 'libvirt_uri_system|bool' > @@ -123,7 +119,6 @@ > > - name: Look for console.log files in guestfs subdirectories to check for CI enablement > become: yes > - become_flags: 'su - -c' > become_method: sudo > find: > paths: "{{ topdir_path }}/guestfs" > @@ -137,7 +132,6 @@ > > - name: Ensure console.log files are owned by the main user for CI monitoring > become: yes > - become_flags: 'su - -c' > become_method: sudo > file: > path: "{{ item.path }}" > 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 5797bd6..2fa31ce 100644 > --- a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > +++ b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > @@ -49,7 +49,6 @@ > - name: Ensure our user is part of the libvirt/kvm groups > tags: [ 'journal', 'build-deps' ] > become: yes > - become_flags: 'su - -c' > become_method: sudo > ansible.builtin.user: > name: "{{ ansible_user_id }}" > diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile > index dbfb372..480bc4f 100644 > --- a/scripts/guestfs.Makefile > +++ b/scripts/guestfs.Makefile > @@ -73,7 +73,7 @@ $(KDEVOPS_PROVISIONED_SSH): > $(Q)touch $(KDEVOPS_PROVISIONED_SSH) > > bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) > - $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ > + $(Q)ansible-playbook -K $(ANSIBLE_VERBOSE) --connection=local \ I think using the long version, --ask-become-pass, makes it easier to read and understand what we are doing here, also it will be as the rest of the arguments (except for -e). Daniel > --inventory localhost, \ > playbooks/bringup_guestfs.yml \ > -e 'ansible_python_interpreter=/usr/bin/python3' \ > > -- > 2.44.2 > >
On Mon, Jan 13, 2025 at 01:32:01PM +0100, Daniel Gomez wrote: > On Mon, Jan 13, 2025 at 12:53:05PM +0100, Joel Granados wrote: > > This patch does two things: > > * Remove the become_flags from sudo tasks in bringup_guestfs and > > libvirt_user roles > > * Call the playbook with the -K argument so the user enters the > > sudo password just once after the playbook call > > > > Signed-off-by: Joel Granados <joel.granados@kernel.org> > > --- > > playbooks/roles/bringup_guestfs/tasks/main.yml | 6 ------ > > playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml | 1 - > > scripts/guestfs.Makefile | 2 +- > > 3 files changed, 1 insertion(+), 8 deletions(-) > > > > diff --git a/playbooks/roles/bringup_guestfs/tasks/main.yml b/playbooks/roles/bringup_guestfs/tasks/main.yml > > index 07e0fc4..95631aa 100644 > > --- a/playbooks/roles/bringup_guestfs/tasks/main.yml > > +++ b/playbooks/roles/bringup_guestfs/tasks/main.yml > > @@ -59,7 +59,6 @@ > > > > - name: Check dnsmasq service status > > become: yes > > - become_flags: 'su - -c' > > become_method: sudo > > command: systemctl is-enabled dnsmasq > > register: dnsmasq_enabled > > @@ -72,7 +71,6 @@ > > > > - name: Check if dnsmasq service is active > > become: yes > > - become_flags: 'su - -c' > > become_method: sudo > > command: systemctl is-active dnsmasq > > register: dnsmasq_active > > @@ -94,7 +92,6 @@ > > > > - name: Check if libvirt default network is running > > become: yes > > - become_flags: 'su - -c' > > become_method: sudo > > shell: virsh net-list | grep -q default > > register: libvirt_default_net > > @@ -106,7 +103,6 @@ > > - name: Start the libvirt default network if not running > > command: virsh net-start default > > become: yes > > - become_flags: 'su - -c' > > become_method: sudo > > when: > > - 'libvirt_uri_system|bool' > > @@ -123,7 +119,6 @@ > > > > - name: Look for console.log files in guestfs subdirectories to check for CI enablement > > become: yes > > - become_flags: 'su - -c' > > become_method: sudo > > find: > > paths: "{{ topdir_path }}/guestfs" > > @@ -137,7 +132,6 @@ > > > > - name: Ensure console.log files are owned by the main user for CI monitoring > > become: yes > > - become_flags: 'su - -c' > > become_method: sudo > > file: > > path: "{{ item.path }}" > > 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 5797bd6..2fa31ce 100644 > > --- a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > > +++ b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > > @@ -49,7 +49,6 @@ > > - name: Ensure our user is part of the libvirt/kvm groups > > tags: [ 'journal', 'build-deps' ] > > become: yes > > - become_flags: 'su - -c' > > become_method: sudo > > ansible.builtin.user: > > name: "{{ ansible_user_id }}" > > diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile > > index dbfb372..480bc4f 100644 > > --- a/scripts/guestfs.Makefile > > +++ b/scripts/guestfs.Makefile > > @@ -73,7 +73,7 @@ $(KDEVOPS_PROVISIONED_SSH): > > $(Q)touch $(KDEVOPS_PROVISIONED_SSH) > > > > bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) > > - $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ > > + $(Q)ansible-playbook -K $(ANSIBLE_VERBOSE) --connection=local \ > > I think using the long version, --ask-become-pass, makes it easier to > read and understand what we are doing here, also it will be as the rest > of the arguments (except for -e). good idea. Done! > > Daniel > > > --inventory localhost, \ > > playbooks/bringup_guestfs.yml \ > > -e 'ansible_python_interpreter=/usr/bin/python3' \ > > > > -- > > 2.44.2 > > > >
On Mon, Jan 13, 2025 at 12:53:05PM +0100, Joel Granados wrote: > This patch does two things: > * Remove the become_flags from sudo tasks in bringup_guestfs and > libvirt_user roles > * Call the playbook with the -K argument so the user enters the > sudo password just once after the playbook call > > Signed-off-by: Joel Granados <joel.granados@kernel.org> > --- > playbooks/roles/bringup_guestfs/tasks/main.yml | 6 ------ > playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml | 1 - > scripts/guestfs.Makefile | 2 +- > 3 files changed, 1 insertion(+), 8 deletions(-) > > diff --git a/playbooks/roles/bringup_guestfs/tasks/main.yml b/playbooks/roles/bringup_guestfs/tasks/main.yml > index 07e0fc4..95631aa 100644 > --- a/playbooks/roles/bringup_guestfs/tasks/main.yml > +++ b/playbooks/roles/bringup_guestfs/tasks/main.yml > @@ -59,7 +59,6 @@ > > - name: Check dnsmasq service status > become: yes > - become_flags: 'su - -c' > become_method: sudo > command: systemctl is-enabled dnsmasq > register: dnsmasq_enabled > @@ -72,7 +71,6 @@ > > - name: Check if dnsmasq service is active > become: yes > - become_flags: 'su - -c' > become_method: sudo > command: systemctl is-active dnsmasq > register: dnsmasq_active > @@ -94,7 +92,6 @@ > > - name: Check if libvirt default network is running > become: yes > - become_flags: 'su - -c' > become_method: sudo > shell: virsh net-list | grep -q default > register: libvirt_default_net > @@ -106,7 +103,6 @@ > - name: Start the libvirt default network if not running > command: virsh net-start default > become: yes > - become_flags: 'su - -c' > become_method: sudo > when: > - 'libvirt_uri_system|bool' > @@ -123,7 +119,6 @@ > > - name: Look for console.log files in guestfs subdirectories to check for CI enablement > become: yes > - become_flags: 'su - -c' > become_method: sudo > find: > paths: "{{ topdir_path }}/guestfs" > @@ -137,7 +132,6 @@ > > - name: Ensure console.log files are owned by the main user for CI monitoring > become: yes > - become_flags: 'su - -c' > become_method: sudo > file: > path: "{{ item.path }}" > 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 5797bd6..2fa31ce 100644 > --- a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > +++ b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml > @@ -49,7 +49,6 @@ > - name: Ensure our user is part of the libvirt/kvm groups > tags: [ 'journal', 'build-deps' ] > become: yes > - become_flags: 'su - -c' > become_method: sudo > ansible.builtin.user: > name: "{{ ansible_user_id }}" > diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile > index dbfb372..480bc4f 100644 > --- a/scripts/guestfs.Makefile > +++ b/scripts/guestfs.Makefile > @@ -73,7 +73,7 @@ $(KDEVOPS_PROVISIONED_SSH): > $(Q)touch $(KDEVOPS_PROVISIONED_SSH) > > bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) > - $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ > + $(Q)ansible-playbook -K $(ANSIBLE_VERBOSE) --connection=local \ Unfortunately, we can't do this. I had the same change for other playbooks in the past. An example in commit f1d8c698fcd0bfd9f4a726d070e47e25b2218c29 ("bootlinux: Makefile: Ask for password for sudo permissions") available only in kdevops-history repository. The change was reverted [1] back then for the following reason: Revert "bootlinux: Makefile: Ask for password for sudo permissions" This is being reverted as passwordless sudo systems were working without human intervention, this now forces a human to enter a password even if a user did not have to enter a password for sudo. [1] 5198eee7a653bc22e569af9a32bee292aa83b3d3 (Revert "bootlinux: Makefile: Ask for password for sudo permissions"). I think we can use --become and pass the sudo user password via ansible_become_password inventory var. And probably and encrypted vault would be required to avoid storing the password in plain text. > --inventory localhost, \ > playbooks/bringup_guestfs.yml \ > -e 'ansible_python_interpreter=/usr/bin/python3' \ > > -- > 2.44.2 > >
diff --git a/playbooks/roles/bringup_guestfs/tasks/main.yml b/playbooks/roles/bringup_guestfs/tasks/main.yml index 07e0fc4..95631aa 100644 --- a/playbooks/roles/bringup_guestfs/tasks/main.yml +++ b/playbooks/roles/bringup_guestfs/tasks/main.yml @@ -59,7 +59,6 @@ - name: Check dnsmasq service status become: yes - become_flags: 'su - -c' become_method: sudo command: systemctl is-enabled dnsmasq register: dnsmasq_enabled @@ -72,7 +71,6 @@ - name: Check if dnsmasq service is active become: yes - become_flags: 'su - -c' become_method: sudo command: systemctl is-active dnsmasq register: dnsmasq_active @@ -94,7 +92,6 @@ - name: Check if libvirt default network is running become: yes - become_flags: 'su - -c' become_method: sudo shell: virsh net-list | grep -q default register: libvirt_default_net @@ -106,7 +103,6 @@ - name: Start the libvirt default network if not running command: virsh net-start default become: yes - become_flags: 'su - -c' become_method: sudo when: - 'libvirt_uri_system|bool' @@ -123,7 +119,6 @@ - name: Look for console.log files in guestfs subdirectories to check for CI enablement become: yes - become_flags: 'su - -c' become_method: sudo find: paths: "{{ topdir_path }}/guestfs" @@ -137,7 +132,6 @@ - name: Ensure console.log files are owned by the main user for CI monitoring become: yes - become_flags: 'su - -c' become_method: sudo file: path: "{{ item.path }}" 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 5797bd6..2fa31ce 100644 --- a/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml +++ b/playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml @@ -49,7 +49,6 @@ - name: Ensure our user is part of the libvirt/kvm groups tags: [ 'journal', 'build-deps' ] become: yes - become_flags: 'su - -c' become_method: sudo ansible.builtin.user: name: "{{ ansible_user_id }}" diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile index dbfb372..480bc4f 100644 --- a/scripts/guestfs.Makefile +++ b/scripts/guestfs.Makefile @@ -73,7 +73,7 @@ $(KDEVOPS_PROVISIONED_SSH): $(Q)touch $(KDEVOPS_PROVISIONED_SSH) bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) - $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ + $(Q)ansible-playbook -K $(ANSIBLE_VERBOSE) --connection=local \ --inventory localhost, \ playbooks/bringup_guestfs.yml \ -e 'ansible_python_interpreter=/usr/bin/python3' \
This patch does two things: * Remove the become_flags from sudo tasks in bringup_guestfs and libvirt_user roles * Call the playbook with the -K argument so the user enters the sudo password just once after the playbook call Signed-off-by: Joel Granados <joel.granados@kernel.org> --- playbooks/roles/bringup_guestfs/tasks/main.yml | 6 ------ playbooks/roles/libvirt_user/tasks/enable-user/debian/main.yml | 1 - scripts/guestfs.Makefile | 2 +- 3 files changed, 1 insertion(+), 8 deletions(-)