| Message ID | 20250306-fix-bringup-network-v2-2-85298d7a0ba5@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | guestfs: bringup: fix networking issue for debian hosts | expand |
On 3/6/25 7:38 AM, Daniel Gomez wrote: > From: Daniel Gomez <da.gomez@samsung.com> > > When doing make bringup, virt-builder boots an image with network to be > able to install packages for image baseline creation. In order to have > network in the booted image, virt-builder assumes isc-dhcp-client is the > default Debian package for this task. However, this is not the case as > Debian trixie has replaced the default dhcp client isc-dhcp-client with > dhcpcd. > > The proper way to fix this is to address the problem at libguestfs > and ensure both packages can be used. This was supported already in > libguestfs for Fedora/Red Hat but not for Debian. So, package support > has been added to libguestfs for Debian hosts [1][2][3]. The fix is now > merged in libguestfs v1.54.1. > > [1] > https://lists.libguestfs.org/archives/list/guestfs@lists.libguestfs.org/ > thread/ANINW4N4D2VYRD5GTAUQWDZDECAN6XFM/#ANINW4N4D2VYRD5GTAUQWDZDECAN6XF > M > > [2] 91fab3f498b6847454656acd2f3d5788c533033b ("appliance: add dhcpcd > support on Debian") > > [3] > https://github.com/libguestfs/libguestfs/commit/91fab3f498b6847454656acd > 2f3d5788c533033b > > While we wait Debian to upgrade the package to a newer libguestfs > release that includes the fix, we need to ensure isc-dhcp-client is part > of the host tools. Note that we need to be explicit as isc-dhcp-client > is listed as optional runtime dependency for libguestfs [4]. > > [4] https://packages.debian.org/trixie/libguestfs0t64 > > This was reported already in a previous Debian release: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=775514 > > In order to fix this, introduce a new target under the bringup_guestfs > playbook to install libguestfs and it's dependencies. Add this for > Debian, Suse and Fedora/RedHat. > > This new target also allows to remove the dependency check at > min_deps introduced by commit 0111187b7ac23a12b6c71e99ff39092e6e752f2d > ("Makefile.min_deps: ensure you have virt-builder when using > guestfs"). Min deps is intended for minimal kdevops dependencies to > run kdevops. And it's kdevops job with Ansible/Makefile dedicated > playbooks-roles/targets to ensure libguestfs (including virt-builder) is > installed in the host. > > To replicate the issue in Debian testing (without isc-dhcp-client > installed and a libguestfs version < 1.54.1 ), run any of the following > virt-builder commands: > > virt-builder fedora-27 --install qemu-guest-agent > [ 3.2] Downloading: http://archive.libguestfs.org/builder/fedora-27.xz > [ 3.7] Planning how to build this image > [ 3.7] Uncompressing > [ 5.3] Opening the new disk > libguestfs: warning: current user is not a member of the KVM group > (group ID 994). This user cannot access /dev/kvm, so libguestfs may run > very slowly. It is recommended that you 'chmod 0666 /dev/kvm' or add > the current user to the KVM group (you might need to log out and log > in again). > [ 27.3] Setting a random seed > [ 27.4] Installing packages: qemu-guest-agent > Error: Failed to synchronize cache for repo 'updates' > virt-builder: error: dnf -y install 'qemu-guest-agent': command exited > with an error > > If reporting bugs, run virt-builder with debugging enabled and include the > complete output: > > virt-builder -v -x [...] > > sudo virt-builder debian-12 --install sudo > [ 0.7] Downloading: http://builder.libguestfs.org/debian-12.xz > [ 1.0] Planning how to build this image > [ 1.0] Uncompressing > [ 2.9] Opening the new disk > [ 6.3] Setting a random seed > virt-builder: warning: random seed could not be set for this type of guest > [ 6.4] Installing packages: sudo > Ign:1 http://security.debian.org/debian-security bookworm-security > InRelease > Ign:2 http://deb.debian.org/debian bookworm InRelease > Ign:3 http://deb.debian.org/debian bookworm-updates InRelease > Ign:1 http://security.debian.org/debian-security bookworm-security > InRelease > Ign:2 http://deb.debian.org/debian bookworm InRelease > Ign:3 http://deb.debian.org/debian bookworm-updates InRelease > Ign:2 http://deb.debian.org/debian bookworm InRelease > Ign:1 http://security.debian.org/debian-security bookworm-security > InRelease > Ign:3 http://deb.debian.org/debian bookworm-updates InRelease > Err:2 http://deb.debian.org/debian bookworm InRelease > Temporary failure resolving 'deb.debian.org' > Err:1 http://security.debian.org/debian-security bookworm-security > InRelease > Temporary failure resolving 'security.debian.org' > Err:3 http://deb.debian.org/debian bookworm-updates InRelease > Temporary failure resolving 'deb.debian.org' > Reading package lists... > W: Failed to fetch http://deb.debian.org/debian/dists/bookworm/InRelease > Temporary failure resolving 'deb.debian.org' > W: Failed to fetch > http://security.debian.org/debian-security/dists/bookworm-security/InRel > ease > Temporary failure resolving 'security.debian.org' > W: Failed to fetch > http://deb.debian.org/debian/dists/bookworm-updates/InRelease Temporary > failure resolving 'deb.debian.org' > W: Some index files failed to download. They have been ignored, or old > ones used instead. > Reading package lists... > Building dependency tree... > Reading state information... > The following NEW packages will be installed: > sudo > 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. > Need to get 1889 kB of archives. > After this operation, 6199 kB of additional disk space will be used. > Ign:1 http://deb.debian.org/debian bookworm/main amd64 sudo amd64 1.9.13p3-1 > Ign:1 http://deb.debian.org/debian bookworm/main amd64 sudo amd64 1.9.13p3-1 > Ign:1 http://deb.debian.org/debian bookworm/main amd64 sudo amd64 1.9.13p3-1 > Err:1 http://deb.debian.org/debian bookworm/main amd64 sudo amd64 1.9.13p3-1 > Temporary failure resolving 'deb.debian.org' > E: Failed to fetch > http://deb.debian.org/debian/pool/main/s/sudo/sudo_1.9.13p3-1_amd64.deb > Temporary failure resolving 'deb.debian.org' > E: Unable to fetch some archives, maybe run apt-get update or try with > --fix-missing? > virt-builder: error: > export DEBIAN_FRONTEND=noninteractive > apt_opts='-q -y -o Dpkg::Options::=--force-confnew' > apt-get $apt_opts update > apt-get $apt_opts install 'sudo' > : command exited with an error > > If reporting bugs, run virt-builder with debugging enabled and include > the complete output: > > virt-builder -v -x [...] > > Signed-off-by: Daniel Gomez <da.gomez@samsung.com> > --- > Makefile.min_deps | 5 ----- > .../bringup_guestfs/tasks/install-deps/debian/main.yml | 11 +++++++++++ > .../roles/bringup_guestfs/tasks/install-deps/main.yml | 17 +++++++++++++++++ > .../bringup_guestfs/tasks/install-deps/redhat/main.yml | 11 +++++++++++ > .../bringup_guestfs/tasks/install-deps/suse/main.yml | 10 ++++++++++ > playbooks/roles/bringup_guestfs/tasks/main.yml | 4 ++++ > scripts/guestfs.Makefile | 9 +++++++++ > 7 files changed, 62 insertions(+), 5 deletions(-) > > diff --git a/Makefile.min_deps b/Makefile.min_deps > index a1cb99a500575807042fdd7c47b1b192df5db1bd..9d8f205799c463ca85dd80cff7a813ded5b11eea 100644 > --- a/Makefile.min_deps > +++ b/Makefile.min_deps > @@ -10,11 +10,6 @@ BINARY_DEPS += sudo > BINARY_DEPS += make > BINARY_DEPS += nc > BINARY_DEPS += ansible-playbook > -ifneq (y,$(CONFIG_KDEVOPS_FIRST_RUN)) > -ifeq (y,$(CONFIG_GUESTFS)) > -BINARY_DEPS += virt-builder > -endif > -endif > > BINARY_DEPS_VCHECKS := $(subst $(TOPDIR)/,,$(wildcard $(TOPDIR)/scripts/version_check/*)) > > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml > new file mode 100644 > index 0000000000000000000000000000000000000000..9fbfb4703fd268500f643b4522b872a70b0c1808 > --- /dev/null > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml > @@ -0,0 +1,11 @@ > +--- > +- name: Install libguestfs > + become: true > + become_method: ansible.builtin.sudo > + ansible.builtin.apt: > + update_cache: true > + name: > + - libguestfs-tools > + - isc-dhcp-client > + state: present > + tags: ['install-deps'] > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > new file mode 100644 > index 0000000000000000000000000000000000000000..af3be6dea3c3445ecca4f3b3302e4640272882c6 > --- /dev/null > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > @@ -0,0 +1,17 @@ > +--- > +- name: Debian-specific setup > + ansible.builtin.include_tasks: debian/main.yml > + when: ansible_facts['os_family']|lower == 'debian' > + tags: ['install-deps'] > + > +- name: SuSE-specific setup > + ansible.builtin.include_tasks: suse/main.yml > + when: ansible_facts['os_family']|lower == 'suse' > + tags: ['install-deps'] > + > +- name: Fedora/Red Hat Enterprise-specific setup > + ansible.builtin.include_tasks: redhat/main.yml > + when: > + - ansible_facts['os_family']|lower == 'redhat' > + - ansible_facts['distribution']|lower != "fedora" > + tags: ['install-deps'] > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml > new file mode 100644 > index 0000000000000000000000000000000000000000..029b76a2f34927718b36c50080d330f80ee283ba > --- /dev/null > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml > @@ -0,0 +1,11 @@ > +--- > +- name: Install libguestfs on Fedora/RedHat > + become: true > + become_method: ansible.builtin.sudo > + ansible.builtin.yum: > + update_cache: true > + name: > + - libguestfs-tools > + - dhclient > + state: present > + tags: ['install-deps'] > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml > new file mode 100644 > index 0000000000000000000000000000000000000000..9aa5178fd972b90d008f01e9dc127539608feb15 > --- /dev/null > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml > @@ -0,0 +1,10 @@ > +--- > +- name: Install libguestfs on SUSE > + become: true > + become_method: ansible.builtin.sudo > + ansible.builtin.zypper: > + name: > + - libguestfs-tools > + - dhclient > + state: present > + tags: ['install-deps'] > diff --git a/playbooks/roles/bringup_guestfs/tasks/main.yml b/playbooks/roles/bringup_guestfs/tasks/main.yml > index 8e0f82de5d0b9fa958fd1107ca75827f1b394b09..dcbbaef02522ef561b9d990b74a3e7573d912afb 100644 > --- a/playbooks/roles/bringup_guestfs/tasks/main.yml > +++ b/playbooks/roles/bringup_guestfs/tasks/main.yml > @@ -1,3 +1,7 @@ > +- name: Install dependencies > + ansible.builtin.include_tasks: install-deps/main.yml > + tags: ['install-deps'] > + > - name: Verify we're configured {{ topdir_path }}/.config directory exists > stat: > path: "{{ topdir_path }}/.config" > diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile > index 52397d54b89b584b675cf6182113d5a7e22da3c3..a8aa82fa96fa5bd9c181f10ef347bff8e3f78f44 100644 > --- a/scripts/guestfs.Makefile > +++ b/scripts/guestfs.Makefile > @@ -47,6 +47,7 @@ ANSIBLE_EXTRA_ARGS += $(GUESTFS_ARGS) > GUESTFS_BRINGUP_DEPS := > GUESTFS_BRINGUP_DEPS += $(9P_HOST_CLONE) > GUESTFS_BRINGUP_DEPS += $(LIBVIRT_PCIE_PASSTHROUGH) > +GUESTFS_BRINGUP_DEPS += install_libguestfs > > KDEVOPS_PROVISION_METHOD := bringup_guestfs > KDEVOPS_PROVISION_DESTROY_METHOD := destroy_guestfs > @@ -72,6 +73,14 @@ $(KDEVOPS_PROVISIONED_SSH): > $(Q)ansible $(ANSIBLE_VERBOSE) -i hosts all -e 'ansible_python_interpreter=/usr/bin/python3' -m wait_for_connection > $(Q)touch $(KDEVOPS_PROVISIONED_SSH) > > +install_libguestfs: > + $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ > + --inventory localhost, \ > + playbooks/bringup_guestfs.yml \ > + -e 'ansible_python_interpreter=/usr/bin/python3' \ > + --extra-vars=@./extra_vars.yaml \ > + --tags install-deps > + > bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) > $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ > --inventory localhost, \ > Reviewed-by: Chuck Lever <chuck.lever@oracle.com> Nit: consider using ansible.builtin.package rather than yum, zypper, and apt.
On Thu, Mar 06, 2025 at 09:05:08AM +0100, Chuck Lever wrote: > On 3/6/25 7:38 AM, Daniel Gomez wrote: > > From: Daniel Gomez <da.gomez@samsung.com> > > > > When doing make bringup, virt-builder boots an image with network to be > > able to install packages for image baseline creation. In order to have > > network in the booted image, virt-builder assumes isc-dhcp-client is the > > default Debian package for this task. However, this is not the case as > > Debian trixie has replaced the default dhcp client isc-dhcp-client with > > dhcpcd. > > > > The proper way to fix this is to address the problem at libguestfs > > and ensure both packages can be used. This was supported already in > > libguestfs for Fedora/Red Hat but not for Debian. So, package support > > has been added to libguestfs for Debian hosts [1][2][3]. The fix is now > > merged in libguestfs v1.54.1. > > > > [1] > > https://lists.libguestfs.org/archives/list/guestfs@lists.libguestfs.org/ > > thread/ANINW4N4D2VYRD5GTAUQWDZDECAN6XFM/#ANINW4N4D2VYRD5GTAUQWDZDECAN6XF > > M > > > > [2] 91fab3f498b6847454656acd2f3d5788c533033b ("appliance: add dhcpcd > > support on Debian") > > > > [3] > > https://github.com/libguestfs/libguestfs/commit/91fab3f498b6847454656acd > > 2f3d5788c533033b > > > > While we wait Debian to upgrade the package to a newer libguestfs > > release that includes the fix, we need to ensure isc-dhcp-client is part > > of the host tools. Note that we need to be explicit as isc-dhcp-client > > is listed as optional runtime dependency for libguestfs [4]. > > > > [4] https://packages.debian.org/trixie/libguestfs0t64 > > > > This was reported already in a previous Debian release: > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=775514 > > > > In order to fix this, introduce a new target under the bringup_guestfs > > playbook to install libguestfs and it's dependencies. Add this for > > Debian, Suse and Fedora/RedHat. > > > > This new target also allows to remove the dependency check at > > min_deps introduced by commit 0111187b7ac23a12b6c71e99ff39092e6e752f2d > > ("Makefile.min_deps: ensure you have virt-builder when using > > guestfs"). Min deps is intended for minimal kdevops dependencies to > > run kdevops. And it's kdevops job with Ansible/Makefile dedicated > > playbooks-roles/targets to ensure libguestfs (including virt-builder) is > > installed in the host. > > > > To replicate the issue in Debian testing (without isc-dhcp-client > > installed and a libguestfs version < 1.54.1 ), run any of the following > > virt-builder commands: > > > > virt-builder fedora-27 --install qemu-guest-agent > > [ 3.2] Downloading: http://archive.libguestfs.org/builder/fedora-27.xz > > [ 3.7] Planning how to build this image > > [ 3.7] Uncompressing > > [ 5.3] Opening the new disk > > libguestfs: warning: current user is not a member of the KVM group > > (group ID 994). This user cannot access /dev/kvm, so libguestfs may run > > very slowly. It is recommended that you 'chmod 0666 /dev/kvm' or add > > the current user to the KVM group (you might need to log out and log > > in again). > > [ 27.3] Setting a random seed > > [ 27.4] Installing packages: qemu-guest-agent > > Error: Failed to synchronize cache for repo 'updates' > > virt-builder: error: dnf -y install 'qemu-guest-agent': command exited > > with an error > > > > If reporting bugs, run virt-builder with debugging enabled and include the > > complete output: > > > > virt-builder -v -x [...] > > > > sudo virt-builder debian-12 --install sudo > > [ 0.7] Downloading: http://builder.libguestfs.org/debian-12.xz > > [ 1.0] Planning how to build this image > > [ 1.0] Uncompressing > > [ 2.9] Opening the new disk > > [ 6.3] Setting a random seed > > virt-builder: warning: random seed could not be set for this type of guest > > [ 6.4] Installing packages: sudo > > Ign:1 http://security.debian.org/debian-security bookworm-security > > InRelease > > Ign:2 http://deb.debian.org/debian bookworm InRelease > > Ign:3 http://deb.debian.org/debian bookworm-updates InRelease > > Ign:1 http://security.debian.org/debian-security bookworm-security > > InRelease > > Ign:2 http://deb.debian.org/debian bookworm InRelease > > Ign:3 http://deb.debian.org/debian bookworm-updates InRelease > > Ign:2 http://deb.debian.org/debian bookworm InRelease > > Ign:1 http://security.debian.org/debian-security bookworm-security > > InRelease > > Ign:3 http://deb.debian.org/debian bookworm-updates InRelease > > Err:2 http://deb.debian.org/debian bookworm InRelease > > Temporary failure resolving 'deb.debian.org' > > Err:1 http://security.debian.org/debian-security bookworm-security > > InRelease > > Temporary failure resolving 'security.debian.org' > > Err:3 http://deb.debian.org/debian bookworm-updates InRelease > > Temporary failure resolving 'deb.debian.org' > > Reading package lists... > > W: Failed to fetch http://deb.debian.org/debian/dists/bookworm/InRelease > > Temporary failure resolving 'deb.debian.org' > > W: Failed to fetch > > http://security.debian.org/debian-security/dists/bookworm-security/InRel > > ease > > Temporary failure resolving 'security.debian.org' > > W: Failed to fetch > > http://deb.debian.org/debian/dists/bookworm-updates/InRelease Temporary > > failure resolving 'deb.debian.org' > > W: Some index files failed to download. They have been ignored, or old > > ones used instead. > > Reading package lists... > > Building dependency tree... > > Reading state information... > > The following NEW packages will be installed: > > sudo > > 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. > > Need to get 1889 kB of archives. > > After this operation, 6199 kB of additional disk space will be used. > > Ign:1 http://deb.debian.org/debian bookworm/main amd64 sudo amd64 1.9.13p3-1 > > Ign:1 http://deb.debian.org/debian bookworm/main amd64 sudo amd64 1.9.13p3-1 > > Ign:1 http://deb.debian.org/debian bookworm/main amd64 sudo amd64 1.9.13p3-1 > > Err:1 http://deb.debian.org/debian bookworm/main amd64 sudo amd64 1.9.13p3-1 > > Temporary failure resolving 'deb.debian.org' > > E: Failed to fetch > > http://deb.debian.org/debian/pool/main/s/sudo/sudo_1.9.13p3-1_amd64.deb > > Temporary failure resolving 'deb.debian.org' > > E: Unable to fetch some archives, maybe run apt-get update or try with > > --fix-missing? > > virt-builder: error: > > export DEBIAN_FRONTEND=noninteractive > > apt_opts='-q -y -o Dpkg::Options::=--force-confnew' > > apt-get $apt_opts update > > apt-get $apt_opts install 'sudo' > > : command exited with an error > > > > If reporting bugs, run virt-builder with debugging enabled and include > > the complete output: > > > > virt-builder -v -x [...] > > > > Signed-off-by: Daniel Gomez <da.gomez@samsung.com> > > --- > > Makefile.min_deps | 5 ----- > > .../bringup_guestfs/tasks/install-deps/debian/main.yml | 11 +++++++++++ > > .../roles/bringup_guestfs/tasks/install-deps/main.yml | 17 +++++++++++++++++ > > .../bringup_guestfs/tasks/install-deps/redhat/main.yml | 11 +++++++++++ > > .../bringup_guestfs/tasks/install-deps/suse/main.yml | 10 ++++++++++ > > playbooks/roles/bringup_guestfs/tasks/main.yml | 4 ++++ > > scripts/guestfs.Makefile | 9 +++++++++ > > 7 files changed, 62 insertions(+), 5 deletions(-) > > > > diff --git a/Makefile.min_deps b/Makefile.min_deps > > index a1cb99a500575807042fdd7c47b1b192df5db1bd..9d8f205799c463ca85dd80cff7a813ded5b11eea 100644 > > --- a/Makefile.min_deps > > +++ b/Makefile.min_deps > > @@ -10,11 +10,6 @@ BINARY_DEPS += sudo > > BINARY_DEPS += make > > BINARY_DEPS += nc > > BINARY_DEPS += ansible-playbook > > -ifneq (y,$(CONFIG_KDEVOPS_FIRST_RUN)) > > -ifeq (y,$(CONFIG_GUESTFS)) > > -BINARY_DEPS += virt-builder > > -endif > > -endif > > > > BINARY_DEPS_VCHECKS := $(subst $(TOPDIR)/,,$(wildcard $(TOPDIR)/scripts/version_check/*)) > > > > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml > > new file mode 100644 > > index 0000000000000000000000000000000000000000..9fbfb4703fd268500f643b4522b872a70b0c1808 > > --- /dev/null > > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml > > @@ -0,0 +1,11 @@ > > +--- > > +- name: Install libguestfs > > + become: true > > + become_method: ansible.builtin.sudo > > + ansible.builtin.apt: > > + update_cache: true > > + name: > > + - libguestfs-tools > > + - isc-dhcp-client > > + state: present > > + tags: ['install-deps'] > > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > > new file mode 100644 > > index 0000000000000000000000000000000000000000..af3be6dea3c3445ecca4f3b3302e4640272882c6 > > --- /dev/null > > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > > @@ -0,0 +1,17 @@ > > +--- > > +- name: Debian-specific setup > > + ansible.builtin.include_tasks: debian/main.yml > > + when: ansible_facts['os_family']|lower == 'debian' > > + tags: ['install-deps'] > > + > > +- name: SuSE-specific setup > > + ansible.builtin.include_tasks: suse/main.yml > > + when: ansible_facts['os_family']|lower == 'suse' > > + tags: ['install-deps'] > > + > > +- name: Fedora/Red Hat Enterprise-specific setup > > + ansible.builtin.include_tasks: redhat/main.yml > > + when: > > + - ansible_facts['os_family']|lower == 'redhat' > > + - ansible_facts['distribution']|lower != "fedora" > > + tags: ['install-deps'] > > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml > > new file mode 100644 > > index 0000000000000000000000000000000000000000..029b76a2f34927718b36c50080d330f80ee283ba > > --- /dev/null > > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml > > @@ -0,0 +1,11 @@ > > +--- > > +- name: Install libguestfs on Fedora/RedHat > > + become: true > > + become_method: ansible.builtin.sudo > > + ansible.builtin.yum: > > + update_cache: true > > + name: > > + - libguestfs-tools > > + - dhclient > > + state: present > > + tags: ['install-deps'] > > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml > > new file mode 100644 > > index 0000000000000000000000000000000000000000..9aa5178fd972b90d008f01e9dc127539608feb15 > > --- /dev/null > > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml > > @@ -0,0 +1,10 @@ > > +--- > > +- name: Install libguestfs on SUSE > > + become: true > > + become_method: ansible.builtin.sudo > > + ansible.builtin.zypper: > > + name: > > + - libguestfs-tools > > + - dhclient > > + state: present > > + tags: ['install-deps'] > > diff --git a/playbooks/roles/bringup_guestfs/tasks/main.yml b/playbooks/roles/bringup_guestfs/tasks/main.yml > > index 8e0f82de5d0b9fa958fd1107ca75827f1b394b09..dcbbaef02522ef561b9d990b74a3e7573d912afb 100644 > > --- a/playbooks/roles/bringup_guestfs/tasks/main.yml > > +++ b/playbooks/roles/bringup_guestfs/tasks/main.yml > > @@ -1,3 +1,7 @@ > > +- name: Install dependencies > > + ansible.builtin.include_tasks: install-deps/main.yml > > + tags: ['install-deps'] > > + > > - name: Verify we're configured {{ topdir_path }}/.config directory exists > > stat: > > path: "{{ topdir_path }}/.config" > > diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile > > index 52397d54b89b584b675cf6182113d5a7e22da3c3..a8aa82fa96fa5bd9c181f10ef347bff8e3f78f44 100644 > > --- a/scripts/guestfs.Makefile > > +++ b/scripts/guestfs.Makefile > > @@ -47,6 +47,7 @@ ANSIBLE_EXTRA_ARGS += $(GUESTFS_ARGS) > > GUESTFS_BRINGUP_DEPS := > > GUESTFS_BRINGUP_DEPS += $(9P_HOST_CLONE) > > GUESTFS_BRINGUP_DEPS += $(LIBVIRT_PCIE_PASSTHROUGH) > > +GUESTFS_BRINGUP_DEPS += install_libguestfs > > > > KDEVOPS_PROVISION_METHOD := bringup_guestfs > > KDEVOPS_PROVISION_DESTROY_METHOD := destroy_guestfs > > @@ -72,6 +73,14 @@ $(KDEVOPS_PROVISIONED_SSH): > > $(Q)ansible $(ANSIBLE_VERBOSE) -i hosts all -e 'ansible_python_interpreter=/usr/bin/python3' -m wait_for_connection > > $(Q)touch $(KDEVOPS_PROVISIONED_SSH) > > > > +install_libguestfs: > > + $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ > > + --inventory localhost, \ > > + playbooks/bringup_guestfs.yml \ > > + -e 'ansible_python_interpreter=/usr/bin/python3' \ > > + --extra-vars=@./extra_vars.yaml \ > > + --tags install-deps > > + > > bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) > > $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ > > --inventory localhost, \ > > > > Reviewed-by: Chuck Lever <chuck.lever@oracle.com> > > Nit: consider using ansible.builtin.package rather than yum, zypper, and > apt. Ok, I replaced the specific package manager with the suggested one + renamed the dhcp clients for Red Hat and Suse using the names found in libguestfs [1]. [1] https://github.com/libguestfs/libguestfs/blob/master/appliance/packagelist.in git diff sent/20250129-fix-bringup-network-5f7e43b3bafe-v2 diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml index 9fbfb47..6b502f5 100644 --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml @@ -2,7 +2,7 @@ - name: Install libguestfs become: true become_method: ansible.builtin.sudo - ansible.builtin.apt: + ansible.builtin.package: update_cache: true name: - libguestfs-tools diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml index 029b76a..5e4bfa9 100644 --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml @@ -2,10 +2,10 @@ - name: Install libguestfs on Fedora/RedHat become: true become_method: ansible.builtin.sudo - ansible.builtin.yum: + ansible.builtin.package: update_cache: true name: - libguestfs-tools - - dhclient + - dhcpcd state: present tags: ['install-deps'] diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml index 9aa5178..730bc20 100644 --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml @@ -2,9 +2,9 @@ - name: Install libguestfs on SUSE become: true become_method: ansible.builtin.sudo - ansible.builtin.zypper: + ansible.builtin.package: name: - libguestfs-tools - - dhclient + - dhcpcd state: present tags: ['install-deps'] I'll push these changes after CI finishes. > > > -- > Chuck Lever
On Thu, Mar 06, 2025 at 07:58:09PM +0100, Daniel Gomez wrote: > Ok, I replaced the specific package manager with the suggested one + renamed the > dhcp clients for Red Hat and Suse using the names found in libguestfs [1]. The idea was not to use it on each distro's thing but rather *one* single entry for all distros. But since that requires porting first the list, I think for this patch we can keep the old apt / yum / etc and later we can migrate the entries. That is, with that solution we should no thave to do the mapping of packages as I understand it. Luis
On Thu, Mar 06, 2025 at 06:13:32PM +0100, Luis Chamberlain wrote: > On Thu, Mar 06, 2025 at 07:58:09PM +0100, Daniel Gomez wrote: > > Ok, I replaced the specific package manager with the suggested one + renamed the > > dhcp clients for Red Hat and Suse using the names found in libguestfs [1]. > > The idea was not to use it on each distro's thing but rather *one* > single entry for all distros. But since that requires porting first the > list, I think for this patch we can keep the old apt / yum / etc and > later we can migrate the entries. Ok, I'll provide a diff. > > That is, with that solution we should no thave to do the mapping of > packages as I understand it. Quoting from [1]: "While ansible.builtin.package abstracts package managers to ease dealing with multiple distributions, package name often differs for the same software." I think we can do things like "{{ libguestfs }}" and make the variable dependent on the distro. But we still need to manage distro-specific package names. [1] https://docs.ansible.com/ansible/latest/collections/ansible/builtin/package_module.html > > Luis
On Fri, Mar 07, 2025 at 09:48:30AM +0100, Daniel Gomez wrote: > On Thu, Mar 06, 2025 at 06:13:32PM +0100, Luis Chamberlain wrote: > > On Thu, Mar 06, 2025 at 07:58:09PM +0100, Daniel Gomez wrote: > > > Ok, I replaced the specific package manager with the suggested one + renamed the > > > dhcp clients for Red Hat and Suse using the names found in libguestfs [1]. > > > > The idea was not to use it on each distro's thing but rather *one* > > single entry for all distros. But since that requires porting first the > > list, I think for this patch we can keep the old apt / yum / etc and > > later we can migrate the entries. > > Ok, I'll provide a diff. This works for me, let me know what you think: Note that Ansible Lint reported var-nameing[no-role-prefix] [1][2], so variables use the bringup_guestfs_ prefix suggested. [1] var-naming[no-role-prefix]: Variables names from within roles should use role_name_ as a prefix. Underlines are accepted before the prefix. [2] https://ansible.readthedocs.io/projects/lint/rules/var-naming/ diff --git a/playbooks/roles/bringup_guestfs/defaults/main.yml b/playbooks/roles/bringup_guestfs/defaults/main.yml index 65867cd..a5bcb15 100644 --- a/playbooks/roles/bringup_guestfs/defaults/main.yml +++ b/playbooks/roles/bringup_guestfs/defaults/main.yml @@ -10,3 +10,12 @@ distro_suse: False distro_suse_based: False distro_ubuntu: False dnsmasq_files_exist: False +bringup_guestfs_libguestfs_rdepends_debian: + - libguestfs-tools + - isc-dhcp-client +bringup_guestfs_libguestfs_rdepends_suse: + - libguestfs-tools + - dhcpcd +bringup_guestfs_libguestfs_rdepends_redhat: + - libguestfs-tools + - dhcpcd diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml deleted file mode 100644 index 6b502f5..0000000 --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml +++ /dev/null @@ -1,11 +0,0 @@ ---- -- name: Install libguestfs - become: true - become_method: ansible.builtin.sudo - ansible.builtin.package: - update_cache: true - name: - - libguestfs-tools - - isc-dhcp-client - state: present - tags: ['install-deps'] diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml index af3be6d..790d446 100644 --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml @@ -1,17 +1,21 @@ --- -- name: Debian-specific setup - ansible.builtin.include_tasks: debian/main.yml - when: ansible_facts['os_family']|lower == 'debian' +- name: Set libguestfs dependencies based on OS family + ansible.builtin.set_fact: ...skipping... +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml @@ -1,17 +1,21 @@ --- -- name: Debian-specific setup - ansible.builtin.include_tasks: debian/main.yml - when: ansible_facts['os_family']|lower == 'debian' +- name: Set libguestfs runtime dependencies + ansible.builtin.set_fact: + bringup_guestfs_libguestfs_rdepends: >- + {{ + { + 'debian': bringup_guestfs_libguestfs_rdepends_debian, + 'fedora': bringup_guestfs_libguestfs_rdepends_redhat, + 'redhat': bringup_guestfs_libguestfs_rdepends_redhat, + 'suse': bringup_guestfs_libguestfs_rdepends_suse + }[ansible_facts['os_family'] | lower] + }} tags: ['install-deps'] -- name: SuSE-specific setup - ansible.builtin.include_tasks: suse/main.yml - when: ansible_facts['os_family']|lower == 'suse' - tags: ['install-deps'] - -- name: Fedora/Red Hat Enterprise-specific setup - ansible.builtin.include_tasks: redhat/main.yml - when: - - ansible_facts['os_family']|lower == 'redhat' - - ansible_facts['distribution']|lower != "fedora" +- name: Install libguestfs + become: true + become_method: ansible.builtin.sudo + ansible.builtin.package: + name: "{{ bringup_guestfs_libguestfs_rdepends }}" + state: present tags: ['install-deps'] diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml deleted file mode 100644 index 5e4bfa9..0000000 --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml +++ /dev/null @@ -1,11 +0,0 @@ ---- -- name: Install libguestfs on Fedora/RedHat - become: true - become_method: ansible.builtin.sudo - ansible.builtin.package: - update_cache: true - name: - - libguestfs-tools - - dhcpcd - state: present - tags: ['install-deps'] diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml deleted file mode 100644 index 730bc20..0000000 --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml +++ /dev/null @@ -1,10 +0,0 @@ ---- -- name: Install libguestfs on SUSE - become: true - become_method: ansible.builtin.sudo - ansible.builtin.package: - name: - - libguestfs-tools - - dhcpcd - state: present - tags: ['install-deps'] > > > > > That is, with that solution we should no thave to do the mapping of > > packages as I understand it. > > Quoting from [1]: > > "While ansible.builtin.package abstracts package managers to ease dealing with > multiple distributions, package name often differs for the same software." > > I think we can do things like "{{ libguestfs }}" and make the variable dependent > on the distro. But we still need to manage distro-specific package names. > > [1] https://docs.ansible.com/ansible/latest/collections/ansible/builtin/package_module.html > > > > > > Luis
On Fri, Mar 07, 2025 at 02:00:16PM +0100, Daniel Gomez wrote: > On Fri, Mar 07, 2025 at 09:48:30AM +0100, Daniel Gomez wrote: > > On Thu, Mar 06, 2025 at 06:13:32PM +0100, Luis Chamberlain wrote: > > > On Thu, Mar 06, 2025 at 07:58:09PM +0100, Daniel Gomez wrote: > > > > Ok, I replaced the specific package manager with the suggested one + renamed the > > > > dhcp clients for Red Hat and Suse using the names found in libguestfs [1]. > > > > > > The idea was not to use it on each distro's thing but rather *one* > > > single entry for all distros. But since that requires porting first the > > > list, I think for this patch we can keep the old apt / yum / etc and > > > later we can migrate the entries. > > > > Ok, I'll provide a diff. > > This works for me, let me know what you think: > > Note that Ansible Lint reported var-nameing[no-role-prefix] [1][2], so variables > use the bringup_guestfs_ prefix suggested. > > [1] var-naming[no-role-prefix]: Variables names from within roles should use role_name_ as a prefix. Underlines are accepted before the prefix. > [2] https://ansible.readthedocs.io/projects/lint/rules/var-naming/ > > diff --git a/playbooks/roles/bringup_guestfs/defaults/main.yml b/playbooks/roles/bringup_guestfs/defaults/main.yml > index 65867cd..a5bcb15 100644 > --- a/playbooks/roles/bringup_guestfs/defaults/main.yml > +++ b/playbooks/roles/bringup_guestfs/defaults/main.yml > @@ -10,3 +10,12 @@ distro_suse: False > distro_suse_based: False > distro_ubuntu: False > dnsmasq_files_exist: False > +bringup_guestfs_libguestfs_rdepends_debian: > + - libguestfs-tools > + - isc-dhcp-client > +bringup_guestfs_libguestfs_rdepends_suse: > + - libguestfs-tools > + - dhcpcd > +bringup_guestfs_libguestfs_rdepends_redhat: > + - libguestfs-tools > + - dhcpcd > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml > deleted file mode 100644 > index 6b502f5..0000000 > --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml > +++ /dev/null > @@ -1,11 +0,0 @@ > ---- > -- name: Install libguestfs > - become: true > - become_method: ansible.builtin.sudo > - ansible.builtin.package: > - update_cache: true > - name: > - - libguestfs-tools > - - isc-dhcp-client > - state: present > - tags: ['install-deps'] > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > index af3be6d..790d446 100644 > --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > @@ -1,17 +1,21 @@ > --- > -- name: Debian-specific setup > - ansible.builtin.include_tasks: debian/main.yml > - when: ansible_facts['os_family']|lower == 'debian' > +- name: Set libguestfs dependencies based on OS family > + ansible.builtin.set_fact: > ...skipping... > +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > @@ -1,17 +1,21 @@ > --- > -- name: Debian-specific setup > - ansible.builtin.include_tasks: debian/main.yml > - when: ansible_facts['os_family']|lower == 'debian' > +- name: Set libguestfs runtime dependencies > + ansible.builtin.set_fact: > + bringup_guestfs_libguestfs_rdepends: >- > + {{ > + { > + 'debian': bringup_guestfs_libguestfs_rdepends_debian, > + 'fedora': bringup_guestfs_libguestfs_rdepends_redhat, > + 'redhat': bringup_guestfs_libguestfs_rdepends_redhat, > + 'suse': bringup_guestfs_libguestfs_rdepends_suse > + }[ansible_facts['os_family'] | lower] > + }} > tags: ['install-deps'] > > -- name: SuSE-specific setup > - ansible.builtin.include_tasks: suse/main.yml > - when: ansible_facts['os_family']|lower == 'suse' > - tags: ['install-deps'] > - > -- name: Fedora/Red Hat Enterprise-specific setup > - ansible.builtin.include_tasks: redhat/main.yml > - when: > - - ansible_facts['os_family']|lower == 'redhat' > - - ansible_facts['distribution']|lower != "fedora" > +- name: Install libguestfs > + become: true > + become_method: ansible.builtin.sudo > + ansible.builtin.package: > + name: "{{ bringup_guestfs_libguestfs_rdepends }}" > + state: present > tags: ['install-deps'] > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml > deleted file mode 100644 > index 5e4bfa9..0000000 > --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml > +++ /dev/null > @@ -1,11 +0,0 @@ > ---- > -- name: Install libguestfs on Fedora/RedHat > - become: true > - become_method: ansible.builtin.sudo > - ansible.builtin.package: > - update_cache: true > - name: > - - libguestfs-tools > - - dhcpcd > - state: present > - tags: ['install-deps'] > diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml > deleted file mode 100644 > index 730bc20..0000000 > --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml > +++ /dev/null > @@ -1,10 +0,0 @@ > ---- > -- name: Install libguestfs on SUSE > - become: true > - become_method: ansible.builtin.sudo > - ansible.builtin.package: > - name: > - - libguestfs-tools > - - dhcpcd > - state: present > - tags: ['install-deps'] Ah, in that case this looks good. I had written a pkg role simply to deal with the odd fact that a debian package can be different in different versions of debian. The idea was to stuff the generic variables into group_vars/all in this case the first one was pkg_libaio and then if you *know* that the differ you just: +- include_role: + name: pkg Refer to commit 0639d617282a ("roles: add new pkg role") so later on (it doesn't have to be now), we can slowly start moving generic package names to group_vars/all and import this when we know that there is a difference. Or another way to put it -- if we know the name for a package is the same for all distros we don't use a variable name, but once we do know a package name does change depending on the disro we can use this? That is, in terms of scaling longeterm, I am not sure if it makes sense to be adding variable names for a group of dependencies per distro for say guestfs, but a one-to-one mapping of package names seems generally useful. Because then if you need these deps and mappings for other stuff, you just use the same name. Thoughts? Luis
On 3/7/25 12:15 PM, Luis Chamberlain wrote: > On Fri, Mar 07, 2025 at 02:00:16PM +0100, Daniel Gomez wrote: >> On Fri, Mar 07, 2025 at 09:48:30AM +0100, Daniel Gomez wrote: >>> On Thu, Mar 06, 2025 at 06:13:32PM +0100, Luis Chamberlain wrote: >>>> On Thu, Mar 06, 2025 at 07:58:09PM +0100, Daniel Gomez wrote: >>>>> Ok, I replaced the specific package manager with the suggested one + renamed the >>>>> dhcp clients for Red Hat and Suse using the names found in libguestfs [1]. >>>> >>>> The idea was not to use it on each distro's thing but rather *one* >>>> single entry for all distros. But since that requires porting first the >>>> list, I think for this patch we can keep the old apt / yum / etc and >>>> later we can migrate the entries. >>> >>> Ok, I'll provide a diff. >> >> This works for me, let me know what you think: >> >> Note that Ansible Lint reported var-nameing[no-role-prefix] [1][2], so variables >> use the bringup_guestfs_ prefix suggested. >> >> [1] var-naming[no-role-prefix]: Variables names from within roles should use role_name_ as a prefix. Underlines are accepted before the prefix. >> [2] https://ansible.readthedocs.io/projects/lint/rules/var-naming/ >> >> diff --git a/playbooks/roles/bringup_guestfs/defaults/main.yml b/playbooks/roles/bringup_guestfs/defaults/main.yml >> index 65867cd..a5bcb15 100644 >> --- a/playbooks/roles/bringup_guestfs/defaults/main.yml >> +++ b/playbooks/roles/bringup_guestfs/defaults/main.yml >> @@ -10,3 +10,12 @@ distro_suse: False >> distro_suse_based: False >> distro_ubuntu: False >> dnsmasq_files_exist: False >> +bringup_guestfs_libguestfs_rdepends_debian: >> + - libguestfs-tools >> + - isc-dhcp-client >> +bringup_guestfs_libguestfs_rdepends_suse: >> + - libguestfs-tools >> + - dhcpcd >> +bringup_guestfs_libguestfs_rdepends_redhat: >> + - libguestfs-tools >> + - dhcpcd >> diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml >> deleted file mode 100644 >> index 6b502f5..0000000 >> --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml >> +++ /dev/null >> @@ -1,11 +0,0 @@ >> ---- >> -- name: Install libguestfs >> - become: true >> - become_method: ansible.builtin.sudo >> - ansible.builtin.package: >> - update_cache: true >> - name: >> - - libguestfs-tools >> - - isc-dhcp-client >> - state: present >> - tags: ['install-deps'] >> diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml >> index af3be6d..790d446 100644 >> --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml >> +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml >> @@ -1,17 +1,21 @@ >> --- >> -- name: Debian-specific setup >> - ansible.builtin.include_tasks: debian/main.yml >> - when: ansible_facts['os_family']|lower == 'debian' >> +- name: Set libguestfs dependencies based on OS family >> + ansible.builtin.set_fact: >> ...skipping... >> +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml >> @@ -1,17 +1,21 @@ >> --- >> -- name: Debian-specific setup >> - ansible.builtin.include_tasks: debian/main.yml >> - when: ansible_facts['os_family']|lower == 'debian' >> +- name: Set libguestfs runtime dependencies >> + ansible.builtin.set_fact: >> + bringup_guestfs_libguestfs_rdepends: >- >> + {{ >> + { >> + 'debian': bringup_guestfs_libguestfs_rdepends_debian, >> + 'fedora': bringup_guestfs_libguestfs_rdepends_redhat, >> + 'redhat': bringup_guestfs_libguestfs_rdepends_redhat, >> + 'suse': bringup_guestfs_libguestfs_rdepends_suse >> + }[ansible_facts['os_family'] | lower] >> + }} >> tags: ['install-deps'] >> >> -- name: SuSE-specific setup >> - ansible.builtin.include_tasks: suse/main.yml >> - when: ansible_facts['os_family']|lower == 'suse' >> - tags: ['install-deps'] >> - >> -- name: Fedora/Red Hat Enterprise-specific setup >> - ansible.builtin.include_tasks: redhat/main.yml >> - when: >> - - ansible_facts['os_family']|lower == 'redhat' >> - - ansible_facts['distribution']|lower != "fedora" >> +- name: Install libguestfs >> + become: true >> + become_method: ansible.builtin.sudo >> + ansible.builtin.package: >> + name: "{{ bringup_guestfs_libguestfs_rdepends }}" >> + state: present >> tags: ['install-deps'] >> diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml >> deleted file mode 100644 >> index 5e4bfa9..0000000 >> --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml >> +++ /dev/null >> @@ -1,11 +0,0 @@ >> ---- >> -- name: Install libguestfs on Fedora/RedHat >> - become: true >> - become_method: ansible.builtin.sudo >> - ansible.builtin.package: >> - update_cache: true >> - name: >> - - libguestfs-tools >> - - dhcpcd >> - state: present >> - tags: ['install-deps'] >> diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml >> deleted file mode 100644 >> index 730bc20..0000000 >> --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml >> +++ /dev/null >> @@ -1,10 +0,0 @@ >> ---- >> -- name: Install libguestfs on SUSE >> - become: true >> - become_method: ansible.builtin.sudo >> - ansible.builtin.package: >> - name: >> - - libguestfs-tools >> - - dhcpcd >> - state: present >> - tags: ['install-deps'] > > Ah, in that case this looks good. I had written a pkg role > simply to deal with the odd fact that a debian package can be different > in different versions of debian. The idea was to stuff the generic > variables into group_vars/all in this case the first one was pkg_libaio > and then if you *know* that the differ you just: > > +- include_role: > + name: pkg > > Refer to commit 0639d617282a ("roles: add new pkg role") so later on > (it doesn't have to be now), we can slowly start moving generic package > names to group_vars/all and import this when we know that there is a > difference. > > Or another way to put it -- if we know the name for a package is the > same for all distros we don't use a variable name, but once we do know > a package name does change depending on the disro we can use this? > > That is, in terms of scaling longeterm, I am not sure if it makes sense > to be adding variable names for a group of dependencies per distro for > say guestfs, but a one-to-one mapping of package names seems generally useful. > Because then if you need these deps and mappings for other stuff, you > just use the same name. > > Thoughts? > > Luis > Just responding off the top of my head without having looked at "pkg", I think that's very close to the same approach I have taken with workflows I've written recently -- a vars/ directory that includes a file for each distro, and one invocation of ansible.builtin.package. Sometimes that doesn't work, though, if one of the distros needs to have some special attributes set on the package module.
On Fri, Mar 07, 2025 at 01:30:43PM +0100, Chuck Lever wrote: > On 3/7/25 12:15 PM, Luis Chamberlain wrote: > > On Fri, Mar 07, 2025 at 02:00:16PM +0100, Daniel Gomez wrote: > >> On Fri, Mar 07, 2025 at 09:48:30AM +0100, Daniel Gomez wrote: > >>> On Thu, Mar 06, 2025 at 06:13:32PM +0100, Luis Chamberlain wrote: > >>>> On Thu, Mar 06, 2025 at 07:58:09PM +0100, Daniel Gomez wrote: > >>>>> Ok, I replaced the specific package manager with the suggested one + renamed the > >>>>> dhcp clients for Red Hat and Suse using the names found in libguestfs [1]. > >>>> > >>>> The idea was not to use it on each distro's thing but rather *one* > >>>> single entry for all distros. But since that requires porting first the > >>>> list, I think for this patch we can keep the old apt / yum / etc and > >>>> later we can migrate the entries. > >>> > >>> Ok, I'll provide a diff. > >> > >> This works for me, let me know what you think: > >> > >> Note that Ansible Lint reported var-nameing[no-role-prefix] [1][2], so variables > >> use the bringup_guestfs_ prefix suggested. > >> > >> [1] var-naming[no-role-prefix]: Variables names from within roles should use role_name_ as a prefix. Underlines are accepted before the prefix. > >> [2] https://ansible.readthedocs.io/projects/lint/rules/var-naming/ > >> > >> diff --git a/playbooks/roles/bringup_guestfs/defaults/main.yml b/playbooks/roles/bringup_guestfs/defaults/main.yml > >> index 65867cd..a5bcb15 100644 > >> --- a/playbooks/roles/bringup_guestfs/defaults/main.yml > >> +++ b/playbooks/roles/bringup_guestfs/defaults/main.yml > >> @@ -10,3 +10,12 @@ distro_suse: False > >> distro_suse_based: False > >> distro_ubuntu: False > >> dnsmasq_files_exist: False > >> +bringup_guestfs_libguestfs_rdepends_debian: > >> + - libguestfs-tools > >> + - isc-dhcp-client > >> +bringup_guestfs_libguestfs_rdepends_suse: > >> + - libguestfs-tools > >> + - dhcpcd > >> +bringup_guestfs_libguestfs_rdepends_redhat: > >> + - libguestfs-tools > >> + - dhcpcd > >> diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml > >> deleted file mode 100644 > >> index 6b502f5..0000000 > >> --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml > >> +++ /dev/null > >> @@ -1,11 +0,0 @@ > >> ---- > >> -- name: Install libguestfs > >> - become: true > >> - become_method: ansible.builtin.sudo > >> - ansible.builtin.package: > >> - update_cache: true > >> - name: > >> - - libguestfs-tools > >> - - isc-dhcp-client > >> - state: present > >> - tags: ['install-deps'] > >> diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > >> index af3be6d..790d446 100644 > >> --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > >> +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > >> @@ -1,17 +1,21 @@ > >> --- > >> -- name: Debian-specific setup > >> - ansible.builtin.include_tasks: debian/main.yml > >> - when: ansible_facts['os_family']|lower == 'debian' > >> +- name: Set libguestfs dependencies based on OS family > >> + ansible.builtin.set_fact: > >> ...skipping... > >> +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml > >> @@ -1,17 +1,21 @@ > >> --- > >> -- name: Debian-specific setup > >> - ansible.builtin.include_tasks: debian/main.yml > >> - when: ansible_facts['os_family']|lower == 'debian' > >> +- name: Set libguestfs runtime dependencies > >> + ansible.builtin.set_fact: > >> + bringup_guestfs_libguestfs_rdepends: >- > >> + {{ > >> + { > >> + 'debian': bringup_guestfs_libguestfs_rdepends_debian, > >> + 'fedora': bringup_guestfs_libguestfs_rdepends_redhat, > >> + 'redhat': bringup_guestfs_libguestfs_rdepends_redhat, > >> + 'suse': bringup_guestfs_libguestfs_rdepends_suse > >> + }[ansible_facts['os_family'] | lower] > >> + }} > >> tags: ['install-deps'] > >> > >> -- name: SuSE-specific setup > >> - ansible.builtin.include_tasks: suse/main.yml > >> - when: ansible_facts['os_family']|lower == 'suse' > >> - tags: ['install-deps'] > >> - > >> -- name: Fedora/Red Hat Enterprise-specific setup > >> - ansible.builtin.include_tasks: redhat/main.yml > >> - when: > >> - - ansible_facts['os_family']|lower == 'redhat' > >> - - ansible_facts['distribution']|lower != "fedora" > >> +- name: Install libguestfs > >> + become: true > >> + become_method: ansible.builtin.sudo > >> + ansible.builtin.package: > >> + name: "{{ bringup_guestfs_libguestfs_rdepends }}" > >> + state: present > >> tags: ['install-deps'] > >> diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml > >> deleted file mode 100644 > >> index 5e4bfa9..0000000 > >> --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml > >> +++ /dev/null > >> @@ -1,11 +0,0 @@ > >> ---- > >> -- name: Install libguestfs on Fedora/RedHat > >> - become: true > >> - become_method: ansible.builtin.sudo > >> - ansible.builtin.package: > >> - update_cache: true > >> - name: > >> - - libguestfs-tools > >> - - dhcpcd > >> - state: present > >> - tags: ['install-deps'] > >> diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml > >> deleted file mode 100644 > >> index 730bc20..0000000 > >> --- a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml > >> +++ /dev/null > >> @@ -1,10 +0,0 @@ > >> ---- > >> -- name: Install libguestfs on SUSE > >> - become: true > >> - become_method: ansible.builtin.sudo > >> - ansible.builtin.package: > >> - name: > >> - - libguestfs-tools > >> - - dhcpcd > >> - state: present > >> - tags: ['install-deps'] > > > > Ah, in that case this looks good. I had written a pkg role > > simply to deal with the odd fact that a debian package can be different > > in different versions of debian. The idea was to stuff the generic > > variables into group_vars/all in this case the first one was pkg_libaio > > and then if you *know* that the differ you just: > > > > +- include_role: > > + name: pkg > > > > Refer to commit 0639d617282a ("roles: add new pkg role") so later on > > (it doesn't have to be now), we can slowly start moving generic package > > names to group_vars/all and import this when we know that there is a > > difference. > > > > Or another way to put it -- if we know the name for a package is the > > same for all distros we don't use a variable name, but once we do know > > a package name does change depending on the disro we can use this? > > > > That is, in terms of scaling longeterm, I am not sure if it makes sense > > to be adding variable names for a group of dependencies per distro for > > say guestfs, but a one-to-one mapping of package names seems generally useful. > > Because then if you need these deps and mappings for other stuff, you > > just use the same name. > > > > Thoughts? > > > > Luis > > > > Just responding off the top of my head without having looked at "pkg", > I think that's very close to the same approach I have taken with > workflows I've written recently -- a vars/ directory that includes > a file for each distro, and one invocation of ansible.builtin.package. > > Sometimes that doesn't work, though, if one of the distros needs to > have some special attributes set on the package module. > > > -- > Chuck Lever Besides that, the dependency list may not be the same for all distros. I'm not referring to the case above with runtime dependencies that should be detected and installed by the package manager, but rather to the ones we build ourselves for playbook/workflow purposes (e.g. fstests). Daniel
diff --git a/Makefile.min_deps b/Makefile.min_deps index a1cb99a500575807042fdd7c47b1b192df5db1bd..9d8f205799c463ca85dd80cff7a813ded5b11eea 100644 --- a/Makefile.min_deps +++ b/Makefile.min_deps @@ -10,11 +10,6 @@ BINARY_DEPS += sudo BINARY_DEPS += make BINARY_DEPS += nc BINARY_DEPS += ansible-playbook -ifneq (y,$(CONFIG_KDEVOPS_FIRST_RUN)) -ifeq (y,$(CONFIG_GUESTFS)) -BINARY_DEPS += virt-builder -endif -endif BINARY_DEPS_VCHECKS := $(subst $(TOPDIR)/,,$(wildcard $(TOPDIR)/scripts/version_check/*)) diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml new file mode 100644 index 0000000000000000000000000000000000000000..9fbfb4703fd268500f643b4522b872a70b0c1808 --- /dev/null +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/debian/main.yml @@ -0,0 +1,11 @@ +--- +- name: Install libguestfs + become: true + become_method: ansible.builtin.sudo + ansible.builtin.apt: + update_cache: true + name: + - libguestfs-tools + - isc-dhcp-client + state: present + tags: ['install-deps'] diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml new file mode 100644 index 0000000000000000000000000000000000000000..af3be6dea3c3445ecca4f3b3302e4640272882c6 --- /dev/null +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/main.yml @@ -0,0 +1,17 @@ +--- +- name: Debian-specific setup + ansible.builtin.include_tasks: debian/main.yml + when: ansible_facts['os_family']|lower == 'debian' + tags: ['install-deps'] + +- name: SuSE-specific setup + ansible.builtin.include_tasks: suse/main.yml + when: ansible_facts['os_family']|lower == 'suse' + tags: ['install-deps'] + +- name: Fedora/Red Hat Enterprise-specific setup + ansible.builtin.include_tasks: redhat/main.yml + when: + - ansible_facts['os_family']|lower == 'redhat' + - ansible_facts['distribution']|lower != "fedora" + tags: ['install-deps'] diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml new file mode 100644 index 0000000000000000000000000000000000000000..029b76a2f34927718b36c50080d330f80ee283ba --- /dev/null +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/redhat/main.yml @@ -0,0 +1,11 @@ +--- +- name: Install libguestfs on Fedora/RedHat + become: true + become_method: ansible.builtin.sudo + ansible.builtin.yum: + update_cache: true + name: + - libguestfs-tools + - dhclient + state: present + tags: ['install-deps'] diff --git a/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml new file mode 100644 index 0000000000000000000000000000000000000000..9aa5178fd972b90d008f01e9dc127539608feb15 --- /dev/null +++ b/playbooks/roles/bringup_guestfs/tasks/install-deps/suse/main.yml @@ -0,0 +1,10 @@ +--- +- name: Install libguestfs on SUSE + become: true + become_method: ansible.builtin.sudo + ansible.builtin.zypper: + name: + - libguestfs-tools + - dhclient + state: present + tags: ['install-deps'] diff --git a/playbooks/roles/bringup_guestfs/tasks/main.yml b/playbooks/roles/bringup_guestfs/tasks/main.yml index 8e0f82de5d0b9fa958fd1107ca75827f1b394b09..dcbbaef02522ef561b9d990b74a3e7573d912afb 100644 --- a/playbooks/roles/bringup_guestfs/tasks/main.yml +++ b/playbooks/roles/bringup_guestfs/tasks/main.yml @@ -1,3 +1,7 @@ +- name: Install dependencies + ansible.builtin.include_tasks: install-deps/main.yml + tags: ['install-deps'] + - name: Verify we're configured {{ topdir_path }}/.config directory exists stat: path: "{{ topdir_path }}/.config" diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile index 52397d54b89b584b675cf6182113d5a7e22da3c3..a8aa82fa96fa5bd9c181f10ef347bff8e3f78f44 100644 --- a/scripts/guestfs.Makefile +++ b/scripts/guestfs.Makefile @@ -47,6 +47,7 @@ ANSIBLE_EXTRA_ARGS += $(GUESTFS_ARGS) GUESTFS_BRINGUP_DEPS := GUESTFS_BRINGUP_DEPS += $(9P_HOST_CLONE) GUESTFS_BRINGUP_DEPS += $(LIBVIRT_PCIE_PASSTHROUGH) +GUESTFS_BRINGUP_DEPS += install_libguestfs KDEVOPS_PROVISION_METHOD := bringup_guestfs KDEVOPS_PROVISION_DESTROY_METHOD := destroy_guestfs @@ -72,6 +73,14 @@ $(KDEVOPS_PROVISIONED_SSH): $(Q)ansible $(ANSIBLE_VERBOSE) -i hosts all -e 'ansible_python_interpreter=/usr/bin/python3' -m wait_for_connection $(Q)touch $(KDEVOPS_PROVISIONED_SSH) +install_libguestfs: + $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ + --inventory localhost, \ + playbooks/bringup_guestfs.yml \ + -e 'ansible_python_interpreter=/usr/bin/python3' \ + --extra-vars=@./extra_vars.yaml \ + --tags install-deps + bringup_guestfs: $(GUESTFS_BRINGUP_DEPS) $(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \ --inventory localhost, \