Merge lp:~raharper/curtin/trunk.vmtest-reuse-output into lp:~curtin-dev/curtin/trunk

[Ryan Harper]

Allow re-use of vmtest output

When developing or debugging vmtest unittest failures of collected data
it's useful to skip the install/boot/collect phases and just re-run the
unittest against a directory of collected output.

Introduce CURTIN_VMTEST_REUSE_TOPDIR setting to enable this. If the testcase directory is present then vmtest will skip vmlaunch/boot/collect
stages. If the directory is missing, the test runs as usual.

CURTIN_VMTEST_REUSE_TOPDIR=1 ./tools/jenkins-runner \
  tests/vmtests/test_basic.py:XenialTestBasic

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:472
https://jenkins.ubuntu.com/server/job/curtin-ci/389/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/389
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/389
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/389
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-amd64/389
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/389

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/389/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

add documentation of the new environment variable (as you have in the commit message)

the other question is that this change possibly hides failures ...
I'm just reading the diff in line and not thinking too much, but if an error causaed there to be no grub_config or reporting_config, it looks like we'd now just go on, where before we would fail a test.

review: Needs Fixing

[Ryan Harper (raharper)]

On Thu, Feb 23, 2017 at 2:18 PM, Scott Moser <email address hidden> wrote:

> Review: Needs Fixing
>
> add documentation of the new environment variable (as you have in the
> commit message)
>

ACK

>
> the other question is that this change possibly hides failures ...
> I'm just reading the diff in line and not thinking too much, but if an
> error causaed there to be no grub_config or reporting_config, it looks like
> we'd now just go on, where before we would fail a test.
>

we avoid *rewriting* files if they exist; I don't think that skips any
errors;

The basic concept is, if the tempdir has a cls.__name__ directory, to
rebuild the cls and TempDir paths to files that may get referenced in the
unittests;
however, if we're re-using, we don't want to rewrite the contents a second
time; so all of those checks are if not os.path.exists(foo): write_foo().

>
> --
> https://code.launchpad.net/~raharper/curtin/trunk.vmtest-
> reuse-output/+merge/318150
> You are the owner of lp:~raharper/curtin/trunk.vmtest-reuse-output.
>

[Scott Moser (smoser)]

On Thu, 23 Feb 2017, Ryan Harper wrote:

> On Thu, Feb 23, 2017 at 2:18 PM, Scott Moser <email address hidden> wrote:
>
> > Review: Needs Fixing
> >
> > add documentation of the new environment variable (as you have in the
> > commit message)
> >
>
> ACK
>
>
> >
> > the other question is that this change possibly hides failures ...
> > I'm just reading the diff in line and not thinking too much, but if an
> > error causaed there to be no grub_config or reporting_config, it looks like
> > we'd now just go on, where before we would fail a test.
> >
>
> we avoid *rewriting* files if they exist; I don't think that skips any
> errors;
>
> The basic concept is, if the tempdir has a cls.__name__ directory, to
> rebuild the cls and TempDir paths to files that may get referenced in the
> unittests;
> however, if we're re-using, we don't want to rewrite the contents a second
> time; so all of those checks are if not os.path.exists(foo): write_foo().
>

Thats sane then.
As I said, i just looked quick an thought maybe you were just going to
skip over errors.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:472
https://jenkins.ubuntu.com/server/job/curtin-ci/395/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/395
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/395
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/395
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/395
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/395

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/395/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:472
https://jenkins.ubuntu.com/server/job/curtin-ci/411/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/411
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/411
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/411
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/411
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/411

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/411/rebuild

review: Approve (continuous-integration)

[lp:~raharper/curtin/trunk.vmtest-reuse-output]

473. By Ryan Harper on 2017-04-03

merge from trunk

474. By Ryan Harper on 2017-04-04

Do not attempt to re-write launch configs when reusing result data

475. By Ryan Harper on 2017-04-05

merge from trunk

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:475
https://jenkins.ubuntu.com/server/job/curtin-ci/437/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/437
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/437
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/437
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/437
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/437

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/437/rebuild

review: Approve (continuous-integration)

[lp:~raharper/curtin/trunk.vmtest-reuse-output]

476. By Ryan Harper on 2017-04-19

merge from trunk

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:476
https://jenkins.ubuntu.com/server/job/curtin-ci/452/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/452
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/452
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/452
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/452
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/452

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/452/rebuild

review: Approve (continuous-integration)

[lp:~raharper/curtin/trunk.vmtest-reuse-output]

477. By Ryan Harper on 2017-05-15

merge from trunk

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:477
https://jenkins.ubuntu.com/server/job/curtin-ci/506/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/506
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/506
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/506
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/506

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/506/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

Over all this looks really good. I have some minor nits.
I do think we should respect/expect CURTIN_VMTEST_REUSE_TOPDIR in python in addition to tools/jenkins-runner.

[Ryan Harper (raharper)]

Thanks for the review, I'll clean up the mkdir stuff, make sure to use the environment variable to set cls.restored value instead of just the presence of the tempdir.

[lp:~raharper/curtin/trunk.vmtest-reuse-output]

478. By Ryan Harper on 2017-05-19

merge from trunk

479. By Ryan Harper on 2017-05-19

Use environment variable to set restored class attribute

480. By Ryan Harper on 2017-05-19

Fix exception message (include the tmpdir value)

481. By Ryan Harper on 2017-05-19

Reorder exception message

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:481
https://jenkins.ubuntu.com/server/job/curtin-ci/514/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/514
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/514
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/514
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/514
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/514

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/514/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

one tiny nit. fix that and i approve.

review: Approve

[lp:~raharper/curtin/trunk.vmtest-reuse-output]

482. By Ryan Harper on 2017-05-19

bool() REUSE_TOPDIR integer

483. By Ryan Harper on 2017-05-19

Add documentation of CURTIN_VMTEST_REUSE_TOPDIR

=== modified file 'doc/topics/integration-testing.rst'
--- doc/topics/integration-testing.rst	2017-04-26 16:27:20 +0000
+++ doc/topics/integration-testing.rst	2017-05-19 20:26:52 +0000
@@ -169,6 +169,15 @@
   If you set this value, you must ensure that the directory is either
   non-existent or clean.
 
+- ``CURTIN_VMTEST_REUSE_TOPDIR``: default 0
+
+  If this variable is set to 1, then vmtest will detect if the test
+  specified already exists in the configured ``CURTIN_VMTEST_TOPDIR``
+  location.  If present, vmtest will skip executing the install and
+  boot phase of vmtest, and install just execute the unittests 
+  specified.  This allows developers to re-run unittests on existing
+  data that's already been collected.
+
 - ``CURTIN_VMTEST_LOG``: default $TMPDIR/vmtest-<timestamp>.log
 
   Vmtest writes extended log information to this file.

=== modified file 'tests/vmtests/__init__.py'
--- tests/vmtests/__init__.py	2017-05-19 15:23:30 +0000
+++ tests/vmtests/__init__.py	2017-05-19 20:26:52 +0000
@@ -44,6 +44,7 @@
 OUTPUT_DISK_NAME = 'output_disk.img'
 BOOT_TIMEOUT = int(os.environ.get("CURTIN_VMTEST_BOOT_TIMEOUT", 300))
 INSTALL_TIMEOUT = int(os.environ.get("CURTIN_VMTEST_INSTALL_TIMEOUT", 3000))
+REUSE_TOPDIR = bool(int(os.environ.get("CURTIN_VMTEST_REUSE_TOPDIR", 0)))
 
 _TOPDIR = None
 
@@ -260,16 +261,9 @@
     output_disk = None
 
     def __init__(self, name, user_data):
+        self.restored = REUSE_TOPDIR
         # Create tmpdir
         self.tmpdir = os.path.join(_topdir(), name)
-        try:
-            os.mkdir(self.tmpdir)
-        except OSError as e:
-            if e.errno == errno.EEXIST:
-                raise ValueError("name '%s' already exists in %s" %
-                                 (name, _topdir))
-            else:
-                raise e
 
         # make subdirs
         self.collect = os.path.join(self.tmpdir, "collect")
@@ -280,12 +274,25 @@
 
         self.dirs = (self.collect, self.install, self.boot, self.logs,
                      self.disks)
+
+        self.success_file = os.path.join(self.logs, "success")
+        self.errors_file = os.path.join(self.logs, "errors.json")
+        self.target_disk = os.path.join(self.disks, "install_disk.img")
+        self.seed_disk = os.path.join(self.boot, "seed.img")
+        self.output_disk = os.path.join(self.boot, OUTPUT_DISK_NAME)
+
+        if self.restored:
+            if os.path.exists(self.tmpdir):
+                logger.info('Reusing existing TempDir: %s', self.tmpdir)
+                return
+            else:
+                raise ValueError("REUSE_TOPDIR set, but TempDir not found:"
+                                 " %s" % self.tmpdir)
+
+        os.mkdir(self.tmpdir)
         for d in self.dirs:
             os.mkdir(d)
 
-        self.success_file = os.path.join(self.logs, "success")
-        self.errors_file = os.path.join(self.logs, "errors.json")
-
         # write cloud-init for installed system
         meta_data_file = os.path.join(self.install, "meta-data")
         with open(meta_data_file, "w") as fp:
@@ -296,21 +303,18 @@
 
         # create target disk
         logger.debug('Creating target disk')
-        self.target_disk = os.path.join(self.disks, "install_disk.img")
         subprocess.check_call(["qemu-img", "create", "-f", TARGET_IMAGE_FORMAT,
                               self.target_disk, "10G"],
                               stdout=DEVNULL, stderr=subprocess.STDOUT)
 
         # create seed.img for installed system's cloud init
         logger.debug('Creating seed disk')
-        self.seed_disk = os.path.join(self.boot, "seed.img")
         subprocess.check_call(["cloud-localds", self.seed_disk,
                               user_data_file, meta_data_file],
                               stdout=DEVNULL, stderr=subprocess.STDOUT)
 
         # create output disk, mount ro
         logger.debug('Creating output disk')
-        self.output_disk = os.path.join(self.boot, OUTPUT_DISK_NAME)
         subprocess.check_call(["qemu-img", "create", "-f", TARGET_IMAGE_FORMAT,
                               self.output_disk, "10M"],
                               stdout=DEVNULL, stderr=subprocess.STDOUT)
@@ -549,7 +553,6 @@
         cls.boot_log = os.path.join(cls.td.logs, 'boot-serial.log')
         logger.debug('Install console log: {}'.format(cls.install_log))
         logger.debug('Boot console log: {}'.format(cls.boot_log))
-        ftypes = cls.get_test_files()
 
         # if interactive, launch qemu without 'background & wait'
         if cls.interactive:
@@ -569,6 +572,7 @@
             cmd.extend(["--append=" + cls.extra_kern_args])
 
         # publish the root tarball
+        ftypes = cls.get_test_files()
         install_src = "PUBURL/" + os.path.basename(ftypes[cls.target_ftype])
         if cls.target_ftype == 'vmtest.root-tgz':
             cmd.append("--publish=%s" % ftypes['vmtest.root-tgz'])
@@ -652,10 +656,11 @@
         # proxy config
         configs = [cls.conf_file]
         cls.proxy = get_apt_proxy()
-        if cls.proxy is not None:
+        if cls.proxy is not None and not cls.td.restored:
             proxy_config = os.path.join(cls.td.install, 'proxy.cfg')
-            with open(proxy_config, "w") as fp:
-                fp.write(json.dumps({'apt_proxy': cls.proxy}) + "\n")
+            if not os.path.exists(proxy_config):
+                with open(proxy_config, "w") as fp:
+                    fp.write(json.dumps({'apt_proxy': cls.proxy}) + "\n")
             configs.append(proxy_config)
 
         uefi_flags = []
@@ -666,8 +671,9 @@
 
             # always attempt to update target nvram (via grub)
             grub_config = os.path.join(cls.td.install, 'grub.cfg')
-            with open(grub_config, "w") as fp:
-                fp.write(json.dumps({'grub': {'update_nvram': True}}))
+            if not os.path.exists(grub_config):
+                with open(grub_config, "w") as fp:
+                    fp.write(json.dumps({'grub': {'update_nvram': True}}))
             configs.append(grub_config)
 
         if cls.dirty_disks and storage_config:
@@ -690,20 +696,21 @@
         reporting_config = os.path.join(cls.td.install, 'reporting.cfg')
         localhost_url = ('http://' + get_lan_ip() +
                          ':{:d}/'.format(reporting_logger.port))
-        with open(reporting_config, 'w') as fp:
-            fp.write(json.dumps({
-                'install': {
-                    'log_file': '/tmp/install.log',
-                    'post_files': ['/tmp/install.log'],
-                },
-                'reporting': {
-                    'maas': {
-                        'level': 'DEBUG',
-                        'type': 'webhook',
-                        'endpoint': localhost_url,
-                    },
-                },
-            }))
+        if not os.path.exists(reporting_config) and not cls.td.restored:
+            with open(reporting_config, 'w') as fp:
+                fp.write(json.dumps({
+                    'install': {
+                        'log_file': '/tmp/install.log',
+                        'post_files': ['/tmp/install.log'],
+                    },
+                    'reporting': {
+                        'maas': {
+                            'level': 'DEBUG',
+                            'type': 'webhook',
+                            'endpoint': localhost_url,
+                        },
+                    },
+                }))
         configs.append(reporting_config)
 
         cmd.extend(uefi_flags + netdevs + disks +
@@ -713,6 +720,11 @@
                    ["--config=%s" % f for f in configs] +
                    [install_src])
 
+        # don't run the vm stages if we're just re-running testcases on output
+        if cls.td.restored:
+            logger.info('Using existing outputdata, skipping install/boot')
+            return
+
         # run vm with installer
         lout_path = os.path.join(cls.td.logs, "install-launch.log")
         logger.info('Running curtin installer: {}'.format(cls.install_log))

=== modified file 'tools/jenkins-runner'
--- tools/jenkins-runner	2017-02-17 21:02:20 +0000
+++ tools/jenkins-runner	2017-05-19 20:26:52 +0000
@@ -3,6 +3,8 @@
 topdir="${CURTIN_VMTEST_TOPDIR:-${WORKSPACE:-$PWD}/output}"
 pkeep=${CURTIN_VMTEST_KEEP_DATA_PASS:-logs,collect}
 fkeep=${CURTIN_VMTEST_KEEP_DATA_FAIL:-logs,collect}
+reuse=${CURTIN_VMTEST_REUSE_TOPDIR:-0}
+export CURTIN_VMTEST_REUSE_TOPDIR=$reuse
 export CURTIN_VMTEST_IMAGE_SYNC=${CURTIN_VMTEST_IMAGE_SYNC:-0}
 export CURTIN_VMTEST_KEEP_DATA_PASS=$pkeep
 export CURTIN_VMTEST_KEEP_DATA_FAIL=$fkeep
@@ -13,10 +15,12 @@
 
 fail() { echo "$@" 1>&2; exit 1; }
 
-if [ -d "$topdir" ]; then
-    fail "topdir '$topdir' existed."
+if [ "$reuse" != "1" ]; then
+    if [ -d "$topdir" ]; then
+        fail "topdir '$topdir' existed."
+    fi
+    mkdir -p "$topdir" || fail "failed mkdir $topdir"
 fi
-mkdir -p "$topdir" || fail "failed mkdir $topdir"
 
 start_s=$(date +%s)
 parallel=${CURTIN_VMTEST_PARALLEL}