Commit Graph

394 Commits

Author SHA1 Message Date
Colin Walters
5037c85330 tests: Extend staging to cover vmcheck/upgrades
This now works after https://github.com/ostreedev/ostree/pull/1570

However, I was hitting an issue with the temp httpd running the
tests serially, let's split them up.

Really the only sane thing is to run each of these tests in a fresh
VM; we'll do the VM-in-container pattern here hopefully soon.

Closes: #1355
Approved by: jlebon
2018-05-02 19:20:26 +00:00
Colin Walters
506910d930 Add an experimental option to use libostree's "staging" API
Now that infrastructure for this has landed in libostree,
let's make it easy for people to opt-in to testing it.  This is a distinct first
step for adding it as an update policy.

Closes: #1352
Approved by: jlebon
2018-05-02 13:46:37 +00:00
Jonathan Lebon
ee458c3c50 daemon/upgrade: Write out new cached update
Right now, cached updates generated during "check" policy runs are
completely decoupled from upgrade operations. This can lead
to the surprising situation where the "Available update" is *older* than
a freshly deployed pending tree with `rpm-ostree upgrade`.

We should just generate a cached update after upgrade operations. This
is also prep for staged deployments, where we'll want to do this as
well.

Note that we write out the cached update here even if automatic updates
are turned off since it's essentially free. I've been thinking about
always displaying that information after an `rpm-ostree upgrade` in
`status`. Though not sure if we should keep it in a separate "Available
update" section, or somehow morph it as part of the pending deployment
output.

Closes: #1344
Approved by: cgwalters
2018-04-26 21:16:52 +00:00
Colin Walters
8726be65e3 core: Finish implementing `override replace ./kernel*.x86_64.rpm
Previously we merged: #1228 AKA 12dc565b00
My recollection is that was working on it the background, while doing
something else, and I clearly didn't get to the point of testing it "for real".

There are many interlocking issues here to make this work.  For example,
the "remove RPM" logic needs special handling for the kernel, because
we also inject content into `/usr/lib/ostree-boot` and also generate
the initramfs, etc.

The architecture I chose is to have the core *detect* when a kernel
is changed, and also call into the kernel processing code when removing
a kernel package.  But the logic for doing kernel reinstallation client-side
is best alongside the initramfs generation logic which already existed
in the sysroot upgrader.

I extended the test suite to cover what was failing before, and I
tested this interactively.  But I'm uncertain about adding a test
for actually *booting* into the GA kernel as it's quite possible
some bits in userspace rely on a newer kernel.  Fixing this properly
really wants some infrastructure to better "re-version" an existing
package without changing its content.

Closes: https://github.com/projectatomic/rpm-ostree/issues/1334

Closes: #1346
Approved by: jlebon
2018-04-26 16:49:53 +00:00
Colin Walters
9df2428a1a tests: Move upgrade/rebase tests from unit to vmcheck/test-upgrades
Now we stop running rpm-ostreed as non-root, which is going to be
a maintenance pain going forward.  If we do introduce non-VM based
tests I think we should look to doing in-container testing.

Closes: #1339
Approved by: cgwalters
2018-04-25 20:28:56 +00:00
Colin Walters
a50a6cb959 tests: Move more update-generation bits to libvm.sh
The autoupdate test had a lot of useful infrastructure.  Prep
for removing `tests/check/test-upgrade-rebase.sh`.

Closes: #1339
Approved by: cgwalters
2018-04-25 20:28:56 +00:00
Colin Walters
1521dc32c9 status: Show rpmmd-repo data in verbose mode
Building on:

 - 9cbec27d4c
 - e7a42f70a9

I was looking at a rpm-ostree run that imports a variety of rpmmd-repos,
and information about the source repositories is really useful for determining
the up-to-dateness.  We've been capturing this data for a while, it's
about time we started showing it somewhere.

This does make `status --verbose` notably more verbose, but eh, that
seems fine for now.

See also https://github.com/projectatomic/rpm-ostree/issues/774

Closes: #1345
Approved by: jlebon
2018-04-25 14:52:23 +00:00
Colin Walters
e34f17fc3e core: Don't rm-rf directories when doing overrides
It's very normal for base packages to own directories with
dependent packages installing files there.  Doing an rm-rf
for directories was just wrong.  Concretely this fixes
an `override replace ./systemd-*.rpm`.

librpm is also pretty conservative here (for good reason)
and just ignores `ENOTEMPTY`, so let's match that.

I opted to split things up so we remove not-directories
in a first pass, then remove all directories we can in the
second.  This should maximize our chances of removing what we can
in a scenario where e.g. two co-dependent packages install files
to a directory one of them owns.

Closes: https://github.com/projectatomic/rpm-ostree/issues/1340

Closes: #1341
Approved by: jlebon
2018-04-20 16:07:44 +00:00
Colin Walters
4c3d9cdbce tests: Split unit test-basic.sh into introspection + upgrade-rebase
Following up to: https://github.com/projectatomic/rpm-ostree/pull/1336

It makes sense to keep the library tests as unit tests (although
we should also support doing them installed).

The upgrade-rebase tests will move into vmcheck/ soon.

Closes: #1338
Approved by: jlebon
2018-04-17 13:08:24 +00:00
Colin Walters
1c8c755e81 tests: Split test-basic into misc-{1,2}
Our test suite originated when package layering was still being
developed, but now that that's mature, the logic where layering
tests are distinct makes less sense.

The `basic` test had grown to really be a collection of many
miscellaneous things.  Let's make that more explicit.  Further,
let's avoid having each test suite grow too large; when a single
test fails we don't have an easy way to rerun just that test,
so a crude way to have faster local iteration is to split into groups.

My plan is to reintroduce a `basic` test that covers the basics
of all functionality - update, deploy, layering, etc.  The advanced/corner
cases of layering like the `rm -rf /` test would still live in a
`test-layering.sh` or so.

Closes: #1336
Approved by: jlebon
2018-04-16 17:53:20 +00:00
Colin Walters
73fa64eb12 tests: Move ostree-update creation functions into libvm.sh
So they can be reused outside of just the autoupdate test.

Closes: #1336
Approved by: jlebon
2018-04-16 17:53:20 +00:00
Colin Walters
4f42b94da7 tests: Migrate some basic unit tests that start daemon to vmcheck
The unit tests run an rpm-ostree daemon as non-root, which worked
surprisingly well for quite a while.  But it started failing when
working on a patch which adds caching that writes to `/var`.

Since we have the vmcheck system now, let's switch over to that.
This PR moves the random "basic" tests we'd accumulated like
one to verify `StateRoot` is only in `status --verbose`, but not
the tests for the `rebase` command etc.

Closes: #1336
Approved by: jlebon
2018-04-16 17:53:20 +00:00
Jonathan Lebon
60a59cbbe4 vmcheck/override-replace: Fix status jq test
We should be checking that there are no requested base replacements
here, not removals. This was likely a copy-paste slip-up from copying
the override-remove tests. I still kept those checks for sanity, and
just tacked on the correct ones.

Closes: #1323
Approved by: cgwalters
2018-04-05 15:26:46 +00:00
Jonathan Lebon
bb07a29bc2 overrides: Allow resetting inactive overrides
This fixes a small regression from #852 which prevented inactive
overrides to be reset. Which is funny, because that's exactly the most
likely time when you would want to reset an override.

Basically, the `!is_layered --> no overrides` doesn't make sense for
inactive overrides. I suspect most people worked around this by just
using `reset --all`.

Closes: #1323
Approved by: cgwalters
2018-04-05 15:26:46 +00:00
Jonathan Lebon
3a1fd2c139 tests/libvm: Support SSH_CONFIG env var
When juggling multiple test VMs for different purposes, it's useful to
be able to easily e.g. `make vmcheck` a specific one by overriding the
ssh-config file to use, rather than editing the latter each time.

Closes: #1324
Approved by: cgwalters
2018-04-04 20:48:39 +00:00
Colin Walters
defa1dc38a core/scripts: Support /var/lib/rpm-state
I was trying a `--ex-unified-core` compose of FAW, and things fell over
on `urw-base35-fonts` which does a dance of setting a stamp file in
`%post` and checking it in `%posttrans`.

This whole pattern should be considered deprecated by file triggers. But let's
support it for now.

Note there's a lot of parameter passing as we need a single directory which is
held across multiple script invocations.

Closes: #1319
Approved by: jlebon
2018-03-28 18:37:17 +00:00
Colin Walters
779f764fcd app: Call Reload right after getting sysroot proxy
A final followup to our pile of "add reload" PRs.  This one is
necessary to fix the `status` race.

Closes: #1312
Approved by: jlebon
2018-03-26 13:05:40 +00:00
Jonathan Lebon
706506b2d1 app/dbus: Always call Reload() after transaction
Specifically in this case, this allows us to close a race condition
during `upgrade --check` where the `CachedUpdate` property might not
have been updated yet when we read it after finishing the transaction.

Closes: #1311
Approved by: cgwalters
2018-03-23 21:50:33 +00:00
Colin Walters
60aa9a4a33 daemon: Automatically reload sysroot before txn
In this PR: https://github.com/projectatomic/rpm-ostree/pull/1309
I was hitting race conditions running `ostree admin pin` then
`rpm-ostree cleanup` as it was possible that the daemon hadn't handled
the inotify on the sysroot and reloaded the deployment state before
the txn request came in.

Close this race by doing an implicit `reload` before starting a txn.
This is a pretty efficient operation because for the sysroot we're
just doing a `stat()` and comparing mtime.

Implementation wise, change the external API to drop the "did change"
boolean as nothing outside of the `sysroot.c` file used it.

A followup to this would be changing the `status` CLI to call a
(new) DBus API like `RequestReload` that at least did the sysroot
reload if the daemon was otherwise idle or so?  And it'd be available
to unprivileged users.

Closes: #1311
Approved by: cgwalters
2018-03-23 21:50:33 +00:00
Colin Walters
9ddf65aade daemon: Retain pinned deployments
Followup to: https://github.com/ostreedev/ostree/pull/1464

Ideally, we'd delegate more logic around these things to libostree, but we're
not there yet.

This means e.g. `rpm-ostree cleanup -r` for a pinned rollback will just silently
skip it.  It'd be nicer to emit an error probably, but that'd be quite a bit
more work.

Closes: https://github.com/projectatomic/rpm-ostree/issues/1293

Closes: #1309
Approved by: jlebon
2018-03-23 13:25:33 +00:00
Jonathan Lebon
4291500368 daemon: Also generate CachedUpdate in DownloadUpdateRpmDiff
Moving to caching the GVariant to disk rather than during RPMOSTreeOS
reloads broke the legacy `DownloadUpdateRpmDiff` path because it relied
on the implied recalculations that occur on reloads to update
`CachedUpdate`.

This patch series was initially going to be about just migrating all the
legacy APIs to make use of the new metadata, which would have fixed this
properly. But we first need some real coverage in that aread, which is
very poor right now. I'd like to investigate integrating the Cockpit
tests (at least the ostree-specific parts) into our CI to remedy this.

Anyways, for now at least, let's fix Cockpit.

Closes: #1300

Closes: #1303
Approved by: cgwalters
2018-03-17 20:17:32 +00:00
Jonathan Lebon
bc97c3aa53 app/db-list: Use pkglist metadata when possible
This patch teaches `db list` to also use the pkglist metadata when it
can, just like we did for `db diff`. To spell it out: this then allows
`db list` to work on commits for which we only have the commit object.

I went for the surgical incision here and didn't try to support
invocations which use fnmatch patterns for now. Definitely possible,
though it didn't feel like it was worth the effort given that the common
case is just a raw `db list` (I'd wager most people are probably
hard-wired to pipe to `grep` anyway for filtering).

Also fix the usage string, which had the arguments flipped.

Closes: #1299
Approved by: cgwalters
2018-03-15 15:49:44 +00:00
Jonathan Lebon
d6172ddee2 app/db-diff: Add --base switch
Support a `--base` switch to perform the diff on the base layer if
deployment checksum was locally composed. This is useful to filter out
from the diff changes due to layered packages.

Closes: #1299
Approved by: cgwalters
2018-03-15 15:49:44 +00:00
Jonathan Lebon
2eb424b9ce vmcheck: Add vm_shell_inline
Initially suggested in #1298. Wanted to try this out for an upcoming
patch.

Closes: #1304
Approved by: cgwalters
2018-03-14 21:49:16 +00:00
Jonathan Lebon
3e39fda4aa vmcheck: Make vm_ansible_inline more verbose
Because otherwise, there's no way to see the output of the script.

Also, turn off `gather_facts` since in the majority of cases, we don't
need it, so let's avoid the overhead. We can make this an opt-in flag
later on if needed.

Closes: #1304
Approved by: cgwalters
2018-03-14 21:49:16 +00:00
Jonathan Lebon
aaf2c30807 vmcheck: Obey VMCHECK_DEBUG for rpm-ostreed.conf
Closes: #1304
Approved by: cgwalters
2018-03-14 21:49:16 +00:00
Colin Walters
f6d08183b6 daemon: Don't error out if a remote isn't found
This came up in a few places recently; it happens for RHEL in some
cases, and in general we don't want to completely fail the daemon
start if someone messes up their remote config.

Closes: https://github.com/projectatomic/rpm-ostree/issues/1301

Closes: #1302
Approved by: jlebon
2018-03-14 18:50:13 +00:00
Colin Walters
63f545a6f4 tests: Replace some more libvm bits with Ansible
`vm_cmdfile()` was clearly calling out for this. I also replaced `vm_send()`
calls.

Closes: #1298
Approved by: jlebon
2018-03-14 14:13:07 +00:00
Colin Walters
bb86912de9 tests: Dip our toes into using Ansible
This adds a shell primitive to make it easy to execute a playbook
task list.

The big picture idea is to sync with https://github.com/ostreedev/ostree/pull/1462
and rewrite some of the libvm shell stuff as playbooks, allowing easier
code sharing with a-h-t and just in general being a better library for
talking ssh and executing commnads.

Closes: #1297
Approved by: jlebon
2018-03-09 19:04:32 +00:00
Colin Walters
9cbec27d4c core: Add rpmostree.rpmmd-repos metadata to client layers too
This is a logical followup to:

 - "core: Add rpmostree.repo metadata to imported packages" AKA a52cb7d78e
 - "compose: Add rpmostree.rpmmd-repo metadata to commits by default" AKA e7a42f70a9

Basically I'd like to display this in `status` at least, though how we render it
gets...interesting when there is layering in play. For now though let's just
capture the data the same way we do server side.

Theoretically we could reconstruct this data by walking all of the pkgrefs in
the tree but...ugh. It's just a lot simpler to have it in this form too just
like the server-side path.

Closes: #1296
Approved by: jlebon
2018-03-08 20:59:15 +00:00
Jonathan Lebon
cdd3227f70 app/db-diff: assume booted deployment if less args
Very often when I have a pending deployment, I just want to find out
what the diff is before rebooting. We do print it after whatever
transaction laid down the new deployment, though we may not reboot right
away. It seems like a common enough operation to warrant some shortcuts.

This makes `db diff` more useful by automatically subbing in the booted
deployment if less arguments are given:

    rpm-ostree db diff
      <diff booted and pending deployment>
    rpm-ostree db diff $csum
      <diff booted and $csum>
    rpm-ostree db diff $csum1 $csum2
      <as before, diff $csum1 and $csum2>

As before, we never try to even load the sysroot in the last invocation,
so it still works equally well on non-OSTree systems/unprivileged.

Closes: #1294
Approved by: cgwalters
2018-03-08 17:30:42 +00:00
Colin Walters
60d4470e05 status: Show deployment pinned 📌 state
Implemented in libostree in https://github.com/ostreedev/ostree/pull/1464
Let's display it - wrapping the command will come later.

I also just noticed `rpmostree_syscore_filter_deployments()` at least is
going to have to learn about pinning; will need to improve the test suite
around this too.

Closes: #1292
Approved by: jlebon
2018-03-08 13:43:50 +00:00
Jonathan Lebon
5173f579ca tests/vmcheck: add coverage for --check and --preview
We had no coverage for `--check` and `--preview`. Now that they use the
more efficient auto-updates API, it should be fine to document/recommend
to users. Let's add some coverage in the existing auto-updates tests to
make sure we don't regress on them e.g. only working when auto-update
"check" mode is on.

Closes: #1268
Approved by: cgwalters
2018-03-05 22:42:26 +00:00
Jonathan Lebon
05e09cd72c auto-updates: Cache cached-update GVariant to disk
Rather than recalculating `cached-update` as part of transaction
cleanups and RpmostreedOS internal reloads, write it directly to a file
from `deploy_transaction_execute`. This gives two major benefits:

1. Auto-updates now has virtually zero impact to daemon startup time.
2. We get to directly use the `DnfSack` created during metadata refresh
   rather than reconstructing it later on. This greatly simplifies code.

This makes use of new APIs in libdnf to skip filelists and load
updateinfo metadata right from the start.

Closes: #1268
Approved by: cgwalters
2018-03-05 22:42:26 +00:00
Jonathan Lebon
b54b8744b9 tests/libvm: Handle transient service already existing
We would error out when trying to start the transient httpd service if
it already exists, e.g. from a previous test.

Depending on how we exit, the `vm_stop_httpd` trap for the previous test
might not have been able to kick in. I think this happens when we exit
using `fatal`, which just does an `exit 1`. It's not strictly an error,
so doesn't trip the `ERR` handler.

Let's just go the extra mile and explicitly delete transient services if
they already exist.

Closes: #1284
Approved by: cgwalters
2018-03-01 23:29:17 +00:00
Colin Walters
36071d3c69 rojig-rename: (almost) Everything else
This renames the remaining C files, tests, etc.  There are only
a few hits for `jigdo` left; changing them would be a format break,
so let's wait to do that until we need to.

Closes: #1279
Approved by: jlebon
2018-03-01 22:35:46 +00:00
Colin Walters
b304a433a0 Second renaming pass for "rojig"
We're continuing an incremental renaming process; previously we changed
the most user-visible strings.  Now we're doing some internal variables,
and notably the cached refs and the origin files - the latter set is
things that end up on disk.

This leaves the biggest items; renaming APIs, files, and tests.

Closes: #1276
Approved by: jlebon
2018-02-27 18:45:10 +00:00
Jonathan Lebon
829a746821 lib/package: Handle arch pkg transitions
Let's try to match expectations a bit from the dnf/yum world and
describe e.g. a `noarch` package become archful as an upgrade rather
than a removal and an addition. This was originally implicitly supported
(before PR #1230) by the fact that we didn't compare arches at all (and
in fact, arches don't even show up in a `db diff` output for modified
packages).

We bring this back here, but only in the simple case that it's a single
package. We still don't try to do any fancy handling for packages of the
same name.

Closes: #1272

Closes: #1274
Approved by: cgwalters
2018-02-27 15:44:11 +00:00
Jonathan Lebon
0874152ba6 vmcheck: Drop selinux-policy hack
This shouldn't be necessary anymore.

Closes: #1274
Approved by: cgwalters
2018-02-27 15:44:11 +00:00
Colin Walters
3dd132285a Initial renaming pass of "jigdo" to "rojig"
I noticed that Ubuntu also uses the original "jigdo", so let's start
pulling off the band-aid here and do a mass rename.

For this first pass I'm focusing on CLI entrypoints and docs, as that's what
people are going to see; renaming all of the internal C functions, structure
variables etc. can come later.

Closes: #1269
Approved by: jlebon
2018-02-26 15:32:50 +00:00
Colin Walters
d03cd8a7b7 tests/jigdo-client: Re-enable
FAHC is rebuilt now with v5.

Closes: #1263
Approved by: jlebon
2018-02-20 14:27:54 +00:00
Colin Walters
2e25732ba1 status: Add -b/--booted to only print that deployment
It's usually what I want to paste into bug reports for example; I rarely want to
show the pending state or rollback.

Closes: #1260
Approved by: jlebon
2018-02-17 19:00:24 +00:00
Jonathan Lebon
ff9c19acd3 app/override: allow removing and replacing atomically
This is an essential functionality rather than a nicety. Some
replacements can *only* be done without conflicts if we can remove
packages at the same time.

I do like that this has to be done explicitly, though OTOH, I can
definitely see folks wanting an `--allow-removals` type of switch in the
future.

Closes: #1255

Closes: #1257
Approved by: cgwalters
2018-02-16 19:15:18 +00:00
Colin Walters
5879b96a64 jigdo V5: Use number of objects as cache invalidation trigger
Changes in a server-side tree can cause the need for clients to import different
objects from packages. For example, turning on documentation. Another more
subtle case is where an object might "move" from package A to B by being deleted
from A - then the jigdo build process will pick the B version.

We need a "cache validation key"; a way for the server to tell the client that
the objects it should import from the package have changed. Initially I was
thinking of using the libostree "content hash" but that would be awkward as we'd
have to do an import on the server side too.

After more consideration I realized a simple *count* of the number of objects
actually works, because (as I note in a comment) changing a file in the tree
will result in it ending up in the jigdoRPM (and count as a deletion).  And
obviously adding or removing objects changes the count too.

In fact we could have done this *without* breaking the format by just having the
client start recording the number of xattr entries, but this adds greater
flexibility down the line since we can in theory change how we do cache
invalidation if we *really* need to (but at the cost of triggering clients to
redownload packages).

Note the client logic got moved around as now we need to parse all the xattrs
before we decide what packages to download.

My test case here is turning on docs - I noticed this actually affects *every*
package which was surprising to me; I expected at least some packages wouldn't
have docs. I'll double check this.

It'd be good to have a "moving object" case too which I may look at.

Closes: https://github.com/projectatomic/rpm-ostree/issues/1197

Closes: #1256
Approved by: jlebon
2018-02-16 18:45:51 +00:00
Colin Walters
a4d8980d59 tests/jigdo-e2e: Strictly require npkgs to import
Since we changed things to have `jigdoSet = pkgSet`, we can just require exactly
`${npkgs}` here on import, which is what we found from `db list`.

Closes: #1256
Approved by: jlebon
2018-02-16 18:45:51 +00:00
Colin Walters
a0b6d241d2 status: Render jigdo mode using package NEVRA
What's happened up till now is supporting `rojig://` in the same way as
`ostree://`.  However, part of the high level goal here is to reduce
the need for system administrators to understand ostree.

This patch set starts to introduce some of the ideas for client-side
changes as part of jigdo ♲📦:
https://github.com/projectatomic/rpm-ostree/issues/1081#issuecomment-348540604

Concretely, we start using `${repo}:${nevra}` instead of `rojig://`.

(v2): Keep `Version` (plus timestamp) as a split out field for maximum visual aid.

Also, let's be opinionated here and entirely drop the `Commit` checksum by
default. I believe the Cockpit guys were right here - versions are for humans.
The fact that we have a checksum is powerful; and we still show it with `status
-v`. The way I think of it is: the checksum shows we're really an image system.
But we don't need to show it by default.

Closes: #1240
Approved by: jlebon
2018-02-16 17:02:47 +00:00
Jonathan Lebon
0729487ae5 Check and display pending security advisories
Pick up security advisories when checking for pending updates and
include them in the `cached-update` property. On the client-side,
display them in the output of `status`.

This was part of the original vision for how useful a smart `check` mode
could be. It directly impacts how one manages their individual system
(e.g. when to reboot), and paves the way for integration into
higher-level apps that act at the cluster level.

Closes: #1249
Approved by: cgwalters
2018-02-15 15:30:26 +00:00
Jonathan Lebon
45a3b53558 daemon/api: fix legacy D-Bus API and add coverage
Fix logic to make sure we check if the refspec is of type `ostree://`
even when it's explicitly specified. Also fix `Deploy` in the case where
we didn't just `Download` the RPM diff by adding a new @checksum
parameter to the higher-level API.

Finally, add a basic test for the `GetCached*RpmDiff` APIs so we have at
least *some* coverage. This is also good prep for making sure we don't
break anything when we convert those APIs to use the more efficient
pkglist metadata. The tests completely ignore the `DownloadRpmDiff`
paths for now though.

Closes: #1250

Closes: #1253
Approved by: cgwalters
2018-02-15 15:03:05 +00:00
Colin Walters
799a809c2d Add support for deploy <version> in rojig:// mode
This fleshes out an important piece of the story, showing that
we can support history versioning the same way that we did with
ostree.

Also it's very useful for testing; I'm going to extend the suite after this to
deploy the previous version, clean everything up, then upgrade and verify we
only download changed RPMs.

Closes: #1232
Approved by: jlebon
2018-02-14 21:54:30 +00:00
Colin Walters
c5939b7ca7 tests: Add RPMOSTREE_TEST_NO_OVERLAY to skip overlay
The use case here is to run our tests against the shipped tree;
this could be used by a-h-t for example:
https://github.com/projectatomic/atomic-host-tests/issues/74

I tried this with just `TESTS=basic` and it failed for `usroverlay`. So we'd
have to start adding feature detection to the test suite to make this truly
useful, but let's at least start with the basic bits now to play with it.

Closes: #1251
Approved by: jlebon
2018-02-14 20:00:16 +00:00