[libvirt] [jenkins-ci PATCH v2 00/12] lcitool: Rewrite in Python (and add Dockefile generator)

Read the initial cover letter for background and motivations. Changes from [v1]: * add Dockerfile generator; * rename the 'list' action to 'hosts' to better fit along with the additional 'projects' action; * always list items in alphabetical order; * move some generic functions to an Util class. [v1] https://www.redhat.com/archives/libvir-list/2018-July/msg00717.html Andrea Bolognani (12): lcitool: Drop shell implementation lcitool: Stub out Python implementation lcitool: Add tool configuration handling lcitool: Add inventory handling lcitool: Implement the 'hosts' action lcitool: Implement the 'install' action lcitool: Implement the 'update' action guests: Update documentation guests: Add Docker-related information to the inventory lcitool: Add projects information handling lcitool: Implement the 'projects' action lcitool: Implement the 'dockerfile' action guests/README.markdown | 8 +- guests/host_vars/libvirt-centos-7/docker.yml | 2 + guests/host_vars/libvirt-debian-8/docker.yml | 2 + guests/host_vars/libvirt-debian-9/docker.yml | 2 + .../host_vars/libvirt-debian-sid/docker.yml | 2 + guests/host_vars/libvirt-fedora-27/docker.yml | 2 + guests/host_vars/libvirt-fedora-28/docker.yml | 2 + .../libvirt-fedora-rawhide/docker.yml | 2 + guests/host_vars/libvirt-ubuntu-16/docker.yml | 2 + guests/host_vars/libvirt-ubuntu-18/docker.yml | 2 + guests/lcitool | 722 ++++++++++++------ 11 files changed, 521 insertions(+), 227 deletions(-) create mode 100644 guests/host_vars/libvirt-centos-7/docker.yml create mode 100644 guests/host_vars/libvirt-debian-8/docker.yml create mode 100644 guests/host_vars/libvirt-debian-9/docker.yml create mode 100644 guests/host_vars/libvirt-debian-sid/docker.yml create mode 100644 guests/host_vars/libvirt-fedora-27/docker.yml create mode 100644 guests/host_vars/libvirt-fedora-28/docker.yml create mode 100644 guests/host_vars/libvirt-fedora-rawhide/docker.yml create mode 100644 guests/host_vars/libvirt-ubuntu-16/docker.yml create mode 100644 guests/host_vars/libvirt-ubuntu-18/docker.yml -- 2.17.1

We're going to rewrite the script completely, and getting the current implementation out of the way firts will make the diffs more reasonable. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 227 ------------------------------------------------- 1 file changed, 227 deletions(-) delete mode 100755 guests/lcitool diff --git a/guests/lcitool b/guests/lcitool deleted file mode 100755 index 0c1520e..0000000 --- a/guests/lcitool +++ /dev/null @@ -1,227 +0,0 @@ -#!/bin/sh - -# ------------------- -# Utility functions -# ------------------- - -# die MESSAGE -# -# Abort the program after displaying $MESSAGE on standard error. -die() { - echo "$1" >&2 - exit 1 -} - -# hash_file PASS_FILE -# -# Generate a password hash from the contents of PASS_FILE. -hash_file() { - PASS_FILE="$1" - - perl -le ' - my @chars = ("A".."Z", "a".."z", "0".."9"); - my $salt; $salt .= $chars[rand @chars] for 1..16; - my $handle; open($handle, "'"$PASS_FILE"'"); - my $pass = <$handle>; chomp($pass); - print crypt("$pass", "\$6\$$salt\$");' -} - -# yaml_var FILE VAR -# -# Read $FILE and output the value of YAML variable $VAR. Only trivial YAML -# values are supported, eg. strings and numbers that don't depend on the -# value of other variables. That's enough for our use case. -yaml_var() { - grep "^$2:\\s*" "$1" 2>/dev/null | tail -1 | sed "s/$2:\\s*//g" -} - -# load_install_config FILE -# -# Read all known configuration variables from $FILE and set them in the -# environment. Configuration variables that have already been set in -# the environment will not be updated. -load_install_config() { - INSTALL_URL=${INSTALL_URL:-$(yaml_var "$1" install_url)} - INSTALL_CONFIG=${INSTALL_CONFIG:-$(yaml_var "$1" install_config)} - INSTALL_VIRT_TYPE=${INSTALL_VIRT_TYPE:-$(yaml_var "$1" install_virt_type)} - INSTALL_ARCH=${INSTALL_ARCH:-$(yaml_var "$1" install_arch)} - INSTALL_MACHINE=${INSTALL_MACHINE:-$(yaml_var "$1" install_machine)} - INSTALL_CPU_MODEL=${INSTALL_CPU_MODEL:-$(yaml_var "$1" install_cpu_model)} - INSTALL_VCPUS=${INSTALL_VCPUS:-$(yaml_var "$1" install_vcpus)} - INSTALL_MEMORY_SIZE=${INSTALL_MEMORY_SIZE:-$(yaml_var "$1" install_memory_size)} - INSTALL_DISK_SIZE=${INSTALL_DISK_SIZE:-$(yaml_var "$1" install_disk_size)} - INSTALL_STORAGE_POOL=${INSTALL_STORAGE_POOL:-$(yaml_var "$1" install_storage_pool)} - INSTALL_NETWORK=${INSTALL_NETWORK:-$(yaml_var "$1" install_network)} -} - -# load_config -# -# Read tool configuration and perform the necessary validation. -load_config() { - CONFIG_DIR="$HOME/.config/$PROGRAM_NAME" - - mkdir -p "$CONFIG_DIR" >/dev/null 2>&1 || { - die "$PROGRAM_NAME: $CONFIG_DIR: Unable to create config directory" - } - - FLAVOR_FILE="$CONFIG_DIR/flavor" - VAULT_PASS_FILE="$CONFIG_DIR/vault-password" - ROOT_PASS_FILE="$CONFIG_DIR/root-password" - - # Two flavors are supported: test (default) and jenkins. Read the - # flavor from configuration, validate it and write it back in case - # it was not present - FLAVOR="$(cat "$FLAVOR_FILE" 2>/dev/null)" - FLAVOR=${FLAVOR:-test} - test "$FLAVOR" = test || test "$FLAVOR" = jenkins || { - die "$PROGRAM_NAME: Invalid flavor '$FLAVOR'" - } - echo "$FLAVOR" >"$FLAVOR_FILE" || { - die "$PROGRAM_NAME: $FLAVOR_FILE: Unable to save flavor" - } - - test "$FLAVOR" = jenkins && { - # The vault password is only needed for the jenkins flavor, so only - # validate it in that case - test -f "$VAULT_PASS_FILE" && test "$(cat "$VAULT_PASS_FILE")" || { - die "$PROGRAM_NAME: $VAULT_PASS_FILE: Missing or invalid password" - } - } || { - # For other flavors, undefine the variable so that Ansible - # will not try to read the file at all - VAULT_PASS_FILE= - } - - # Make sure the root password has been configured properly - test -f "$ROOT_PASS_FILE" && test "$(cat "$ROOT_PASS_FILE")" || { - die "$PROGRAM_NAME: $ROOT_PASS_FILE: Missing or invalid password" - } - - ROOT_HASH_FILE="$CONFIG_DIR/.root-password.hash" - - # Regenerate root password hash. Ansible expects passwords as hashes but - # doesn't provide a built-in facility to generate one from plain text - hash_file "$ROOT_PASS_FILE" >"$ROOT_HASH_FILE" || { - die "$PROGRAM_NAME: Failure while hashing root password" - } -} - -# ---------------------- -# User-visible actions -# ---------------------- - -do_help() { - echo "\ -Usage: $CALL_NAME ACTION [OPTIONS] - -Actions: - list List known guests - install GUEST Install GUEST - prepare GUEST|all Prepare or update GUEST. Can be run multiple times - update GUEST|all Alias for prepare - help Display this help" -} - -do_list() { - # List all guests present in the inventory. Skip group names, - # comments and empty lines - grep -vE '^#|^\[|^$' inventory | sort -u -} - -do_install() -{ - GUEST="$1" - - test "$GUEST" || { - die "$(do_help)" - } - do_list | grep -q "$GUEST" || { - die "$PROGRAM_NAME: $GUEST: Unknown guest" - } - test -f "host_vars/$GUEST/install.yml" || { - die "$PROGRAM_NAME: $GUEST: Missing configuration, guest must be installed manually" - } - - load_config - - # Load configuration files. Values don't get overwritten after being - # set the first time, so loading the host-specific configuration before - # the group configuration ensures overrides work as expected - load_install_config "host_vars/$GUEST/install.yml" - load_install_config "group_vars/all/install.yml" - - # Both memory size and disk size use GiB as unit, but virt-install wants - # disk size in GiB and memory size in *MiB*, so perform conversion here - INSTALL_MEMORY_SIZE=$(expr "$INSTALL_MEMORY_SIZE" \* 1024 2>/dev/null) - - # 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 - case "$INSTALL_CONFIG" in - *kickstart*|*ks*) EXTRA_ARGS="ks=file:/${INSTALL_CONFIG##*/}" ;; - esac - - # Only configure autostart for the guest for the jenkins flavor - test "$FLAVOR" = jenkins && { - AUTOSTART="--autostart" - } - - virt-install \ - --name "$GUEST" \ - --location "$INSTALL_URL" \ - --virt-type "$INSTALL_VIRT_TYPE" \ - --arch "$INSTALL_ARCH" \ - --machine "$INSTALL_MACHINE" \ - --cpu "$INSTALL_CPU_MODEL" \ - --vcpus "$INSTALL_VCPUS" \ - --memory "$INSTALL_MEMORY_SIZE" \ - --disk "size=$INSTALL_DISK_SIZE,pool=$INSTALL_STORAGE_POOL,bus=virtio" \ - --network "network=$INSTALL_NETWORK,model=virtio" \ - --graphics none \ - --console pty \ - --sound none \ - --initrd-inject "$INSTALL_CONFIG" \ - --extra-args "console=ttyS0 $EXTRA_ARGS" \ - $AUTOSTART \ - --wait 0 -} - -do_prepare() { - GUEST="$1" - - test "$GUEST" || { - die "$(do_help)" - } - do_list | grep -q "$GUEST" || test "$GUEST" = all || { - die "$PROGRAM_NAME: $GUEST: Unknown guest" - } - - load_config - - EXTRA_VARS="flavor=$FLAVOR root_password_file=$ROOT_HASH_FILE" - - ansible-playbook \ - --vault-password-file "$VAULT_PASS_FILE" \ - --extra-vars "$EXTRA_VARS" \ - -l "$GUEST" \ - site.yml -} - -# --------------------- -# Program entry point -# --------------------- - -CALL_NAME="$0" -PROGRAM_NAME="${0##*/}" - -test -f "$PROGRAM_NAME" || { - die "$PROGRAM_NAME: Must be run from the source directory" -} - -case "$1" in - list) do_list ;; - install) do_install "$2" ;; - prepare|update) do_prepare "$2" ;; - *help) do_help ;; - *) die "$(do_help)" ;; -esac -- 2.17.1

Doesn't do much right now, but it's a start :) Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100755 guests/lcitool diff --git a/guests/lcitool b/guests/lcitool new file mode 100755 index 0000000..1cba8ad --- /dev/null +++ b/guests/lcitool @@ -0,0 +1,69 @@ +#!/usr/bin/env python + +# lcitool - libvirt CI guest management tool +# Copyright (C) 2017-2018 Andrea Bolognani <abologna@redhat.com> +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; either version 2 of the License, or (at your +# option) any later version. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +import argparse +import sys +import textwrap + +# This is necessary to maintain Python 2.7 compatibility +try: + import configparser +except ImportError: + import ConfigParser as configparser + +class Error(Exception): + + def __init__(self, message): + self.message = message + +class Application: + + def __init__(self): + self._parser = argparse.ArgumentParser( + conflict_handler = "resolve", + formatter_class = argparse.RawDescriptionHelpFormatter, + description = "libvirt CI guest management tool", + epilog = textwrap.dedent(""" + supported actions: + """), + ) + self._parser.add_argument( + "-a", + metavar = "ACTION", + required = True, + help = "action to perform (see below)", + ) + + def run(self): + cmdline = self._parser.parse_args() + action = cmdline.a + + method = "_action_{}".format(action.replace("-", "_")) + + if hasattr(self, method): + getattr(self, method).__call__() + else: + raise Error("Invalid action '{}'".format(action)) + +if __name__ == "__main__": + try: + Application().run() + except Error as e: + sys.stderr.write("{}: {}\n".format(sys.argv[0], e.message)) + sys.exit(1) -- 2.17.1

On Thu, Jul 12, 2018 at 05:19:19PM +0200, Andrea Bolognani wrote:
Doesn't do much right now, but it's a start :)
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100755 guests/lcitool
diff --git a/guests/lcitool b/guests/lcitool new file mode 100755 index 0000000..1cba8ad --- /dev/null +++ b/guests/lcitool @@ -0,0 +1,69 @@ +#!/usr/bin/env python + +# lcitool - libvirt CI guest management tool +# Copyright (C) 2017-2018 Andrea Bolognani <abologna@redhat.com> +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; either version 2 of the License, or (at your +# option) any later version. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +
Please use: You should have received a copy of the GNU General Public License along with this program. If not, see <https://www.gnu.org/licenses/>. Per the recommendation on https://www.gnu.org/licenses/gpl-howto.html instead of listing the physical address. Jano

On Thu, Jul 12, 2018 at 05:19:19PM +0200, Andrea Bolognani wrote:
Doesn't do much right now, but it's a start :)
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100755 guests/lcitool
diff --git a/guests/lcitool b/guests/lcitool new file mode 100755 index 0000000..1cba8ad --- /dev/null +++ b/guests/lcitool @@ -0,0 +1,69 @@ +#!/usr/bin/env python + +# lcitool - libvirt CI guest management tool +# Copyright (C) 2017-2018 Andrea Bolognani <abologna@redhat.com> +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; either version 2 of the License, or (at your +# option) any later version. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +import argparse +import sys +import textwrap + +# This is necessary to maintain Python 2.7 compatibility +try: + import configparser +except ImportError: + import ConfigParser as configparser
This import is unused here. Maybe we can introduce it at the point we actually start using it?
+ +class Error(Exception): + + def __init__(self, message): + self.message = message + +class Application: + + def __init__(self): + self._parser = argparse.ArgumentParser( + conflict_handler = "resolve", + formatter_class = argparse.RawDescriptionHelpFormatter, + description = "libvirt CI guest management tool", + epilog = textwrap.dedent(""" + supported actions: + """), + ) + self._parser.add_argument( + "-a", + metavar = "ACTION", + required = True, + help = "action to perform (see below)", + ) + + def run(self): + cmdline = self._parser.parse_args() + action = cmdline.a + + method = "_action_{}".format(action.replace("-", "_")) + + if hasattr(self, method): + getattr(self, method).__call__() + else: + raise Error("Invalid action '{}'".format(action)) + +if __name__ == "__main__": + try: + Application().run() + except Error as e: + sys.stderr.write("{}: {}\n".format(sys.argv[0], e.message)) + sys.exit(1) -- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
I am not very good with python myself, however whenever I am using it I try to follow coding style standards enforced by pep8 (currently renamed pycodestyle). So here is the output of pep8 for this script, feel free to ignore it if you don't agree though I encourage that we enforce pep8 for the python scripts here. $ pycodestyle guests/lcitool guests/lcitool:30:1: E302 expected 2 blank lines, found 1 guests/lcitool:35:1: E302 expected 2 blank lines, found 1 guests/lcitool:39:29: E251 unexpected spaces around keyword / parameter equals guests/lcitool:39:31: E251 unexpected spaces around keyword / parameter equals guests/lcitool:40:28: E251 unexpected spaces around keyword / parameter equals guests/lcitool:40:30: E251 unexpected spaces around keyword / parameter equals guests/lcitool:41:24: E251 unexpected spaces around keyword / parameter equals guests/lcitool:41:26: E251 unexpected spaces around keyword / parameter equals guests/lcitool:42:19: E251 unexpected spaces around keyword / parameter equals guests/lcitool:42:21: E251 unexpected spaces around keyword / parameter equals guests/lcitool:48:20: E251 unexpected spaces around keyword / parameter equals guests/lcitool:48:22: E251 unexpected spaces around keyword / parameter equals guests/lcitool:49:21: E251 unexpected spaces around keyword / parameter equals guests/lcitool:49:23: E251 unexpected spaces around keyword / parameter equals guests/lcitool:50:17: E251 unexpected spaces around keyword / parameter equals guests/lcitool:50:19: E251 unexpected spaces around keyword / parameter equals To actually run the pep8 you don't have to install it globally on your system, but you can create a virtualenv and install it there with "pip install pep8".

On Tue, 2018-07-17 at 11:16 +0200, Katerina Koukiou wrote:
On Thu, Jul 12, 2018 at 05:19:19PM +0200, Andrea Bolognani wrote:
+# This is necessary to maintain Python 2.7 compatibility +try: + import configparser +except ImportError: + import ConfigParser as configparser
This import is unused here. Maybe we can introduce it at the point we actually start using it?
That was the intention, but I apparently messed up during rebase :) [...]
I am not very good with python myself, however whenever I am using it I try to follow coding style standards enforced by pep8 (currently renamed pycodestyle).
So here is the output of pep8 for this script, feel free to ignore it if you don't agree though I encourage that we enforce pep8 for the python scripts here.
I don't necessarily like PEP8 (especially the part about using key=value for keyword arguments, eww) but it's an accepted standard with nice tooling built around it, so I agree we should strive for compilance. -- Andrea Bolognani / Red Hat / Virtualization

The on-disk configuration format and its behavior are fully backwards compatible with the previous implementation. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 112 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/guests/lcitool b/guests/lcitool index 1cba8ad..e03b388 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -18,6 +18,10 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import argparse +import crypt +import os +import random +import string import sys import textwrap @@ -32,9 +36,117 @@ class Error(Exception): def __init__(self, message): self.message = message +class Util: + + @staticmethod + def mksalt(): + alphabeth = string.ascii_letters + string.digits + salt = "".join(random.choice(alphabeth) for x in range(0, 16)) + return "$6${}$".format(salt) + +class Config: + + def _get_config_file(self, name): + try: + config_dir = os.environ["XDG_CONFIG_HOME"] + except KeyError: + config_dir = os.path.join(os.environ["HOME"], ".config/") + config_dir = os.path.join(config_dir, "lcitool/") + + # Create the directory if it doesn't already exist + if not os.path.exists(config_dir): + try: + os.mkdir(config_dir) + except: + raise Error( + "Can't create configuration directory ({})".format( + config_dir, + ) + ) + + return os.path.join(config_dir, name) + + def get_flavor(self): + flavor_file = self._get_config_file("flavor") + + try: + with open(flavor_file, "r") as f: + flavor = f.readline().strip() + except: + # If the flavor has not been configured, we choose the default + # and store it on disk to ensure consistent behavior from now on + flavor = "test" + try: + with open(flavor_file, "w") as f: + f.write("{}\n".format(flavor)) + except: + raise Error( + "Can't write flavor file ({})".format( + flavor_file, + ) + ) + + if flavor != "test" and flavor != "jenkins": + raise Error("Invalid flavor '{}'".format(flavor)) + + return flavor + + def get_vault_password_file(self): + vault_pass_file = None + + # The vault password is only needed for the jenkins flavor, but in + # that case we want to make sure there's *something* in there + if self.get_flavor() != "test": + vault_pass_file = self._get_config_file("vault-password") + + try: + with open(vault_pass_file, 'r') as f: + if len(f.readline().strip()) == 0: + raise ValueError + except: + raise Error( + "Missing or invalid vault password file ({})".format( + vault_pass_file, + ) + ) + + return vault_pass_file + + def get_root_password_file(self): + root_pass_file = self._get_config_file("root-password") + root_hash_file = self._get_config_file(".root-password.hash") + + try: + with open(root_pass_file, "r") as f: + root_pass = f.readline().strip() + except: + raise Error( + "Missing or invalid root password file ({})".format( + root_pass_file, + ) + ) + + # The hash will be different every time we run, but that doesn't + # matter - it will still validate the correct root password + root_hash = crypt.crypt(root_pass, Util.mksalt()) + + try: + with open(root_hash_file, "w") as f: + f.write("{}\n".format(root_hash)) + except: + raise Error( + "Can't write hashed root password file ({})".format( + root_hash_file, + ) + ) + + return root_hash_file + class Application: def __init__(self): + self._config = Config() + self._parser = argparse.ArgumentParser( conflict_handler = "resolve", formatter_class = argparse.RawDescriptionHelpFormatter, -- 2.17.1

On Thu, Jul 12, 2018 at 05:19:20PM +0200, Andrea Bolognani wrote:
The on-disk configuration format and its behavior are fully backwards compatible with the previous implementation.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 112 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+)
diff --git a/guests/lcitool b/guests/lcitool index 1cba8ad..e03b388 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -18,6 +18,10 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
import argparse +import crypt +import os +import random +import string import sys import textwrap
@@ -32,9 +36,117 @@ class Error(Exception): def __init__(self, message): self.message = message
+class Util: + + @staticmethod + def mksalt(): + alphabeth = string.ascii_letters + string.digits + salt = "".join(random.choice(alphabeth) for x in range(0, 16)) + return "$6${}$".format(salt) + +class Config: + + def _get_config_file(self, name): + try: + config_dir = os.environ["XDG_CONFIG_HOME"] + except KeyError: + config_dir = os.path.join(os.environ["HOME"], ".config/") + config_dir = os.path.join(config_dir, "lcitool/") + + # Create the directory if it doesn't already exist + if not os.path.exists(config_dir): + try: + os.mkdir(config_dir) + except: + raise Error( + "Can't create configuration directory ({})".format( + config_dir, + ) + ) + + return os.path.join(config_dir, name) + + def get_flavor(self): + flavor_file = self._get_config_file("flavor") + + try: + with open(flavor_file, "r") as f: + flavor = f.readline().strip() + except:
And it's generally discouraged to use a bare except because it catches BaseException. Be at least as specific as Exception, and here specifically, perhaps catch IOError.
+ # If the flavor has not been configured, we choose the default + # and store it on disk to ensure consistent behavior from now on + flavor = "test" + try: + with open(flavor_file, "w") as f: + f.write("{}\n".format(flavor)) + except: + raise Error( + "Can't write flavor file ({})".format( + flavor_file, + ) + ) + + if flavor != "test" and flavor != "jenkins":
More pythonic would be "flavor not in ('test', 'jenkins')"
+ raise Error("Invalid flavor '{}'".format(flavor)) + + return flavor + + def get_vault_password_file(self): + vault_pass_file = None + + # The vault password is only needed for the jenkins flavor, but in + # that case we want to make sure there's *something* in there + if self.get_flavor() != "test": + vault_pass_file = self._get_config_file("vault-password") + + try: + with open(vault_pass_file, 'r') as f: + if len(f.readline().strip()) == 0: + raise ValueError + except: + raise Error( + "Missing or invalid vault password file ({})".format( + vault_pass_file, + ) + ) + + return vault_pass_file + + def get_root_password_file(self): + root_pass_file = self._get_config_file("root-password") + root_hash_file = self._get_config_file(".root-password.hash") + + try: + with open(root_pass_file, "r") as f: + root_pass = f.readline().strip() + except: + raise Error( + "Missing or invalid root password file ({})".format( + root_pass_file, + ) + ) + + # The hash will be different every time we run, but that doesn't + # matter - it will still validate the correct root password + root_hash = crypt.crypt(root_pass, Util.mksalt()) + + try: + with open(root_hash_file, "w") as f: + f.write("{}\n".format(root_hash)) + except: + raise Error( + "Can't write hashed root password file ({})".format( + root_hash_file, + ) + ) + + return root_hash_file + class Application:
def __init__(self): + self._config = Config() + self._parser = argparse.ArgumentParser( conflict_handler = "resolve", formatter_class = argparse.RawDescriptionHelpFormatter, -- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2018-07-17 at 11:40 +0200, Katerina Koukiou wrote:
On Thu, Jul 12, 2018 at 05:19:20PM +0200, Andrea Bolognani wrote:
+ try: + with open(flavor_file, "r") as f: + flavor = f.readline().strip() + except:
And it's generally discouraged to use a bare except because it catches BaseException. Be at least as specific as Exception, and here specifically, perhaps catch IOError.
Okay, I've done some reading about Exception vs BaseException and I think I see now why you would want to catch the former but not the latter. Catching IOError specifically doesn't seem too useful here, though, because it would still result in an Error being raised and the program ultimately exiting. Or am I missing something?
+ if flavor != "test" and flavor != "jenkins":
More pythonic would be "flavor not in ('test', 'jenkins')"
Okay. -- Andrea Bolognani / Red Hat / Virtualization

We use an actual YAML parser this time around, and bring the behavior more in line with what Ansible is doing, so interoperability should be more solid overall. New in this implementation is more flexibility in defining host lists, including support for explicit lists as well as glob patterns. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/guests/lcitool b/guests/lcitool index e03b388..e90a33b 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -19,11 +19,13 @@ import argparse import crypt +import fnmatch import os import random import string import sys import textwrap +import yaml # This is necessary to maintain Python 2.7 compatibility try: @@ -44,6 +46,32 @@ class Util: salt = "".join(random.choice(alphabeth) for x in range(0, 16)) return "$6${}$".format(salt) + @staticmethod + def expand_pattern(pattern, source, name): + if pattern == None: + raise Error("Missing {} list".format(name)) + + if pattern == "all": + pattern = "*" + + # This works correctly for single items as well as more complex + # cases such as explicit lists, glob patterns and any combination + # of the above + matches = [] + for partial_pattern in pattern.split(","): + + partial_matches = [] + for item in source: + if fnmatch.fnmatch(item, partial_pattern): + partial_matches += [item] + + if len(partial_matches) == 0: + raise Error("Invalid {} list '{}'".format(name, pattern)) + + matches += partial_matches + + return sorted(set(matches)) + class Config: def _get_config_file(self, name): @@ -142,10 +170,73 @@ class Config: return root_hash_file +class Inventory: + + def __init__(self): + try: + parser = configparser.SafeConfigParser() + parser.read("./ansible.cfg") + inventory_path = parser.get("defaults", "inventory") + except: + raise Error("Can't find inventory location in ansible.cfg") + + self._facts = {} + try: + # We can only deal with trivial inventories, but that's + # all we need right now and we can expand support further + # later on if necessary + with open(inventory_path, "r") as f: + for line in f: + host = line.strip() + self._facts[host] = {} + except: + raise Error( + "Missing or invalid inventory ({})".format( + inventory_path, + ) + ) + + for host in self._facts: + try: + self._facts[host] = self._read_all_facts(host) + self._facts[host]["inventory_hostname"] = host + except: + raise Error("Can't load facts for '{}'".format(host)) + + def _add_facts_from_file(self, facts, yaml_path): + with open(yaml_path, "r") as f: + some_facts = yaml.load(f) + for fact in some_facts: + facts[fact] = some_facts[fact] + + def _read_all_facts(self, host): + facts = {} + + # We load from group_vars/ first and host_vars/ second, sorting + # files alphabetically; doing so should result in our view of + # the facts matching Ansible's + for source in ["./group_vars/all/", "./host_vars/{}/".format(host)]: + for item in sorted(os.listdir(source)): + yaml_path = os.path.join(source, item) + if not os.path.isfile(yaml_path): + continue + if not yaml_path.endswith(".yml"): + continue + self._add_facts_from_file(facts, yaml_path) + + return facts + + def expand_pattern(self, pattern): + return Util.expand_pattern(pattern, self._facts, "host") + + def get_facts(self, host): + return self._facts[host] + class Application: def __init__(self): self._config = Config() + self._inventory = Inventory() self._parser = argparse.ArgumentParser( conflict_handler = "resolve", -- 2.17.1

On Thu, Jul 12, 2018 at 05:19:21PM +0200, Andrea Bolognani wrote:
We use an actual YAML parser this time around, and bring the behavior more in line with what Ansible is doing, so interoperability should be more solid overall.
New in this implementation is more flexibility in defining host lists, including support for explicit lists as well as glob patterns.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+)
diff --git a/guests/lcitool b/guests/lcitool index e03b388..e90a33b 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -19,11 +19,13 @@
import argparse import crypt +import fnmatch import os import random import string import sys import textwrap +import yaml
Since you have to install yaml to actually use it, I suggest at this point to create a requirements.txt file for this script, so that users don't have to go over the code to check the dependencies.
# This is necessary to maintain Python 2.7 compatibility try: @@ -44,6 +46,32 @@ class Util: salt = "".join(random.choice(alphabeth) for x in range(0, 16)) return "$6${}$".format(salt)
+ @staticmethod + def expand_pattern(pattern, source, name): + if pattern == None: + raise Error("Missing {} list".format(name)) + + if pattern == "all": + pattern = "*" + + # This works correctly for single items as well as more complex + # cases such as explicit lists, glob patterns and any combination + # of the above + matches = [] + for partial_pattern in pattern.split(","): + + partial_matches = [] + for item in source: + if fnmatch.fnmatch(item, partial_pattern): + partial_matches += [item] + + if len(partial_matches) == 0: + raise Error("Invalid {} list '{}'".format(name, pattern)) + + matches += partial_matches + + return sorted(set(matches)) + class Config:
def _get_config_file(self, name): @@ -142,10 +170,73 @@ class Config:
return root_hash_file
+class Inventory: + + def __init__(self): + try: + parser = configparser.SafeConfigParser() + parser.read("./ansible.cfg") + inventory_path = parser.get("defaults", "inventory") + except: + raise Error("Can't find inventory location in ansible.cfg") + + self._facts = {} + try: + # We can only deal with trivial inventories, but that's + # all we need right now and we can expand support further + # later on if necessary + with open(inventory_path, "r") as f: + for line in f: + host = line.strip() + self._facts[host] = {} + except: + raise Error( + "Missing or invalid inventory ({})".format( + inventory_path, + ) + ) + + for host in self._facts: + try: + self._facts[host] = self._read_all_facts(host) + self._facts[host]["inventory_hostname"] = host + except: + raise Error("Can't load facts for '{}'".format(host)) + + def _add_facts_from_file(self, facts, yaml_path): + with open(yaml_path, "r") as f: + some_facts = yaml.load(f) + for fact in some_facts: + facts[fact] = some_facts[fact] + + def _read_all_facts(self, host): + facts = {} + + # We load from group_vars/ first and host_vars/ second, sorting + # files alphabetically; doing so should result in our view of + # the facts matching Ansible's + for source in ["./group_vars/all/", "./host_vars/{}/".format(host)]: + for item in sorted(os.listdir(source)): + yaml_path = os.path.join(source, item) + if not os.path.isfile(yaml_path): + continue + if not yaml_path.endswith(".yml"): + continue + self._add_facts_from_file(facts, yaml_path) + + return facts + + def expand_pattern(self, pattern): + return Util.expand_pattern(pattern, self._facts, "host") + + def get_facts(self, host): + return self._facts[host] + class Application:
def __init__(self): self._config = Config() + self._inventory = Inventory()
self._parser = argparse.ArgumentParser( conflict_handler = "resolve", -- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2018-07-17 at 11:57 +0200, Katerina Koukiou wrote:
On Thu, Jul 12, 2018 at 05:19:21PM +0200, Andrea Bolognani wrote:
+import yaml
Since you have to install yaml to actually use it, I suggest at this point to create a requirements.txt file for this script, so that users don't have to go over the code to check the dependencies.
Neat idea. If that's okay with you, I'd rather work on that as a follow-up enhancement though, because I need to do some reading on it and I'd rather not leave this series lingering for too long unless it's really necessary. -- Andrea Bolognani / Red Hat / Virtualization

On Tue, Jul 17, 2018 at 02:51:47PM +0200, Andrea Bolognani wrote:
On Tue, 2018-07-17 at 11:57 +0200, Katerina Koukiou wrote:
On Thu, Jul 12, 2018 at 05:19:21PM +0200, Andrea Bolognani wrote:
+import yaml
Since you have to install yaml to actually use it, I suggest at this point to create a requirements.txt file for this script, so that users don't have to go over the code to check the dependencies.
Neat idea.
If that's okay with you, I'd rather work on that as a follow-up enhancement though, because I need to do some reading on it and I'd rather not leave this series lingering for too long unless it's really necessary.
Sure, I am ok with that.
-- Andrea Bolognani / Red Hat / Virtualization

This replaces the 'list' action from the original implementation. We're going to list more than just hosts over time, so a more specific name is warranted. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/guests/lcitool b/guests/lcitool index e90a33b..f11b92e 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -243,7 +243,8 @@ class Application: formatter_class = argparse.RawDescriptionHelpFormatter, description = "libvirt CI guest management tool", epilog = textwrap.dedent(""" - supported actions: + informational actions: + hosts list all known hosts """), ) self._parser.add_argument( @@ -253,6 +254,10 @@ class Application: help = "action to perform (see below)", ) + def _action_list(self): + for host in self._inventory.expand_pattern("all"): + print(host) + def run(self): cmdline = self._parser.parse_args() action = cmdline.a -- 2.17.1

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 74 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/guests/lcitool b/guests/lcitool index f11b92e..486f82f 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -23,6 +23,7 @@ import fnmatch import os import random import string +import subprocess import sys import textwrap import yaml @@ -243,8 +244,13 @@ class Application: formatter_class = argparse.RawDescriptionHelpFormatter, description = "libvirt CI guest management tool", epilog = textwrap.dedent(""" + common actions: + install perform unattended host installation + informational actions: hosts list all known hosts + + glob patterns are supported for HOSTS """), ) self._parser.add_argument( @@ -253,19 +259,83 @@ class Application: required = True, help = "action to perform (see below)", ) + self._parser.add_argument( + "-h", + metavar = "HOSTS", + help = "list of hosts to act on", + ) - def _action_list(self): + def _action_list(self, hosts): for host in self._inventory.expand_pattern("all"): print(host) + def _action_install(self, hosts): + flavor = self._config.get_flavor() + + for host in self._inventory.expand_pattern(hosts): + facts = self._inventory.get_facts(host) + + # Both memory size and disk size are stored as GiB in the + # inventory, but virt-install expects the disk size in GiB + # and the memory size in *MiB*, so perform conversion here + memory_arg = str(int(facts["install_memory_size"]) * 1024) + + vcpus_arg = str(facts["install_vcpus"]) + + disk_arg = "size={},pool={},bus=virtio".format( + facts["install_disk_size"], + facts["install_storage_pool"], + ) + network_arg = "network={},model=virtio".format( + facts["install_network"], + ) + + # 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"], + ) + + cmd = [ + "virt-install", + "--name", host, + "--location", facts["install_url"], + "--virt-type", facts["install_virt_type"], + "--arch", facts["install_arch"], + "--machine", facts["install_machine"], + "--cpu", facts["install_cpu_model"], + "--vcpus", vcpus_arg, + "--memory", memory_arg, + "--disk", disk_arg, + "--network", network_arg, + "--graphics", "none", + "--console", "pty", + "--sound", "none", + "--initrd-inject", facts["install_config"], + "--extra-args", extra_arg, + "--wait", "0", + ] + + # Only configure autostart for the guest for the jenkins flavor + if flavor == "jenkins": + cmd += [ "--autostart" ] + + try: + subprocess.check_call(cmd) + except: + raise Error("Failed to install '{}'".format(host)) + def run(self): cmdline = self._parser.parse_args() action = cmdline.a + hosts = cmdline.h method = "_action_{}".format(action.replace("-", "_")) if hasattr(self, method): - getattr(self, method).__call__() + getattr(self, method).__call__(hosts) else: raise Error("Invalid action '{}'".format(action)) -- 2.17.1

On Thu, Jul 12, 2018 at 05:19:23PM +0200, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 74 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 72 insertions(+), 2 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool index f11b92e..486f82f 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -23,6 +23,7 @@ import fnmatch import os import random import string +import subprocess import sys import textwrap import yaml @@ -243,8 +244,13 @@ class Application: formatter_class = argparse.RawDescriptionHelpFormatter, description = "libvirt CI guest management tool", epilog = textwrap.dedent(""" + common actions: + install perform unattended host installation + informational actions: hosts list all known hosts + + glob patterns are supported for HOSTS """), ) self._parser.add_argument( @@ -253,19 +259,83 @@ class Application: required = True, help = "action to perform (see below)", ) + self._parser.add_argument( + "-h", + metavar = "HOSTS", + help = "list of hosts to act on", + )
- def _action_list(self): + def _action_list(self, hosts):
'hosts' argument is not used here.
for host in self._inventory.expand_pattern("all"): print(host)
+ def _action_install(self, hosts): + flavor = self._config.get_flavor() + + for host in self._inventory.expand_pattern(hosts): + facts = self._inventory.get_facts(host) + + # Both memory size and disk size are stored as GiB in the + # inventory, but virt-install expects the disk size in GiB + # and the memory size in *MiB*, so perform conversion here + memory_arg = str(int(facts["install_memory_size"]) * 1024) + + vcpus_arg = str(facts["install_vcpus"]) + + disk_arg = "size={},pool={},bus=virtio".format( + facts["install_disk_size"], + facts["install_storage_pool"], + ) + network_arg = "network={},model=virtio".format( + facts["install_network"], + ) + + # 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"], + ) + + cmd = [ + "virt-install", + "--name", host, + "--location", facts["install_url"], + "--virt-type", facts["install_virt_type"], + "--arch", facts["install_arch"], + "--machine", facts["install_machine"], + "--cpu", facts["install_cpu_model"], + "--vcpus", vcpus_arg, + "--memory", memory_arg, + "--disk", disk_arg, + "--network", network_arg, + "--graphics", "none", + "--console", "pty", + "--sound", "none", + "--initrd-inject", facts["install_config"], + "--extra-args", extra_arg, + "--wait", "0", + ] + + # Only configure autostart for the guest for the jenkins flavor + if flavor == "jenkins": + cmd += [ "--autostart" ] + + try: + subprocess.check_call(cmd) + except: + raise Error("Failed to install '{}'".format(host)) + def run(self): cmdline = self._parser.parse_args() action = cmdline.a + hosts = cmdline.h
method = "_action_{}".format(action.replace("-", "_"))
if hasattr(self, method): - getattr(self, method).__call__() + getattr(self, method).__call__(hosts) else: raise Error("Invalid action '{}'".format(action))
-- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2018-07-17 at 14:52 +0200, Katerina Koukiou wrote:
On Thu, Jul 12, 2018 at 05:19:23PM +0200, Andrea Bolognani wrote:
- def _action_list(self): + def _action_list(self, hosts):
'hosts' argument is not used here.
Sure, but...
+ def _action_install(self, hosts): + flavor = self._config.get_flavor()
... it's used here, and...
def run(self): cmdline = self._parser.parse_args() action = cmdline.a + hosts = cmdline.h
method = "_action_{}".format(action.replace("-", "_"))
if hasattr(self, method): - getattr(self, method).__call__() + getattr(self, method).__call__(hosts) else: raise Error("Invalid action '{}'".format(action))
... we call all functions implementing actions with the same arguments, so I don't really see a way around it... -- Andrea Bolognani / Red Hat / Virtualization

The 'prepare' alias was kinda redundant and offered dubious value, so it has been dropped. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/guests/lcitool b/guests/lcitool index 486f82f..d82c36f 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -246,6 +246,7 @@ class Application: epilog = textwrap.dedent(""" common actions: install perform unattended host installation + update prepare hosts and keep them updated informational actions: hosts list all known hosts @@ -327,6 +328,35 @@ class Application: except: raise Error("Failed to install '{}'".format(host)) + def _action_update(self, hosts): + flavor = self._config.get_flavor() + vault_pass_file = self._config.get_vault_password_file() + root_pass_file = self._config.get_root_password_file() + + ansible_hosts = ",".join(self._inventory.expand_pattern(hosts)) + + extra_vars = "flavor={} root_password_file={}".format( + flavor, + root_pass_file, + ) + + cmd = [ "ansible-playbook" ] + + # Provide the vault password if available + if vault_pass_file is not None: + cmd += [ "--vault-password-file", vault_pass_file ] + + cmd += [ + "--limit", ansible_hosts, + "--extra-vars", extra_vars, + "./site.yml", + ] + + try: + subprocess.check_call(cmd) + except: + raise Error("Failed to update '{}'".format(hosts)) + def run(self): cmdline = self._parser.parse_args() action = cmdline.a -- 2.17.1

On Thu, Jul 12, 2018 at 05:19:24PM +0200, Andrea Bolognani wrote:
The 'prepare' alias was kinda redundant and offered dubious value, so it has been dropped.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/guests/lcitool b/guests/lcitool index 486f82f..d82c36f 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -246,6 +246,7 @@ class Application: epilog = textwrap.dedent(""" common actions: install perform unattended host installation + update prepare hosts and keep them updated
informational actions: hosts list all known hosts @@ -327,6 +328,35 @@ class Application: except: raise Error("Failed to install '{}'".format(host))
+ def _action_update(self, hosts): + flavor = self._config.get_flavor() + vault_pass_file = self._config.get_vault_password_file() + root_pass_file = self._config.get_root_password_file() + + ansible_hosts = ",".join(self._inventory.expand_pattern(hosts)) + + extra_vars = "flavor={} root_password_file={}".format( + flavor, + root_pass_file, + ) + + cmd = [ "ansible-playbook" ]
IMO this requirement should be as well in python requirements file. In this way you can also specify which ansible version is expected for the site.yaml playbook to run.
+ + # Provide the vault password if available + if vault_pass_file is not None: + cmd += [ "--vault-password-file", vault_pass_file ] + + cmd += [ + "--limit", ansible_hosts, + "--extra-vars", extra_vars, + "./site.yml", + ] + + try: + subprocess.check_call(cmd) + except: + raise Error("Failed to update '{}'".format(hosts)) + def run(self): cmdline = self._parser.parse_args() action = cmdline.a -- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2018-07-17 at 14:57 +0200, Katerina Koukiou wrote:
On Thu, Jul 12, 2018 at 05:19:24PM +0200, Andrea Bolognani wrote:
+ cmd = [ "ansible-playbook" ]
IMO this requirement should be as well in python requirements file. In this way you can also specify which ansible version is expected for the site.yaml playbook to run.
I had no idea you could require commands as well as Python modules. Clearly shows that I need to read up on the stuff :) -- Andrea Bolognani / Red Hat / Virtualization

On Tue, Jul 17, 2018 at 04:45:39PM +0200, Andrea Bolognani wrote:
On Tue, 2018-07-17 at 14:57 +0200, Katerina Koukiou wrote:
On Thu, Jul 12, 2018 at 05:19:24PM +0200, Andrea Bolognani wrote:
+ cmd = [ "ansible-playbook" ]
IMO this requirement should be as well in python requirements file. In this way you can also specify which ansible version is expected for the site.yaml playbook to run.
I had no idea you could require commands as well as Python modules. Clearly shows that I need to read up on the stuff :)
Ansible is available from pypi https://pypi.org/project/ansible/ Well I had in mind only pip requirements just put together in a requirements file so that you can install them with `pip install -r requirements.txt`. Then you can easily test different ansible versions for example. Anyway, of course this can be addressed in the future.
-- Andrea Bolognani / Red Hat / Virtualization

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/README.markdown | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/guests/README.markdown b/guests/README.markdown index bc780f3..4a40619 100644 --- a/guests/README.markdown +++ b/guests/README.markdown @@ -6,16 +6,16 @@ of the guests used by the Jenkins-based libvirt CI environment. There are two steps to bringing up a guest: -* `./lcitool install $guest` will perform an unattended installation +* `./lcitool -a install -h $guest` will perform an unattended installation of `$guest`. Not all guests can be installed this way: see the "FreeBSD" section below; -* `./lcitool prepare $guest` will go through all the post-installation +* `./lcitool -a update -h $guest` will go through all the post-installation configuration steps required to make the newly-created guest usable; Once those steps have been performed, maintainance will involve running: -* `./lcitool update $guest` +* `./lcitool -a update -h $guest` periodically to ensure the guest configuration is sane and all installed packages are updated. @@ -40,7 +40,7 @@ you'll want to use the `libvirt_guest` variant of the plugin. To keep guests up to date over time, it's recommended to have an entry along the lines of - 0 0 * * * cd ~/libvirt-jenkins-ci/guests && ./lcitool update all + 0 0 * * * cd ~/libvirt-jenkins-ci/guests && ./lcitool -a update -h all in your crontab. -- 2.17.1

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/host_vars/libvirt-centos-7/docker.yml | 2 ++ guests/host_vars/libvirt-debian-8/docker.yml | 2 ++ guests/host_vars/libvirt-debian-9/docker.yml | 2 ++ guests/host_vars/libvirt-debian-sid/docker.yml | 2 ++ guests/host_vars/libvirt-fedora-27/docker.yml | 2 ++ guests/host_vars/libvirt-fedora-28/docker.yml | 2 ++ guests/host_vars/libvirt-fedora-rawhide/docker.yml | 2 ++ guests/host_vars/libvirt-ubuntu-16/docker.yml | 2 ++ guests/host_vars/libvirt-ubuntu-18/docker.yml | 2 ++ 9 files changed, 18 insertions(+) create mode 100644 guests/host_vars/libvirt-centos-7/docker.yml create mode 100644 guests/host_vars/libvirt-debian-8/docker.yml create mode 100644 guests/host_vars/libvirt-debian-9/docker.yml create mode 100644 guests/host_vars/libvirt-debian-sid/docker.yml create mode 100644 guests/host_vars/libvirt-fedora-27/docker.yml create mode 100644 guests/host_vars/libvirt-fedora-28/docker.yml create mode 100644 guests/host_vars/libvirt-fedora-rawhide/docker.yml create mode 100644 guests/host_vars/libvirt-ubuntu-16/docker.yml create mode 100644 guests/host_vars/libvirt-ubuntu-18/docker.yml diff --git a/guests/host_vars/libvirt-centos-7/docker.yml b/guests/host_vars/libvirt-centos-7/docker.yml new file mode 100644 index 0000000..59f7f12 --- /dev/null +++ b/guests/host_vars/libvirt-centos-7/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: centos:centos7 diff --git a/guests/host_vars/libvirt-debian-8/docker.yml b/guests/host_vars/libvirt-debian-8/docker.yml new file mode 100644 index 0000000..235f0fd --- /dev/null +++ b/guests/host_vars/libvirt-debian-8/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: debian:8 diff --git a/guests/host_vars/libvirt-debian-9/docker.yml b/guests/host_vars/libvirt-debian-9/docker.yml new file mode 100644 index 0000000..0b4ccee --- /dev/null +++ b/guests/host_vars/libvirt-debian-9/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: debian:9 diff --git a/guests/host_vars/libvirt-debian-sid/docker.yml b/guests/host_vars/libvirt-debian-sid/docker.yml new file mode 100644 index 0000000..e20a37e --- /dev/null +++ b/guests/host_vars/libvirt-debian-sid/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: debian:sid diff --git a/guests/host_vars/libvirt-fedora-27/docker.yml b/guests/host_vars/libvirt-fedora-27/docker.yml new file mode 100644 index 0000000..dcada18 --- /dev/null +++ b/guests/host_vars/libvirt-fedora-27/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: fedora:27 diff --git a/guests/host_vars/libvirt-fedora-28/docker.yml b/guests/host_vars/libvirt-fedora-28/docker.yml new file mode 100644 index 0000000..a874018 --- /dev/null +++ b/guests/host_vars/libvirt-fedora-28/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: fedora:28 diff --git a/guests/host_vars/libvirt-fedora-rawhide/docker.yml b/guests/host_vars/libvirt-fedora-rawhide/docker.yml new file mode 100644 index 0000000..39dda1c --- /dev/null +++ b/guests/host_vars/libvirt-fedora-rawhide/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: fedora:rawhide diff --git a/guests/host_vars/libvirt-ubuntu-16/docker.yml b/guests/host_vars/libvirt-ubuntu-16/docker.yml new file mode 100644 index 0000000..2d4eb25 --- /dev/null +++ b/guests/host_vars/libvirt-ubuntu-16/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: ubuntu:16.04 diff --git a/guests/host_vars/libvirt-ubuntu-18/docker.yml b/guests/host_vars/libvirt-ubuntu-18/docker.yml new file mode 100644 index 0000000..13d6cc1 --- /dev/null +++ b/guests/host_vars/libvirt-ubuntu-18/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: ubuntu:18.04 -- 2.17.1

The original tool's limited scope meant loadins this information was not needed, but we're going to start making use of it pretty soon. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/guests/lcitool b/guests/lcitool index d82c36f..3bd5fa7 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -233,11 +233,58 @@ class Inventory: def get_facts(self, host): return self._facts[host] +class Projects: + + def __init__(self): + try: + with open("./vars/mappings.yml", "r") as f: + mappings = yaml.load(f) + self._mappings = mappings["mappings"] + except: + raise Error("Can't load mappings") + + self._packages = {} + source = "./vars/projects/" + for item in os.listdir(source): + yaml_path = os.path.join(source, item) + if not os.path.isfile(yaml_path): + continue + if not yaml_path.endswith(".yml"): + continue + + project = os.path.splitext(item)[0] + + try: + with open(yaml_path, "r") as f: + packages = yaml.load(f) + self._packages[project] = packages["packages"] + except: + raise Error("Can't load packages for '{}'".format(project)) + + def expand_pattern(self, pattern): + projects = Util.expand_pattern(pattern, self._packages, "project") + + # Some projects are internal implementation details and should + # not be exposed to the user + internal_projects = [ "base", "blacklist", "jenkins" ] + for project in internal_projects: + if project in projects: + projects.remove(project) + + return projects + + def get_mappings(self): + return self._mappings + + def get_packages(self, project): + return self._packages[project] + class Application: def __init__(self): self._config = Config() self._inventory = Inventory() + self._projects = Projects() self._parser = argparse.ArgumentParser( conflict_handler = "resolve", -- 2.17.1

On Thu, Jul 12, 2018 at 05:19:27PM +0200, Andrea Bolognani wrote:
The original tool's limited scope meant loadins this information was not needed, but we're going to start making use of it pretty soon.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
diff --git a/guests/lcitool b/guests/lcitool index d82c36f..3bd5fa7 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -233,11 +233,58 @@ class Inventory: def get_facts(self, host): return self._facts[host]
+class Projects: + + def __init__(self): + try: + with open("./vars/mappings.yml", "r") as f:
There is clear information where how to run the lcitool in the docs in some patches befor so the relative paths that are used everywhere in the code are not causing a problem. Though IMO, I think it's clearer to have a variable (config option, hardcoded, env variable or whatever you decide), storing the path of these files so that this code is not dependent on relative paths. WDYT?
+ mappings = yaml.load(f) + self._mappings = mappings["mappings"] + except: + raise Error("Can't load mappings") + + self._packages = {} + source = "./vars/projects/" + for item in os.listdir(source): + yaml_path = os.path.join(source, item) + if not os.path.isfile(yaml_path): + continue + if not yaml_path.endswith(".yml"): + continue + + project = os.path.splitext(item)[0] + + try: + with open(yaml_path, "r") as f: + packages = yaml.load(f) + self._packages[project] = packages["packages"] + except: + raise Error("Can't load packages for '{}'".format(project)) + + def expand_pattern(self, pattern): + projects = Util.expand_pattern(pattern, self._packages, "project") + + # Some projects are internal implementation details and should + # not be exposed to the user + internal_projects = [ "base", "blacklist", "jenkins" ] + for project in internal_projects: + if project in projects: + projects.remove(project) + + return projects + + def get_mappings(self): + return self._mappings + + def get_packages(self, project): + return self._packages[project] + class Application:
def __init__(self): self._config = Config() self._inventory = Inventory() + self._projects = Projects()
self._parser = argparse.ArgumentParser( conflict_handler = "resolve", -- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2018-07-17 at 15:04 +0200, Katerina Koukiou wrote:
On Thu, Jul 12, 2018 at 05:19:27PM +0200, Andrea Bolognani wrote:
+class Projects: + + def __init__(self): + try: + with open("./vars/mappings.yml", "r") as f:
There is clear information where how to run the lcitool in the docs in some patches befor so the relative paths that are used everywhere in the code are not causing a problem. Though IMO, I think it's clearer to have a variable (config option, hardcoded, env variable or whatever you decide), storing the path of these files so that this code is not dependent on relative paths. WDYT?
Some of the paths, like vars/mappings.yml, are pretty much entirely arbitrary but others, like group_vars/all/main.yml, can't be changed because that's what Ansible expects. Additionally, none of the paths is repeated more than once in the script so using something like mappings_path = "./vars/mappings.yml" open(mappings_path, "r") instead of open("./vars/mappings.yml", "r") wouldn't IMHO buy us much. -- Andrea Bolognani / Red Hat / Virtualization

On Tue, Jul 17, 2018 at 04:44:24PM +0200, Andrea Bolognani wrote:
On Tue, 2018-07-17 at 15:04 +0200, Katerina Koukiou wrote:
On Thu, Jul 12, 2018 at 05:19:27PM +0200, Andrea Bolognani wrote:
+class Projects: + + def __init__(self): + try: + with open("./vars/mappings.yml", "r") as f:
There is clear information where how to run the lcitool in the docs in some patches befor so the relative paths that are used everywhere in the code are not causing a problem. Though IMO, I think it's clearer to have a variable (config option, hardcoded, env variable or whatever you decide), storing the path of these files so that this code is not dependent on relative paths. WDYT?
Some of the paths, like vars/mappings.yml, are pretty much entirely arbitrary but others, like group_vars/all/main.yml, can't be changed because that's what Ansible expects.
Additionally, none of the paths is repeated more than once in the script so using something like
mappings_path = "./vars/mappings.yml" open(mappings_path, "r")
instead of
open("./vars/mappings.yml", "r")
wouldn't IMHO buy us much.
I meant only having a variable for the playbook location. playbook_path = os.getenv('PLAYBOOK_PATH', './')) And then use os.path.join for all the others relative paths to the playbook path. So that you can actually run the lcitool script from wherever you want. Anyway, since this script usage is not wide, feel free to keep everything hardcoded.
-- Andrea Bolognani / Red Hat / Virtualization

On Tue, 2018-07-17 at 19:55 +0200, Katerina Koukiou wrote:
I meant only having a variable for the playbook location.
playbook_path = os.getenv('PLAYBOOK_PATH', './'))
And then use os.path.join for all the others relative paths to the playbook path. So that you can actually run the lcitool script from wherever you want.
That wouldn't be quite enough: you'd also have to make sure you call ansible / ansible-playbook like ANSIBLE_CONFIG="$PLAYBOOK_PATH/ansible.cfg" \ ansible --playbook-dir "$PLAYBOOK_PATH" ... otherwise it won't work. And at that point, the script might as well just figure out the base directory itself based on its own location :) Should be easy enough. I'll look into it. -- Andrea Bolognani / Red Hat / Virtualization

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/guests/lcitool b/guests/lcitool index 3bd5fa7..d42b7e7 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -296,7 +296,8 @@ class Application: update prepare hosts and keep them updated informational actions: - hosts list all known hosts + hosts list all known hosts + projects list all known projects glob patterns are supported for HOSTS """), @@ -317,6 +318,10 @@ class Application: for host in self._inventory.expand_pattern("all"): print(host) + def _action_projects(self, hosts): + for project in self._projects.expand_pattern("all"): + print(project) + def _action_install(self, hosts): flavor = self._config.get_flavor() -- 2.17.1

This is basically the exact same algorithm used by the Ansible playbooks to process package mappings, implemented in pure Python. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 86 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 6 deletions(-) diff --git a/guests/lcitool b/guests/lcitool index d42b7e7..61cae97 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -299,7 +299,10 @@ class Application: hosts list all known hosts projects list all known projects - glob patterns are supported for HOSTS + uncommon actions: + dockerfile generate Dockerfile (doesn't access the host) + + glob patterns are supported for HOSTS and PROJECTS """), ) self._parser.add_argument( @@ -313,16 +316,21 @@ class Application: metavar = "HOSTS", help = "list of hosts to act on", ) + self._parser.add_argument( + "-p", + metavar = "PROJECTS", + help = "list of projects to consider", + ) - def _action_list(self, hosts): + def _action_hosts(self, hosts, projects): for host in self._inventory.expand_pattern("all"): print(host) - def _action_projects(self, hosts): + def _action_projects(self, hosts, projects): for project in self._projects.expand_pattern("all"): print(project) - def _action_install(self, hosts): + def _action_install(self, hosts, projects): flavor = self._config.get_flavor() for host in self._inventory.expand_pattern(hosts): @@ -380,7 +388,7 @@ class Application: except: raise Error("Failed to install '{}'".format(host)) - def _action_update(self, hosts): + def _action_update(self, hosts, projects): flavor = self._config.get_flavor() vault_pass_file = self._config.get_vault_password_file() root_pass_file = self._config.get_root_password_file() @@ -409,15 +417,81 @@ class Application: except: raise Error("Failed to update '{}'".format(hosts)) + def _action_dockerfile(self, hosts, projects): + mappings = self._projects.get_mappings() + + hosts = self._inventory.expand_pattern(hosts) + if len(hosts) > 1: + raise Error("Can't generate Dockerfile for multiple hosts") + host = hosts[0] + + facts = self._inventory.get_facts(host) + package_format = facts["package_format"] + os_name = facts["os_name"] + os_full = os_name + str(facts["os_version"]) + + if package_format != "deb" and package_format != "rpm": + raise Error("Host {} doesn't support Dockerfiles".format(host)) + + projects = self._projects.expand_pattern(projects) + for project in projects: + if project not in facts['projects']: + raise Error( + "Host {} doesn't support project {}".format( + host, + project, + ) + ) + + temp = {} + + # We need to add the base project manually here: the standard + # machinery hides it because it's an implementation detail + for project in projects + [ "base" ]: + for package in self._projects.get_packages(project): + if "default" in mappings[package]: + temp[package] = mappings[package]["default"] + if package_format in mappings[package]: + temp[package] = mappings[package][package_format] + if os_name in mappings[package]: + temp[package] = mappings[package][os_name] + if os_full in mappings[package]: + temp[package] = mappings[package][os_full] + + flattened = [] + for item in temp: + if temp[item] != None and temp[item] not in flattened: + flattened += [ temp[item] ] + + print("FROM {}".format(facts['docker_base'])) + + sys.stdout.write("ENV PACKAGES ") + sys.stdout.write(" \\\n ".join(sorted(flattened))) + + if package_format == "deb": + sys.stdout.write(textwrap.dedent(""" + RUN apt-get update && \\ + apt-get install -y ${PACKAGES} && \\ + apt-get autoremove -y && \\ + apt-get autoclean -y + """)) + elif package_format == "rpm": + sys.stdout.write(textwrap.dedent(""" + RUN yum install -y ${PACKAGES} && \\ + yum autoremove -y && \\ + yum clean all -y + """)) + def run(self): cmdline = self._parser.parse_args() action = cmdline.a hosts = cmdline.h + projects = cmdline.p method = "_action_{}".format(action.replace("-", "_")) if hasattr(self, method): - getattr(self, method).__call__(hosts) + getattr(self, method).__call__(hosts, projects) else: raise Error("Invalid action '{}'".format(action)) -- 2.17.1

On Thu, Jul 12, 2018 at 05:19:29PM +0200, Andrea Bolognani wrote:
This is basically the exact same algorithm used by the Ansible playbooks to process package mappings, implemented in pure Python.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/lcitool | 86 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 6 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool index d42b7e7..61cae97 100755 --- a/guests/lcitool +++ b/guests/lcitool @@ -299,7 +299,10 @@ class Application: hosts list all known hosts projects list all known projects
- glob patterns are supported for HOSTS + uncommon actions: + dockerfile generate Dockerfile (doesn't access the host) + + glob patterns are supported for HOSTS and PROJECTS """), ) self._parser.add_argument( @@ -313,16 +316,21 @@ class Application: metavar = "HOSTS", help = "list of hosts to act on", ) + self._parser.add_argument( + "-p", + metavar = "PROJECTS", + help = "list of projects to consider", + )
- def _action_list(self, hosts): + def _action_hosts(self, hosts, projects): for host in self._inventory.expand_pattern("all"): print(host)
- def _action_projects(self, hosts): + def _action_projects(self, hosts, projects): for project in self._projects.expand_pattern("all"): print(project)
- def _action_install(self, hosts): + def _action_install(self, hosts, projects): flavor = self._config.get_flavor()
for host in self._inventory.expand_pattern(hosts): @@ -380,7 +388,7 @@ class Application: except: raise Error("Failed to install '{}'".format(host))
- def _action_update(self, hosts): + def _action_update(self, hosts, projects): flavor = self._config.get_flavor() vault_pass_file = self._config.get_vault_password_file() root_pass_file = self._config.get_root_password_file() @@ -409,15 +417,81 @@ class Application: except: raise Error("Failed to update '{}'".format(hosts))
+ def _action_dockerfile(self, hosts, projects): + mappings = self._projects.get_mappings() + + hosts = self._inventory.expand_pattern(hosts) + if len(hosts) > 1: + raise Error("Can't generate Dockerfile for multiple hosts") + host = hosts[0] + + facts = self._inventory.get_facts(host) + package_format = facts["package_format"] + os_name = facts["os_name"] + os_full = os_name + str(facts["os_version"]) + + if package_format != "deb" and package_format != "rpm":
More pythonic would be "package_format not in ('deb', 'rpm')"
+ raise Error("Host {} doesn't support Dockerfiles".format(host)) + + projects = self._projects.expand_pattern(projects) + for project in projects: + if project not in facts['projects']: + raise Error( + "Host {} doesn't support project {}".format( + host, + project, + ) + ) + + temp = {} + + # We need to add the base project manually here: the standard + # machinery hides it because it's an implementation detail + for project in projects + [ "base" ]: + for package in self._projects.get_packages(project): + if "default" in mappings[package]: + temp[package] = mappings[package]["default"] + if package_format in mappings[package]: + temp[package] = mappings[package][package_format] + if os_name in mappings[package]: + temp[package] = mappings[package][os_name] + if os_full in mappings[package]: + temp[package] = mappings[package][os_full] + + flattened = [] + for item in temp: + if temp[item] != None and temp[item] not in flattened: + flattened += [ temp[item] ] + + print("FROM {}".format(facts['docker_base'])) + + sys.stdout.write("ENV PACKAGES ") + sys.stdout.write(" \\\n ".join(sorted(flattened))) + + if package_format == "deb": + sys.stdout.write(textwrap.dedent(""" + RUN apt-get update && \\ + apt-get install -y ${PACKAGES} && \\ + apt-get autoremove -y && \\ + apt-get autoclean -y + """)) + elif package_format == "rpm": + sys.stdout.write(textwrap.dedent(""" + RUN yum install -y ${PACKAGES} && \\ + yum autoremove -y && \\ + yum clean all -y + """)) + def run(self): cmdline = self._parser.parse_args() action = cmdline.a hosts = cmdline.h + projects = cmdline.p
method = "_action_{}".format(action.replace("-", "_"))
if hasattr(self, method): - getattr(self, method).__call__(hosts) + getattr(self, method).__call__(hosts, projects) else: raise Error("Invalid action '{}'".format(action))
-- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Andrea Bolognani
-
Ján Tomko
-
Katerina Koukiou