Revamp cluster test script

Previously, either "--restart" was specified or a prompt was given, with
the prompt being ignored with "--unsafe" in favour of a reboot. This
failed to provide an explicit way to prevent VM restarts with these
commands, which might be desired in some non-interactive situations, and
the interaction of "--unsafe" with this option was an undesired bug.

This is now a complete binary flag with --restart and --no-restart
versions, while still defaulting to a prompt if neither is specified.
This allows full non-interactive control of this option.

api-daemon/provisioner/examples/script/3-debootstrap.py

@@ -441,7 +441,7 @@ class VMBuilderScript(VMBuilder):
 
         # The directory we mounted things on earlier during prepare(); this could very well
         # be exposed as a module-level variable if you so choose
-        temporary_directory = "/tmp/target"
+        temp_dir = "/tmp/target"
 
         # Use these convenient aliases for later (avoiding lots of "self.vm_data" everywhere)
         vm_name = self.vm_name
@@ -469,6 +469,8 @@ class VMBuilderScript(VMBuilder):
                 "grub-pc",
                 "cloud-init",
                 "python3-cffi-backend",
+                "acpid",
+                "acpi-support-base",
                 "wget",
             ]
 
@@ -482,17 +484,17 @@ class VMBuilderScript(VMBuilder):
 
         # Perform a debootstrap installation
         print(
-            f"Installing system with debootstrap: debootstrap --include={','.join(deb_packages)} {deb_release} {temporary_directory} {deb_mirror}"
+            f"Installing system with debootstrap: debootstrap --include={','.join(deb_packages)} {deb_release} {temp_dir} {deb_mirror}"
         )
         os.system(
-            f"debootstrap --include={','.join(deb_packages)} {deb_release} {temporary_directory} {deb_mirror}"
+            f"debootstrap --include={','.join(deb_packages)} {deb_release} {temp_dir} {deb_mirror}"
         )
 
         # Bind mount the devfs so we can grub-install later
-        os.system("mount --bind /dev {}/dev".format(temporary_directory))
+        os.system("mount --bind /dev {}/dev".format(temp_dir))
 
         # Create an fstab entry for each volume
-        fstab_file = "{}/etc/fstab".format(temporary_directory)
+        fstab_file = "{}/etc/fstab".format(temp_dir)
         # The volume ID starts at zero and increments by one for each volume in the fixed-order
         # volume list. This lets us work around the insanity of Libvirt IDs not matching guest IDs,
         # while still letting us have some semblance of control here without enforcing things
@@ -537,13 +539,13 @@ class VMBuilderScript(VMBuilder):
             volume_id += 1
 
         # Write the hostname; you could also take an FQDN argument for this as an example
-        hostname_file = "{}/etc/hostname".format(temporary_directory)
+        hostname_file = "{}/etc/hostname".format(temp_dir)
         with open(hostname_file, "w") as fh:
             fh.write("{}".format(vm_name))
 
         # Fix the cloud-init.target since it's broken by default in Debian 11
         cloudinit_target_file = "{}/etc/systemd/system/cloud-init.target".format(
-            temporary_directory
+            temp_dir
         )
         with open(cloudinit_target_file, "w") as fh:
             # We lose our indent on these raw blocks to preserve the apperance of the files
@@ -557,7 +559,7 @@ After=multi-user.target
             fh.write(data)
 
         # Write the cloud-init configuration
-        ci_cfg_file = "{}/etc/cloud/cloud.cfg".format(temporary_directory)
+        ci_cfg_file = "{}/etc/cloud/cloud.cfg".format(temp_dir)
         with open(ci_cfg_file, "w") as fh:
             fh.write(
                 """
@@ -618,15 +620,15 @@ After=multi-user.target
                      - arches: [default]
                        failsafe:
                          primary: {deb_mirror}
-                """
-            ).format(deb_mirror=deb_mirror)
+                """.format(
+                    deb_mirror=deb_mirror
+                )
+            )
 
         # Due to device ordering within the Libvirt XML configuration, the first Ethernet interface
         # will always be on PCI bus ID 2, hence the name "ens2".
         # Write a DHCP stanza for ens2
-        ens2_network_file = "{}/etc/network/interfaces.d/ens2".format(
-            temporary_directory
-        )
+        ens2_network_file = "{}/etc/network/interfaces.d/ens2".format(temp_dir)
         with open(ens2_network_file, "w") as fh:
             data = """auto ens2
 iface ens2 inet dhcp
@@ -634,7 +636,7 @@ iface ens2 inet dhcp
             fh.write(data)
 
         # Write the DHCP config for ens2
-        dhclient_file = "{}/etc/dhcp/dhclient.conf".format(temporary_directory)
+        dhclient_file = "{}/etc/dhcp/dhclient.conf".format(temp_dir)
         with open(dhclient_file, "w") as fh:
             # We can use fstrings too, since PVC will always have Python 3.6+, though
             # using format() might be preferable for clarity in some situations
@@ -654,7 +656,7 @@ interface "ens2" {{
             fh.write(data)
 
         # Write the GRUB configuration
-        grubcfg_file = "{}/etc/default/grub".format(temporary_directory)
+        grubcfg_file = "{}/etc/default/grub".format(temp_dir)
         with open(grubcfg_file, "w") as fh:
             data = """# Written by the PVC provisioner
 GRUB_DEFAULT=0
@@ -671,7 +673,7 @@ GRUB_DISABLE_LINUX_UUID=false
             fh.write(data)
 
         # Do some tasks inside the chroot using the provided context manager
-        with chroot(temporary_directory):
+        with chroot(temp_dir):
             # Install and update GRUB
             os.system(
                 "grub-install --force /dev/rbd/{}/{}_{}".format(
@@ -704,16 +706,17 @@ GRUB_DISABLE_LINUX_UUID=false
         """
 
         # Run any imports first
+        import os
         from pvcapid.vmbuilder import open_zk
         from pvcapid.Daemon import config
         import daemon_lib.common as pvc_common
         import daemon_lib.ceph as pvc_ceph
 
-        # Set the tempdir we used in the prepare() and install() steps
+        # Set the temp_dir we used in the prepare() and install() steps
         temp_dir = "/tmp/target"
 
         # Unmount the bound devfs
-        os.system("umount {}/dev".format(temporary_directory))
+        os.system("umount {}/dev".format(temp_dir))
 
         # Use this construct for reversing the list, as the normal reverse() messes with the list
         for volume in list(reversed(self.vm_data["volumes"])):

client-cli/pvc/cli/cli.py

@@ -166,32 +166,24 @@ def connection_req(function):
 def restart_opt(function):
     """
     Click Option Decorator:
-    Wraps a Click command which requires confirm_flag or unsafe option or asks for VM restart confirmation
+    Wraps a Click command which requires a VM domain restart, to provide options for/against restart or prompt
     """
 
     @click.option(
-        "-r",
-        "--restart",
+        "-r/-R",
+        "--restart/--no-restart",
         "restart_flag",
         is_flag=True,
-        default=False,
-        help="Immediately restart VM to apply changes.",
+        default=None,
+        show_default=False,
+        help="Immediately restart VM to apply changes or do not restart VM, or prompt if unspecified.",
     )
     @wraps(function)
     def confirm_action(*args, **kwargs):
-        confirm_action = True
-        if "restart_flag" in kwargs:
-            if not kwargs.get("restart_flag", False):
-                if not CLI_CONFIG.get("unsafe", False):
-                    confirm_action = True
-                else:
-                    confirm_action = False
-            else:
-                confirm_action = False
-        else:
-            confirm_action = False
+        restart_state = kwargs.get("restart_flag", None)
 
-        if confirm_action:
+        if restart_state is None:
+            # Neither "--restart" or "--no-restart" was passed: prompt for restart or restart if "--unsafe"
             try:
                 click.confirm(
                     f"Restart VM {kwargs.get('domain')} to apply changes",
@@ -202,6 +194,12 @@ def restart_opt(function):
             except Exception:
                 echo(CLI_CONFIG, "Changes will be applied on next VM start/restart.")
                 kwargs["restart_flag"] = False
+        elif restart_state is True:
+            # "--restart" was passed: allow restart without confirming
+            kwargs["restart_flag"] = True
+        elif restart_state is False:
+            # "--no-restart" was passed: skip confirming and skip restart
+            kwargs["restart_flag"] = False
 
         return function(*args, **kwargs)
 
@@ -5261,54 +5259,7 @@ def cli_provisioner_create(
 
     if retcode and wait_flag:
         task_id = retdata
-
-        echo("Task ID: {}".format(task_id))
-        echo("")
-
-        # Wait for the task to start
-        echo("Waiting for task to start...", nl=False)
-        while True:
-            time.sleep(1)
-            task_status = pvc.lib.provisioner.task_status(
-                CLI_CONFIG, task_id, is_watching=True
-            )
-            if task_status.get("state") != "PENDING":
-                break
-            echo(".", nl=False)
-        echo(" done.")
-        echo("")
-
-        # Start following the task state, updating progress as we go
-        total_task = task_status.get("total")
-        with click.progressbar(length=total_task, show_eta=False) as bar:
-            last_task = 0
-            maxlen = 0
-            while True:
-                time.sleep(1)
-                if task_status.get("state") != "RUNNING":
-                    break
-                if task_status.get("current") > last_task:
-                    current_task = int(task_status.get("current"))
-                    bar.update(current_task - last_task)
-                    last_task = current_task
-                    # The extensive spaces at the end cause this to overwrite longer previous messages
-                    curlen = len(str(task_status.get("status")))
-                    if curlen > maxlen:
-                        maxlen = curlen
-                    lendiff = maxlen - curlen
-                    overwrite_whitespace = " " * lendiff
-                    echo(
-                        "  " + task_status.get("status") + overwrite_whitespace,
-                        nl=False,
-                    )
-                task_status = pvc.lib.provisioner.task_status(
-                    CLI_CONFIG, task_id, is_watching=True
-                )
-            if task_status.get("state") == "SUCCESS":
-                bar.update(total_task - last_task)
-
-        echo("")
-        retdata = task_status.get("state") + ": " + task_status.get("status")
+        retdata = wait_for_provisioner(CLI_CONFIG, task_id)
 
     finish(retcode, retdata)
 
@@ -5597,7 +5548,7 @@ def cli_connection_detail(
     envvar="PVC_UNSAFE",
     is_flag=True,
     default=False,
-    help='Allow unsafe operations without confirmation/"--yes" argument.',
+    help='Perform unsafe operations without confirmation/"--yes" argument.',
 )
 @click.option(
     "--colour",

client-cli/pvc/cli/helpers.py

@@ -20,6 +20,7 @@
 ###############################################################################
 
 from click import echo as click_echo
+from click import progressbar
 from distutils.util import strtobool
 from json import load as jload
 from json import dump as jdump
@@ -27,9 +28,12 @@ from os import chmod, environ, getpid, path
 from socket import gethostname
 from sys import argv
 from syslog import syslog, openlog, closelog, LOG_AUTH
+from time import sleep
 from yaml import load as yload
 from yaml import BaseLoader
 
+import pvc.lib.provisioner
+
 
 DEFAULT_STORE_DATA = {"cfgfile": "/etc/pvc/pvcapid.yaml"}
 DEFAULT_STORE_FILENAME = "pvc.json"
@@ -178,3 +182,60 @@ def update_store(store_path, store_data):
 
     with open(store_file, "w") as fh:
         jdump(store_data, fh, sort_keys=True, indent=4)
+
+
+def wait_for_provisioner(CLI_CONFIG, task_id):
+    """
+    Wait for a provisioner task to complete
+    """
+
+    echo(CLI_CONFIG, f"Task ID: {task_id}")
+    echo(CLI_CONFIG, "")
+
+    # Wait for the task to start
+    echo(CLI_CONFIG, "Waiting for task to start...", newline=False)
+    while True:
+        sleep(1)
+        task_status = pvc.lib.provisioner.task_status(
+            CLI_CONFIG, task_id, is_watching=True
+        )
+        if task_status.get("state") != "PENDING":
+            break
+        echo(".", newline=False)
+    echo(CLI_CONFIG, " done.")
+    echo(CLI_CONFIG, "")
+
+    # Start following the task state, updating progress as we go
+    total_task = task_status.get("total")
+    with progressbar(length=total_task, show_eta=False) as bar:
+        last_task = 0
+        maxlen = 0
+        while True:
+            sleep(1)
+            if task_status.get("state") != "RUNNING":
+                break
+            if task_status.get("current") > last_task:
+                current_task = int(task_status.get("current"))
+                bar.update(current_task - last_task)
+                last_task = current_task
+                # The extensive spaces at the end cause this to overwrite longer previous messages
+                curlen = len(str(task_status.get("status")))
+                if curlen > maxlen:
+                    maxlen = curlen
+                lendiff = maxlen - curlen
+                overwrite_whitespace = " " * lendiff
+                echo(
+                    CLI_CONFIG,
+                    "  " + task_status.get("status") + overwrite_whitespace,
+                    newline=False,
+                )
+            task_status = pvc.lib.provisioner.task_status(
+                CLI_CONFIG, task_id, is_watching=True
+            )
+        if task_status.get("state") == "SUCCESS":
+            bar.update(total_task - last_task)
+
+    echo(CLI_CONFIG, "")
+    retdata = task_status.get("state") + ": " + task_status.get("status")
+
+    return retdata

client-cli/pvc/lib/vm.py

@@ -1017,13 +1017,13 @@ def vm_volumes_add(config, vm, volume, disk_id, bus, disk_type, live, restart):
     from lxml.objectify import fromstring
     from lxml.etree import tostring
     from copy import deepcopy
-    import pvc.lib.ceph as pvc_ceph
+    import pvc.lib.storage as pvc_storage
 
     if disk_type == "rbd":
         # Verify that the provided volume is valid
         vpool = volume.split("/")[0]
         vname = volume.split("/")[1]
-        retcode, retdata = pvc_ceph.ceph_volume_info(config, vpool, vname)
+        retcode, retdata = pvc_storage.ceph_volume_info(config, vpool, vname)
         if not retcode:
             return False, "Volume {} is not present in the cluster.".format(volume)
 

test-cluster.sh

@@ -1,31 +1,48 @@
 #!/usr/bin/env bash
 
-set -o errexit
-
 if [[ -z ${1} ]]; then
     echo "Please specify a cluster to run tests against."
     exit 1
 fi
 test_cluster="${1}"
+shift
+
+if [[ ${1} == "--test-dangerously" ]]; then
+    test_dangerously="y"
+else
+    test_dangerously=""
+fi
 
 _pvc() {
-    echo "> pvc --cluster ${test_cluster} $@"
-    pvc --quiet --cluster ${test_cluster} "$@"
+    echo "> pvc --connection ${test_cluster} $@"
+    pvc --quiet --connection ${test_cluster} "$@"
     sleep 1
 }
 
 time_start=$(date +%s)
 
+pushd $( git rev-parse --show-toplevel ) &>/dev/null
+
 # Cluster tests
-_pvc maintenance on
-_pvc maintenance off
+_pvc cluster maintenance on
+_pvc cluster maintenance off
 backup_tmp=$(mktemp)
-_pvc task backup --file ${backup_tmp}
-_pvc task restore --yes --file ${backup_tmp}
+_pvc cluster backup --file ${backup_tmp}
+if [[ -n ${test_dangerously} ]]; then
+    # This is dangerous, so don't test it unless option given
+    _pvc cluster restore --yes --file ${backup_tmp}
+fi
 rm ${backup_tmp} || true
 
 # Provisioner tests
-_pvc provisioner profile list test
+_pvc provisioner profile list test || true
+_pvc provisioner template system add --vcpus 1 --vram 1024 --serial --vnc --vnc-bind 0.0.0.0 --node-limit hv1 --node-selector mem --node-autostart --migration-method live system-test || true
+_pvc provisioner template network add network-test || true
+_pvc provisioner template network vni add network-test 10000 || true
+_pvc provisioner template storage add storage-test || true
+_pvc provisioner template storage disk add --pool vms --size 8 --filesystem ext4 --mountpoint / storage-test sda || true
+_pvc provisioner script add script-test $( find . -name "3-debootstrap.py" ) || true
+_pvc provisioner profile add --profile-type provisioner --system-template system-test --network-template network-test --storage-template storage-test --userdata empty --script script-test --script-arg deb_release=bullseye test || true
 _pvc provisioner create --wait testx test
 sleep 30
 
@@ -36,7 +53,7 @@ _pvc vm shutdown --yes --wait testx
 _pvc vm start testx
 sleep 30
 _pvc vm stop --yes testx
-_pvc vm disable testx
+_pvc vm disable --yes testx
 _pvc vm undefine --yes testx
 _pvc vm define --target hv3 --tag pvc-test ${vm_tmp}
 _pvc vm start testx
@@ -49,21 +66,21 @@ _pvc vm unmigrate --wait testx
 sleep 5
 _pvc vm move --wait --target hv1 testx
 sleep 5
-_pvc vm meta testx --limit hv1 --selector vms --method live --profile test --no-autostart
+_pvc vm meta testx --limit hv1 --node-selector vms --method live --profile test --no-autostart
 _pvc vm tag add testx mytag
 _pvc vm tag get testx
 _pvc vm list --tag mytag
 _pvc vm tag remove testx mytag
 _pvc vm network get testx
-_pvc vm vcpu set testx 4
+_pvc vm vcpu set --no-restart testx 4
 _pvc vm vcpu get testx
-_pvc vm memory set testx 4096
+_pvc vm memory set --no-restart testx 4096
 _pvc vm memory get testx
-_pvc vm vcpu set testx 2
+_pvc vm vcpu set --no-restart testx 2
 _pvc vm memory set testx 2048 --restart --yes
 sleep 5
 _pvc vm list testx
-_pvc vm info --long testx
+_pvc vm info --format long testx
 rm ${vm_tmp} || true
 
 # Node tests
@@ -78,6 +95,9 @@ _pvc node ready --wait hv1
 _pvc node list hv1
 _pvc node info hv1
 
+_pvc vm start testx
+sleep 30
+
 # Network tests
 _pvc network add 10001 --description testing --type managed --domain testing.local --ipnet 10.100.100.0/24 --gateway 10.100.100.1 --dhcp --dhcp-start 10.100.100.100 --dhcp-end 10.100.100.199
 sleep 5
@@ -95,7 +115,7 @@ _pvc network dhcp remove --yes 10001 12:34:56:78:90:ab
 
 _pvc network modify --domain test10001.local 10001
 _pvc network list
-_pvc network info --long 10001
+_pvc network info --format long 10001
 
 # Network-VM interaction tests
 _pvc vm network add testx 10001 --model virtio --restart --yes
@@ -109,17 +129,20 @@ _pvc network remove --yes 10001
 # Storage tests
 _pvc storage status
 _pvc storage util
-_pvc storage osd set noout
-_pvc storage osd out 0
-_pvc storage osd in 0
-_pvc storage osd unset noout
+if [[ -n ${test_dangerously} ]]; then
+    # This is dangerous, so don't test it unless option given
+    _pvc storage osd set noout
+    _pvc storage osd out 0
+    _pvc storage osd in 0
+    _pvc storage osd unset noout
+fi
 _pvc storage osd list
 _pvc storage pool add testing 64 --replcfg "copies=3,mincopies=2"
 sleep 5
 _pvc storage pool list
 _pvc storage volume add testing testx 1G
-_pvc storage volume resize testing testx 2G
-_pvc storage volume rename testing testx testerX
+_pvc storage volume resize --yes testing testx 2G
+_pvc storage volume rename --yes testing testx testerX
 _pvc storage volume clone testing testerX testerY
 _pvc storage volume list --pool testing
 _pvc storage volume snapshot add testing testerX asnapshotX
@@ -142,6 +165,14 @@ _pvc storage pool remove --yes testing
 _pvc vm stop --yes testx
 _pvc vm remove --yes testx
 
+_pvc provisioner profile remove --yes test
+_pvc provisioner script remove --yes script-test
+_pvc provisioner template system remove --yes system-test
+_pvc provisioner template network remove --yes network-test
+_pvc provisioner template storage remove --yes storage-test
+
+popd
+
 time_end=$(date +%s)
 
 echo