[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [jenkins-ci PATCH 5/8] guests: Drop install_config from inventory



On Thu, Jul 19, 2018 at 06:32:05PM +0200, Andrea Bolognani wrote:
> The information is mostly duplicated and can be easily
> inferred in a programmatic manner, so storing it in the
> inventory is far from the cleanest solution.
>
> As a side-effect, we reintroduce the error message that
> was supposed to be displayed when attempting to install
> a FreeBSD guest but was lost in the Python rewrite.
>
> Signed-off-by: Andrea Bolognani <abologna redhat com>
> ---
>  guests/host_vars/libvirt-centos-7/install.yml |  1 -
>  guests/host_vars/libvirt-debian-8/install.yml |  1 -
>  guests/host_vars/libvirt-debian-9/install.yml |  1 -
>  .../host_vars/libvirt-debian-sid/install.yml  |  1 -
>  .../host_vars/libvirt-fedora-27/install.yml   |  1 -
>  .../host_vars/libvirt-fedora-28/install.yml   |  1 -
>  .../libvirt-fedora-rawhide/install.yml        |  1 -
>  .../host_vars/libvirt-ubuntu-16/install.yml   |  1 -
>  .../host_vars/libvirt-ubuntu-18/install.yml   |  1 -
>  guests/lcitool                                | 19 ++++++++++++++-----
>  10 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/guests/host_vars/libvirt-centos-7/install.yml b/guests/host_vars/libvirt-centos-7/install.yml
> index f003b89..2164ac5 100644
> --- a/guests/host_vars/libvirt-centos-7/install.yml
> +++ b/guests/host_vars/libvirt-centos-7/install.yml
> @@ -1,3 +1,2 @@
>  ---
>  install_url: http://mirror.centos.org/centos/7/os/x86_64/
> -install_config: kickstart.cfg
> diff --git a/guests/host_vars/libvirt-debian-8/install.yml b/guests/host_vars/libvirt-debian-8/install.yml
> index a2c8341..299a1a6 100644
> --- a/guests/host_vars/libvirt-debian-8/install.yml
> +++ b/guests/host_vars/libvirt-debian-8/install.yml
> @@ -1,3 +1,2 @@
>  ---
>  install_url: http://deb.debian.org/debian/dists/jessie/main/installer-amd64/
> -install_config: preseed.cfg
> diff --git a/guests/host_vars/libvirt-debian-9/install.yml b/guests/host_vars/libvirt-debian-9/install.yml
> index 5b1da76..7641753 100644
> --- a/guests/host_vars/libvirt-debian-9/install.yml
> +++ b/guests/host_vars/libvirt-debian-9/install.yml
> @@ -1,3 +1,2 @@
>  ---
>  install_url: http://deb.debian.org/debian/dists/stretch/main/installer-amd64/
> -install_config: preseed.cfg
> diff --git a/guests/host_vars/libvirt-debian-sid/install.yml b/guests/host_vars/libvirt-debian-sid/install.yml
> index da1c7a8..46c6366 100644
> --- a/guests/host_vars/libvirt-debian-sid/install.yml
> +++ b/guests/host_vars/libvirt-debian-sid/install.yml
> @@ -1,3 +1,2 @@
>  ---
>  install_url: http://deb.debian.org/debian/dists/sid/main/installer-amd64/
> -install_config: preseed.cfg
> diff --git a/guests/host_vars/libvirt-fedora-27/install.yml b/guests/host_vars/libvirt-fedora-27/install.yml
> index 66ce38e..f7a45af 100644
> --- a/guests/host_vars/libvirt-fedora-27/install.yml
> +++ b/guests/host_vars/libvirt-fedora-27/install.yml
> @@ -1,3 +1,2 @@
>  ---
>  install_url: https://download.fedoraproject.org/pub/fedora/linux/releases/27/Everything/x86_64/os
> -install_config: kickstart.cfg
> diff --git a/guests/host_vars/libvirt-fedora-28/install.yml b/guests/host_vars/libvirt-fedora-28/install.yml
> index 4b2b9f0..73433f1 100644
> --- a/guests/host_vars/libvirt-fedora-28/install.yml
> +++ b/guests/host_vars/libvirt-fedora-28/install.yml
> @@ -1,3 +1,2 @@
>  ---
>  install_url: https://download.fedoraproject.org/pub/fedora/linux/releases/28/Everything/x86_64/os
> -install_config: kickstart.cfg
> diff --git a/guests/host_vars/libvirt-fedora-rawhide/install.yml b/guests/host_vars/libvirt-fedora-rawhide/install.yml
> index 2216e81..5c67562 100644
> --- a/guests/host_vars/libvirt-fedora-rawhide/install.yml
> +++ b/guests/host_vars/libvirt-fedora-rawhide/install.yml
> @@ -1,3 +1,2 @@
>  ---
>  install_url: https://download.fedoraproject.org/pub/fedora/linux/development/rawhide/Everything/x86_64/os
> -install_config: kickstart.cfg
> diff --git a/guests/host_vars/libvirt-ubuntu-16/install.yml b/guests/host_vars/libvirt-ubuntu-16/install.yml
> index a7bb2da..d8ce841 100644
> --- a/guests/host_vars/libvirt-ubuntu-16/install.yml
> +++ b/guests/host_vars/libvirt-ubuntu-16/install.yml
> @@ -1,3 +1,2 @@
>  ---
>  install_url: http://archive.ubuntu.com/ubuntu/dists/xenial/main/installer-amd64/
> -install_config: preseed.cfg
> diff --git a/guests/host_vars/libvirt-ubuntu-18/install.yml b/guests/host_vars/libvirt-ubuntu-18/install.yml
> index bd3e1d9..544b3f2 100644
> --- a/guests/host_vars/libvirt-ubuntu-18/install.yml
> +++ b/guests/host_vars/libvirt-ubuntu-18/install.yml
> @@ -1,3 +1,2 @@
>  ---
>  install_url: http://archive.ubuntu.com/ubuntu/dists/bionic/main/installer-amd64/
> -install_config: preseed.cfg
> diff --git a/guests/lcitool b/guests/lcitool
> index 2cfb0e9..13f0392 100755
> --- a/guests/lcitool
> +++ b/guests/lcitool
> @@ -382,15 +382,24 @@ class Application:
>                  facts["install_network"],
>              )
>
> -            install_config = os.path.join(base, facts["install_config"])
> +            # Different operating systems require different configuration
> +            # files for unattended installation to work, but some operating
> +            # systems simply don't support unattended installation at all
> +            if facts["os_name"] in ["Debian", "Ubuntu"]:
> +                install_config = "preseed.cfg"
> +            elif facts["os_name"] in ["CentOS", "Fedora"]:
> +                install_config = "kickstart.cfg"
> +            else:
> +                raise Error(
> +                    "Host {} doesn't support installation".format(host)
> +                )
> +            initrd_inject = os.path.join(base, install_config)
>
>              # preseed files must use a well-known name to be picked up by
>              # d-i; for kickstart files, we can use whatever name we please
>              # but we need to point anaconda in the right direction through
>              # a kernel argument
> -            extra_arg = "console=ttyS0 ks=file:/{}".format(
> -                facts["install_config"],
> -            )
> +            extra_arg = "console=ttyS0 ks=file:/{}".format(install_config)

Pre-existing, but ks= should be replaced by inst.ks, as using the inst. prefix
is the current preferred way of passing installer-specific options, the legacy
syntax is still accepted, however, this may be a subject to future changes -
the inst. prefix is supported since fedora 17 and RHEL/CentOS 7 and since we
dropped CentOS 6 from the supported distros, we don't need to keep the legacy
option :).


The patch is fine though.
Reviewed-by: Erik Skultety <eskultet redhat com>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]