kickstart-tests: fix up several tests

this gets quite a few more tests working. We now handle crypted
disks, tests with czech keyboard layout, and the proxy tests. We
also handle the package tests that boot to lightdm because they
install Xfce, by allowing forced console login after graphical
boot with FORCE_CONSOLE_LOGIN.

We also allow (require, in fact) specifying URLs for the HTTP
and NFS test repos (we don't do anything for generating those,
you're expected to do it with the script from kickstart-tests).

We now handle the filesystem type tests when run on a Server
image (previously they would fail because default FS on those
is xfs while the kickstart test expects ext4). We workaround
the kickstart test 'fail' in the openQA test for now, as fixing
the kickstart test is actually quite complex (detecting a Server
install from a %post script is not super clean).

Note these changes depend on kickstart-tests PRs #14 and #15 to
some extent (without #14 in fact the 'kickstarts' command will
die in a fire, as it expects the Makefile from that PR to be
present). Without #15 the reqpart test may fail.

kstest-converter

@@ -5,6 +5,10 @@ import json
 import logging
 import os
 import re
+import shutil
+import subprocess
+import sys
+import warnings
 
 SKIPS = [
     # we haven't really figured out how to do NFS in openQA yet
@@ -13,18 +17,14 @@ SKIPS = [
     'liveimg',
     # i'll figure this one out later
     'basic-ostree',
+    # doesn't install enough bits for the system to be bootable
+    'container',
     # FIXMEs:
-    # keyboard - changes keyboard layout, fucks with openQA's script runner
     # hostname - changes hostname, breaks 'text_console_login' needle
-    # packages-and-groups-1 - boots to GUI login
+    # raid-1 - only works on Rawhide (not 24)
-    # encrypt-device - need to set ENCRYPT_PASSWORD, also 'kickstart insufficient'?
+    # default-fstype, reqpart - trip up on xfs vs. ext4 with Server image
-]
+    # driverdisk-disk - I didn't hook up image generation at all yet
-
+    # proxy-cmdline, proxy-auth - require generation of some test repos
-SUBSTS = [
-    ('@KSTEST_URL@', '--mirrorlist=https://mirrors.fedoraproject.org/mirrorlist?repo=fedora-$releasever&arch=$basearch'),
-    ('@KSTEST_FTP_URL@', 'ftp://mirror.utexas.edu/pub/fedora/linux/development/rawhide/Everything/$basearch/os/'),
-    # we need to reboot not shutdown for openQA
-    ('shutdown', 'reboot')
 ]
 
 def _find_tests(dir):
@@ -38,36 +38,105 @@ def _find_tests(dir):
     # strip .ks.in
     return [ksin.replace('.ks.in', '') for ksin in ksins]
 
-def prep_kickstarts(indir, outdir):
+def _get_texts(path, test):
-    """Produce kickstarts in 'outdir' from .ks.in files in 'indir'.
+    """Return the text of the .ks.in and .sh files."""
+    # get text of .sh and .ks.in files
+    with open('{0}/{1}.sh'.format(path, test), 'r') as shfh:
+        sh = shfh.read()
+    with open('{0}/{1}.ks.in'.format(path, test), 'r') as ksinfh:
+        ksin = ksinfh.read()
+    return (sh, ksin)
+
+def _kickstart_substitutions(ksin, ksurl, httprepo, nfsrepo, kstesturl=None, kstestftpurl=None):
+    """Do the various substitutions necessary on .ks.in files. ksin is
+    the text of the file. 'ksurl' is the URL to the path where the
+    produced kickstarts will be available via HTTP; this is needed for
+    the 'ks-include' test which tests including another kickstart
+    file. 'kstesturl' is the text to replace @KSTEST_URL@ with - it
+    should be the '--mirrorlist=(url)' or '--url=(url)' component of a
+    kickstart 'repo' line. kstestftpurl is, similarly, the replacement
+    text for @KSTEST_FTP_URL@. If not set, defaults are used.
     """
+    if not kstesturl:
+        kstesturl = '--mirrorlist=https://mirrors.fedoraproject.org/mirrorlist?repo=fedora-$releasever&arch=$basearch'
+    if not kstestftpurl:
+        kstestftpurl = 'ftp://mirror.utexas.edu/pub/fedora/linux/development/rawhide/Everything/$basearch/os/'
+
+    ksin = ksin.replace('@KSTEST_URL@', kstesturl)
+    ksin = ksin.replace('@KSTEST_FTP_URL@', kstestftpurl)
+    ksin = ksin.replace('@KSTEST_HTTP_ADDON_REPO@', httprepo)
+    # this has two different placeholders, for some reason...
+    ksin = ksin.replace('HTTP-ADDON-REPO', httprepo)
+    ksin = ksin.replace('@KSTEST_NFS_ADDON_REPO@', nfsrepo)
+    # we always want to reboot, not shutdown
+    ksin = ksin.replace('shutdown', 'reboot')
+
+    # handle includes. note this means we're testing a slightly
+    # different path to the kickstart-tests runner: it tests
+    # inclusion of a local file while we test inclusion of an HTTP
+    # served file.
+    ksin = ksin.replace('KS-TEST-INCLUDE', "{0}/{1}".format(ksurl, 'ks-include-post.ks'))
+    return ksin
+
+def prep_kickstarts(indir, ksurl, httprepo, nfsrepo, outdir, kstesturl=None, kstestftpurl=None):
+    """Produce kickstarts in 'outdir' from .ks.in files in 'indir'.
+    'ksurl' is the URL to the path where the produced kickstarts
+    will be available via HTTP; this is needed for the 'ks-include'
+    test which tests including another kickstart file. 'kstesturl'
+    is the text to replace @KSTEST_URL@ with - it should be the
+    '--mirrorlist=(url)' or '--url=(url)' component of a kickstart
+    'repo' line. kstestftpurl is, similarly, the replacement text
+    for @KSTEST_FTP_URL@. If not set, defaults are used.
+    """
+    if not kstesturl:
+        kstesturl = '--mirrorlist=https://mirrors.fedoraproject.org/mirrorlist?repo=fedora-$releasever&arch=$basearch'
+    if not kstestftpurl:
+        kstestftpurl = 'ftp://mirror.utexas.edu/pub/fedora/linux/development/rawhide/Everything/$basearch/os/'
     if not os.path.isdir(outdir):
         raise ValueError("Output directory {0} does not exist!".format(outdir))
+    # probably safest
+    indir = os.path.abspath(indir)
+
+    # build Makefile-driven .ks.ins
+    ret = subprocess.call(('make', '-s'), cwd=indir)
     tests = _find_tests(indir)
     if not tests:
         raise ValueError("No tests found!")
+
     for test in tests:
-        # read in the .ks.in file
+        (sh, ksin) = _get_texts(indir, test)
-        with open('{0}/{1}.ks.in'.format(indir, test), 'r') as ksinfh:
+        # do standard substitutions
-            kstext = ksinfh.read()
+        ksin = _kickstart_substitutions(
-        for (orig, repl) in SUBSTS:
+            ksin, ksurl, httprepo, nfsrepo, kstesturl=kstesturl, kstestftpurl=kstestftpurl)
-            kstext = kstext.replace(orig, repl)
+
+        # replace '(non-alpha)sd(alpha)(non-alpha)' with '(non-alpha)
+        # vd(alpha)(non-alpha)' (sda -> vda etc). This is because
+        # openQA's default way of attaching disks results in them
+        # being vdX while the kickstart-tests runner's way makes them
+        # sdX. we could maybe harmonize those somehow instead of doing
+        # this.
+        ksin = re.sub(r'([^a-zA-Z])s(d[a-z][^a-zA-Z])', r"\1v\2", ksin)
+
         # write out the processed .ks
         ksout = "{0}.ks".format(test)
         with open('{0}/{1}'.format(outdir, ksout), 'w') as ksoutfh:
-            ksoutfh.write(kstext)
+            ksoutfh.write(ksin)
 
-def merge_templates(indir, baseurl, tempfile, outfile):
+    # copy the 'include' test kickstart straight over.
+    shutil.copy('{0}/{1}'.format(indir, 'ks-include-post.ks'),
+                '{0}/{1}'.format(outdir, 'ks-include-post.ks'))
+
+def merge_templates(indir, ksurl, tempfile, outfile):
     """Produce openQA test suites and job templates for all tests in
     indir, merge them with the existing openQA templates file
     'tempfile', and write the combined templates file as 'outfile'.
-    'baseurl' is the URL to the path where the kickstart files for
+    'ksurl' is the URL to the path where the kickstart files for
     the tests can be found.
     """
     tests = _find_tests(indir)
     if not tests:
         raise ValueError("No tests found!")
-    testsuites = [create_testsuite(test, indir, baseurl) for test in tests]
+    testsuites = [create_testsuite(test, indir, ksurl) for test in tests]
     with open(tempfile, 'r') as tempfh:
         templates = json.loads(tempfh.read())
     templates = merge_testsuites(templates, testsuites)
@@ -118,21 +187,63 @@ def _get_settings_keyboard(ksin):
     settings (currently a single-item list, but we're future-proofing
     here).
     """
-    # if the kickstart sets a keyboard layout, we set VNCKB to the
+    # if the kickstart sets a keyboard layout, we set the LOADKEYS
-    # same layout. This tells openQA to run qemu with '-k (layout)',
+    # var to tell openQA to do 'loadkeys us' after login and LOGIN_
-    # which makes it use that layout for converting keysyms received
+    # KEYMAP to tell it to use the specified keymap for logging in
-    # via VNC to keycodes which are passed on to the guest OS, which
+    # and running loadkeys.
-    # converts them back into keysyms. Basically if we want to set a
-    # non-US layout in the guest and have 'type_string qwerty' still
-    # type 'qwerty' and not 'qwertz' or 'azert' or something, this is
-    # how we do that.
     match = re.search(r'--vckeymap (\S+)', ksin)
     if match:
-        return [{'key': 'VNCKB', 'value': match.group(1)}]
+        return [
+            {'key': 'LOADKEYS', 'value': 1},
+            {'key': 'LOGIN_KEYMAP', 'value': match.group(1)},
+        ]
     else:
         return []
 
-def create_testsuite(test, path, baseurl):
+def _get_settings_encrypt(ksin):
+    """Given text of a kickstart_tests .ks.in file as ksin, return
+    a list of appropriate openQA settings dict for disk encryption
+    settings (currently a single-item list, but we're future-proofing
+    here).
+    """
+    # the (?:) are non-capturing groups. We only care about the second
+    # group. The fun at the end is to capture a passphrase if it's
+    # inside quotes, and to stop matching when we reach either a white
+    # space character or the end of the line.
+    match = re.search(r"(?:logvol|part).*--passphrase='?(\S+?)'?(?:\s|$)", ksin)
+    if match:
+        return [{'key': 'ENCRYPT_PASSWORD', 'value': match.group(1)}]
+    else:
+        return []
+
+def _get_settings_boot(sh, test, ksurl):
+    """Given text of a kickstart_tests test (.sh file) as sh, the test
+    name as test and the kickstar base URL as ksurl, return a list
+    of appropriate openQA settings related to the bootloader. Usually
+    we just set inst.ks, but kickstart-tests has capacity for setting
+    other params, we must handle that; used by proxy-cmdline for e.g.
+    """
+    value = "inst.ks={0}/{1}.ks".format(ksurl.strip('/'), test)
+    # ok, this is an awful excuse for parsing. sorry. It does handle
+    # comments! it'll break if the function is not just a single line
+    # of args, though.
+    lines = sh.splitlines()
+    args = []
+    for (num, line) in enumerate(lines):
+        # start of the 'kernel_args' function that prints the args
+        if line.startswith('kernel_args()'):
+            # take the next line (we assume that's 'echo arg1 arg2...')
+            args = lines[num+1].strip().split()
+    if args:
+        # split the line and ditch some args we don't want to take
+        args = [arg for arg in args if arg not in ('echo', 'vnc')]
+        # stick the inst.ks arg on the end of the list, and...
+        args.append(value)
+        # ...join it back together
+        value = ' '.join(args)
+    return [{'key': 'GRUB', 'value': value}]
+
+def create_testsuite(test, path, ksurl):
     """Create an openQA 'test suite' for a given kickstart_test. test
     is the test name, path is the directory the test files are in.
     """
@@ -140,26 +251,29 @@ def create_testsuite(test, path, baseurl):
     settings = [{'key': 'KICKSTART', 'value': '1'}]
 
     # get text of .sh and .ks.in files
-    with open('{0}/{1}.sh'.format(path, test), 'r') as shfh:
+    (sh, ksin) = _get_texts(path, test)
-        sh = shfh.read()
-    with open('{0}/{1}.ks.in'.format(path, test), 'r') as ksinfh:
-        ksin = ksinfh.read()
 
     # call all the functions for getting different settings.
     settings.extend(_get_settings_disk(sh))
     settings.extend(_get_settings_postinstall(sh))
     settings.extend(_get_settings_keyboard(ksin))
+    settings.extend(_get_settings_encrypt(ksin))
+    settings.extend(_get_settings_boot(sh, test, ksurl))
 
     # do some very simple ones ourselves (to avoid this function
-    # growing too much, the rule is that if it needs more than one
+    # growing too much, the rule is that if it needs more than two
-    # line, split it out).
+    # lines, split it out).
 
     # root password. for now we assume there is one.
     settings.append({'key': 'ROOT_PASSWORD', 'value': re.search(r'rootpw (.+)', ksin).group(1)})
-    # kickstart URL
-    settings.append({'key': 'GRUB', 'value': "inst.ks={0}/{1}.ks".format(baseurl.strip('/'), test)})
     # we never want to do a user login for these
     settings.append({'key': 'USER_LOGIN', 'value': "false"})
+    # these install xfce so they boot to lightdm, force console login
+    if test in ('groups-and-envs-2', 'packages-and-groups-1'):
+        settings.append({'key': 'FORCE_CONSOLE_LOGIN', 'value': "1"})
+    # we have to do an ugly success check workaround for these
+    if test in ('reqpart', 'default-fstype'):
+        settings.append({'key': 'KSTEST_SERVER_FSTYPE', 'value': "1"})
     return {'name': "kstest_{0}".format(test), 'settings': settings}
 
 def merge_testsuites(templates, testsuites, machine='64bit', arch='x86_64',
@@ -208,8 +322,15 @@ def cmd_kickstarts(args):
     """kickstarts subcommand function: produce kickstarts from .ks.in
     files.
     """
+    if args.mirrorlist:
+        kstesturl = '--mirrorlist={0}'.format(args.mirrorlist)
+    elif args.http:
+        kstesturl = '--url={0}'.format(args.http)
+    else:
+        kstesturl = None
     try:
-        prep_kickstarts(args.indir, args.outdir)
+        prep_kickstarts(args.indir, args.ksurl, args.httprepo, args.nfsrepo, args.outdir,
+                        kstesturl, args.ftp)
     except ValueError as err:
         sys.exit(err)
 
@@ -218,7 +339,7 @@ def cmd_templates(args):
     job templates and merge into a templates file.
     """
     try:
-        merge_templates(args.indir, args.baseurl, args.tempfile, args.outfile)
+        merge_templates(args.indir, args.ksurl, args.tempfile, args.outfile)
     except ValueError as err:
         sys.exit(err)
 
@@ -247,8 +368,29 @@ def parse_args():
         "files and write them to a specified directory.")
     parser_kickstarts.add_argument(
         'indir', help="Input directory (containing .ks.in files)")
+    parser_kickstarts.add_argument(
+        'ksurl', help="URL to path where generated .ks files will be "
+        "available")
+    parser_kickstarts.add_argument(
+        'httprepo', help="URL for the HTTP repository required for additional "
+        "repository tests (created with make-addon-pkgs.py). MUST BE HTTP "
+        "not HTTPS for proxy tests to work")
+    parser_kickstarts.add_argument(
+        'nfsrepo', help="URL for the NFS repository required for additional "
+        "repository tests (created with make-addon-pkgs.py)")
     parser_kickstarts.add_argument(
         'outdir', help="Output directory (where .ks files are written")
+    parser_kickstarts.add_argument(
+        '--mirrorlist', '-m', help="Mirror list URL to use as the base repo "
+        "for HTTP repo tests, instead of the default (the official mirror "
+        "list)")
+    parser_kickstarts.add_argument(
+        '--http', help="Direct mirror URL to use as the base repo for "
+        "HTTP repo tests, instead of the default (the official mirror "
+        "list)")
+    parser_kickstarts.add_argument(
+        '--ftp', help="FTP URL to use as the base repo for FTP repo "
+        "tests, instead of the default (a server in Texas, USA)")
     parser_kickstarts.set_defaults(func=cmd_kickstarts)
 
     parser_templates = subparsers.add_parser(
@@ -258,7 +400,7 @@ def parse_args():
     parser_templates.add_argument(
         'indir', help="Input directory (containing .ks.in and .sh files)")
     parser_templates.add_argument(
-        'baseurl', help="URL to a directory containing .ks files (as "
+        'ksurl', help="URL to a directory containing .ks files (as "
         "produced by 'kickstarts' subcommand)")
     parser_templates.add_argument(
         'tempfile', help="Path to openQA templates file (must be JSON "
@@ -272,6 +414,12 @@ def parse_args():
 def main():
     """Main loop."""
     try:
+        # check for needed commands
+        for (cmd, pkg) in (('createrepo_c', 'createrepo_c'),
+                           ('make', 'make')):
+            if not shutil.which(cmd):
+                sys.exit("Required command {0} not found! Please install it (try `dnf install "
+                        "{1}`)".format(cmd, pkg))
         args = parse_args()
         loglevel = getattr(
             logging, args.loglevel.upper(), logging.INFO)

lib/fedorabase.pm

@@ -94,6 +94,27 @@ sub get_milestone {
     return '';
 }
 
+sub keymap_string {
+    # sometimes we want to set the guest OS keyboard layout to czech.
+    # this messes up openQA 'send_key' and 'type_string'. in theory we
+    # can fix this with VNCKB (which sets the '-k' arg for qemu) but
+    # that doesn't really work as the guest OS keymap will be US on
+    # the boot screen, early in anaconda, etc. so instead we have this
+    # awful thing, which parses strings so when you type *most* things
+    # with the guest OS keymap set to cz, they'll turn out right. This
+    # is ugly and shouldn't be relied on too hard, the best thing to
+    # do is use it only to log in and run 'loadkeys us'. Written to be
+    # extensible to other keymaps, for now I've only done cz.
+    my $self = shift;
+    my $string = shift;
+    my $keymap = shift;
+    if ($keymap eq 'cz') {
+        # FIXME: pipes are altgr+w, which is nothing in US...problem.
+        $string =~ tr,yz\-=?":'_;,zy/\-<:>|?`,;
+    }
+    return $string;
+}
+
 1;
 
 # vim: set sw=4 et:

needles/graphical_login_lightdm.json

@@ -0,0 +1,18 @@
+{
+    "area": [
+        {
+            "height": 83,
+            "type": "match",
+            "width": 69,
+            "xpos": 368,
+            "ypos": 324
+        }
+    ],
+    "properties": [],
+    "tags": [
+        "graphical_login",
+        "DESKTOP-xfce",
+        "DESKTOP-lxde",
+        "LANGUAGE-english"
+    ]
+}

tests/_console_wait_login.pm

@@ -2,22 +2,45 @@ use base "fedorabase";
 use strict;
 use testapi;
 
+sub _map_string {
+    my $self = shift;
+    my $string = shift;
+    if (get_var("LOGIN_KEYMAP")) {
+        $string = $self->keymap_string($string, get_var("LOGIN_KEYMAP"));
+    }
+    return $string;
+}
+
 sub run {
     my $self = shift;
 
-    # If KICKSTART is set, then the wait_time needs to
+    # If KICKSTART is set and ENCRYPT_PASSWORD is not, then the
-    #  consider the install time
+    # wait_time needs to consider the install time
-    my $wait_time = get_var("KICKSTART") ? 1800 : 300;
+    my $wait_time = (get_var("KICKSTART") && !(get_var("ENCRYPT_PASSWORD"))) ? 1800 : 300;
 
-    # Reboot and wait for the text login
+    # FORCE_CONSOLE_LOGIN tells us to wait for a graphical login
+    # then switch to a console
+    if (get_var("FORCE_CONSOLE_LOGIN")) {
+        assert_screen "graphical_login", $wait_time;
+        $wait_time = 20;
+        send_key "ctrl-alt-f3";
+    }
+
+    # Wait for the text login
     assert_screen "text_console_login", $wait_time;
 
     # do user login unless USER_LOGIN is set to string 'false'
-    unless (get_var("USER_LOGIN") eq "false") {
+    my $user = get_var("USER_LOGIN", "test");
-        $self->console_login(user=>get_var("USER_LOGIN", "test"), password=>get_var("USER_PASSWORD", "weakpassword"));
+    unless ($user eq "false") {
+        my $userpass = get_var("USER_PASSWORD", "weakpassword");
+        $self->console_login(user=>$self->_map_string($user), password=>$self->_map_string($userpass));
     }
     if (get_var("ROOT_PASSWORD")) {
-        $self->console_login(user=>"root", password=>get_var("ROOT_PASSWORD"));
+        $self->console_login(user=>$self->_map_string("root"), password=>$self->_map_string(get_var("ROOT_PASSWORD")));
+    }
+    # if requested, load US keymap.
+    if (get_var("LOADKEYS")) {
+        type_string $self->_map_string("loadkeys us\n");
     }
 }
 

tests/disk_guided_encrypted_postinstall.pm

@@ -4,7 +4,11 @@ use testapi;
 
 sub run {
     # decrypt disks during boot
-    assert_screen "boot_enter_passphrase", 300; #
+
+    # If KICKSTART is set, then the wait_time needs to
+    #  consider the install time
+    my $wait_time = get_var("KICKSTART") ? 1800 : 300;
+    assert_screen "boot_enter_passphrase", $wait_time;
     type_string get_var("ENCRYPT_PASSWORD");
     send_key "ret";
 }

tests/kstest_root_postinstall.pm

@@ -7,7 +7,24 @@ sub run {
     if (not( check_screen "root_console", 0)) {
         $self->root_console(tty=>3);
     }
-    validate_script_output 'cat /root/RESULT', sub { $_ =~ m/SUCCESS/ };
+    # As a special case, we treat 'default fstype is incorrect (got xfs;
+    # expected ext4)' as a pass when running the two 'default filesystem'
+    # tests on Server images, because xfs is the default filesystem for
+    # Server. This is complex to fix in the tests and I'm not sure
+    # anaconda would accept the PR. We use a very precise match for this
+    # case to make sure we don't pass if the /boot filesystem is wrong.
+    # Using 'ISO' here is kind of a hack, we might want to pass in subv
+    # from the scheduler for all jobs or something.
+    my @elems = split('-', get_var('ISO'));
+    my $subv = $elems[1];
+    if (get_var('KSTEST_SERVER_FSTYPE') && $subv eq 'Server') {
+        validate_script_output 'cat /root/RESULT', sub { $_ eq "default fstype is incorrect (got xfs; expected ext4)" };
+    }
+    else {
+        # we can't use ^SUCCESS$ because sometimes there are messages
+        # like 'SUCCESS but (some non-critical issue)'
+        validate_script_output 'cat /root/RESULT', sub { $_ =~ m/^SUCCESS/ };
+    }
 }
 
 sub test_flags {