Add new `ex history` command #1813

jlebon · 2019-04-16T21:42:25Z

This is the rpm-ostree equivalent of dnf history. As opposed to the
history of the refspec (i.e. ostree log), this shows the history of
the system, i.e. the refspecs the host deployed, checksums, versions,
layered packages, etc... The amount of details remembered is similar to
what shows up in status.

There's definitely some further enhancements possible (e.g. printing
package diffs, displaying rollbacks), though this seems in good enough
shape as a first cut.

Closes: #1489

jlebon · 2019-04-16T21:50:49Z

Requires ostreedev/ostree#1842

rh-atomic-bot · 2019-04-26T12:57:10Z

☔ The latest upstream changes (presumably 97bb57d) made this pull request unmergeable. Please resolve the merge conflicts.

cgwalters

Just two quick comments. Not sure I understand the high level theory of operation, how the history dir and the journal interact, etc. A brief description of that type of stuff (e.g. as a rustdoc header in the history.rs file) could help a lot.

src/app/rpmostree-ex-builtin-history.c

rust/src/history.rs

jlebon · 2019-04-30T14:19:55Z

Fixup! ⬆️

I added docstrings on the functions that missed them and a header describing the overall approach. In the process, I ended up renaming a bunch of things to clearer, more appropriate, names.

jlebon · 2019-05-03T19:36:19Z

Rebased! (Just marking as "Ready for review" to pass it through CI).

jlebon · 2019-05-03T19:37:03Z

bot, retest this please

lucab · 2019-05-07T13:50:01Z

One overall doubt that the docstring didn't answer: is there some garbage-collection mechanism to be considered/planned/implemented here for history files?

jlebon · 2019-05-07T13:54:27Z

is there some garbage-collection mechanism to be considered/planned/implemented here for history files?

Yes, this is implemented by history_prune in history.rs. We bind pruning to journal log pruning.

miabbott · 2019-05-07T17:55:10Z

/me bikesheds

In a recent demo of this work, @jlebon pointed out that DeployCommand was indicating the command that was responsible for creating the deployment. So if a user used rpm-ostree rollback to go back to the previous deployment, the DeployCommand wouldn't be rollback, but rather install ltrace or override remove podman.

@dustymabe suggested that we go a step further and track this difference, so rollback would be recorded in the history somehow.

I think this would be excellent, as long as we could display this info in a sane way. Maybe use DeployCommand in the case of a rollback, but also have something like DeployCreateCommand to show the original command used?

The more I think on this, it seems like it's really only the rollback command where confusion would arise. Other operations (deploy, install, rebase, upgrade, etc) are really about creating a brand new deployment, where as rollback is about using a previously created deployment and you would want to know how that deployment was created.

cgwalters

Overall, this is good code. The doc comment helps significantly, though I still find myself a bit lost while trying to grasp some of the details.

I like the journal unit tests.

We're a bit lacking real e2e tests - we could at least sprinkle a few ex history assertions inside the existing tests probably?

Overall particularly given the fact that this is ex I'm in disposition to merge, though I do have one question around the architecture at the end - it seems dramatically simpler if we can serialize everything we care about in the journal.

rust/src/history.rs

cgwalters · 2019-05-21T01:52:09Z

src/daemon/rpmostree-sysroot-upgrader.c

+   * it breaks the expectation that journal messages should be somewhat easily
+   * introspectable. We could also serialize it to JSON first, though we wouldn't be able to
+   * re-use the printing code in `status.c` as is. Note also the GVariant can be large (e.g.
+   * we include the full `rpmostree.rpmdb.pkglist` in there). */


What elements of this deployment variant do we use? The other option is to log those as structured fields right?

What elements of this deployment variant do we use?

Basically almost everything. :) We pass it to the same function in status.c that normally prints it. I also want to make use of the pkglist in the future so that we can show the diff from one version to the next.

The other option is to log those as structured fields right?

Yeah, I had briefly looked at this. One obvious annoyance is that the field naming convention means we have to do some translation back and forth for the top-level keys. And the values themselves I guess would have to be run through g_variant_print and g_variant_parse. Still, I go back to whether we should shove that much data in the journal in the first place. (Even though the actual limit is a crazy amount like 768M...). It somewhat reminds me of the D-Bus/--json discussion around pkglists and how we ended up blacklisting it, even though it's mostly for machines.

jlebon · 2019-05-24T20:29:19Z

We're a bit lacking real e2e tests

Yeah, I'm pretty confident in the unit tests, though I definitely intend to add some basic vmcheck tests as well!

jlebon · 2019-05-24T20:57:38Z

I just realized now that I squashed changes into the commit and rebased in one shot, which makes it hard to see what changed. Anyway, it was mostly a rebase + supporting --json + using get_file_type(). Still need to add tests and address some of the feedback from #1813 (comment) (though yeah... might leave that as a follow-up).

jlebon · 2019-06-11T15:06:07Z

Now with tests! Dropped WIP and updated the original comment as well with the final commit message.

I haven't addressed the feedback in #1813 (comment) other than renaming the key names to CreateCommand and CreateTimestamp. Will open a follow-up ticket for the remaining items.

This is the rpm-ostree equivalent of `dnf history`. As opposed to the history of the refspec (i.e. `ostree log`), this shows the history of the system, i.e. the refspecs the host deployed, checksums, versions, layered packages, etc... The amount of details remembered is similar to what shows up in `status`. There's definitely some further enhancements possible (e.g. printing package diffs, displaying rollbacks), though this seems in good enough shape as a first cut. Closes: coreos#1489

jlebon · 2019-09-23T13:00:10Z

✔️ Tests all green now!
Let's ship this?

cgwalters · 2019-09-23T18:00:15Z

I think this is safe enough to try out and use to gather feedback. Thanks for the work on this!

@rh-atomic-bot r+ f49100a

rh-atomic-bot · 2019-09-23T18:00:23Z

⌛ Testing commit f49100a with merge d3f08ae...

This is the rpm-ostree equivalent of `dnf history`. As opposed to the history of the refspec (i.e. `ostree log`), this shows the history of the system, i.e. the refspecs the host deployed, checksums, versions, layered packages, etc... The amount of details remembered is similar to what shows up in `status`. There's definitely some further enhancements possible (e.g. printing package diffs, displaying rollbacks), though this seems in good enough shape as a first cut. Closes: #1489 Closes: #1813 Approved by: cgwalters

rh-atomic-bot · 2019-09-23T19:00:16Z

💔 Test failed - status-papr

jlebon · 2019-09-23T19:02:03Z

@rh-atomic-bot retry

rh-atomic-bot · 2019-09-23T19:02:10Z

⌛ Testing commit f49100a with merge b740eb4...

This is the rpm-ostree equivalent of `dnf history`. As opposed to the history of the refspec (i.e. `ostree log`), this shows the history of the system, i.e. the refspecs the host deployed, checksums, versions, layered packages, etc... The amount of details remembered is similar to what shows up in `status`. There's definitely some further enhancements possible (e.g. printing package diffs, displaying rollbacks), though this seems in good enough shape as a first cut. Closes: #1489 Closes: #1813 Approved by: cgwalters

rh-atomic-bot · 2019-09-23T20:03:08Z

💔 Test failed - status-papr

jlebon · 2019-09-23T20:35:16Z

@rh-atomic-bot retry

rh-atomic-bot · 2019-09-23T20:55:26Z

⌛ Testing commit f49100a with merge 33cf82f...

This is the rpm-ostree equivalent of `dnf history`. As opposed to the history of the refspec (i.e. `ostree log`), this shows the history of the system, i.e. the refspecs the host deployed, checksums, versions, layered packages, etc... The amount of details remembered is similar to what shows up in `status`. There's definitely some further enhancements possible (e.g. printing package diffs, displaying rollbacks), though this seems in good enough shape as a first cut. Closes: #1489 Closes: #1813 Approved by: cgwalters

rh-atomic-bot · 2019-09-23T21:54:08Z

💔 Test failed - status-papr

jlebon · 2019-09-24T13:18:21Z

@rh-atomic-bot retry

rh-atomic-bot · 2019-09-24T13:18:28Z

⌛ Testing commit f49100a with merge 1db9670...

This is the rpm-ostree equivalent of `dnf history`. As opposed to the history of the refspec (i.e. `ostree log`), this shows the history of the system, i.e. the refspecs the host deployed, checksums, versions, layered packages, etc... The amount of details remembered is similar to what shows up in `status`. There's definitely some further enhancements possible (e.g. printing package diffs, displaying rollbacks), though this seems in good enough shape as a first cut. Closes: #1489 Closes: #1813 Approved by: cgwalters

rh-atomic-bot · 2019-09-24T14:14:52Z

💔 Test failed - status-papr

jlebon · 2019-09-24T14:17:09Z

@rh-atomic-bot retry

rh-atomic-bot · 2019-09-24T14:17:16Z

⌛ Testing commit f49100a with merge 1075559...

rh-atomic-bot · 2019-09-24T15:08:49Z

☀️ Test successful - status-papr
Approved by: cgwalters
Pushing 1075559 to master...

jlebon added the WIP label Apr 16, 2019

jlebon mentioned this pull request Apr 17, 2019

Consider an rpm-ostree history command #1489

Closed

jlebon force-pushed the pr/history branch 3 times, most recently from e8a03f3 to 75aee0e Compare April 25, 2019 02:25

jlebon mentioned this pull request Apr 25, 2019

Prep patches #1816

Closed

cgwalters reviewed Apr 28, 2019

View reviewed changes

src/app/rpmostree-ex-builtin-history.c Outdated Show resolved Hide resolved

rust/src/history.rs Show resolved Hide resolved

rust/src/history.rs Outdated Show resolved Hide resolved

jlebon force-pushed the pr/history branch from fa611ac to f7a05d8 Compare April 30, 2019 14:16

This was referenced May 1, 2019

Misc patches #1823

Closed

Pass full command-line and use as transaction title #1824

Closed

jlebon force-pushed the pr/history branch from f7a05d8 to dd64ffa Compare May 3, 2019 19:35

jlebon marked this pull request as ready for review May 3, 2019 19:35

cgwalters reviewed May 21, 2019

View reviewed changes

jlebon force-pushed the pr/history branch from dd64ffa to 852f85b Compare May 24, 2019 20:24

jlebon force-pushed the pr/history branch from 852f85b to cf3b9ea Compare June 11, 2019 14:47

jlebon changed the title ~~WIP: Add new ex history command~~ Add new ex history command Jun 11, 2019

jlebon removed the WIP label Jun 11, 2019

lucab mentioned this pull request Aug 8, 2019

Rolling back doesn't work well with automatic updates coreos/fedora-coreos-tracker#247

Closed

lucab mentioned this pull request Aug 26, 2019

rpm-ostree: do not try to auto-update to local deployments coreos/zincati#111

Closed

jlebon force-pushed the pr/history branch 2 times, most recently from 86f7f32 to 443fe25 Compare September 20, 2019 21:10

jlebon force-pushed the pr/history branch from 443fe25 to f49100a Compare September 21, 2019 01:13

rh-atomic-bot closed this in 1075559 Sep 24, 2019

jlebon mentioned this pull request Apr 20, 2020

Add logic to support sending structured entries to the journald log lucab/libsystemd-rs#44

Merged

This was referenced Nov 26, 2020

Operation history support #85

Closed

Keep metadata from previous trees when purging #195

Closed

jlebon deleted the pr/history branch April 23, 2023 23:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new `ex history` command #1813

Add new `ex history` command #1813

jlebon commented Apr 16, 2019 •

edited

Loading

jlebon commented Apr 16, 2019

rh-atomic-bot commented Apr 26, 2019

cgwalters left a comment

jlebon commented Apr 30, 2019

jlebon commented May 3, 2019

jlebon commented May 3, 2019

lucab commented May 7, 2019

jlebon commented May 7, 2019

miabbott commented May 7, 2019

cgwalters left a comment

cgwalters May 21, 2019

jlebon May 24, 2019

jlebon commented May 24, 2019

jlebon commented May 24, 2019

jlebon commented Jun 11, 2019

jlebon commented Sep 23, 2019

cgwalters commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

jlebon commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

jlebon commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

jlebon commented Sep 24, 2019

rh-atomic-bot commented Sep 24, 2019

rh-atomic-bot commented Sep 24, 2019

jlebon commented Sep 24, 2019

rh-atomic-bot commented Sep 24, 2019

rh-atomic-bot commented Sep 24, 2019

Add new ex history command #1813

Add new ex history command #1813

Conversation

jlebon commented Apr 16, 2019 • edited Loading

jlebon commented Apr 16, 2019

rh-atomic-bot commented Apr 26, 2019

cgwalters left a comment

Choose a reason for hiding this comment

jlebon commented Apr 30, 2019

jlebon commented May 3, 2019

jlebon commented May 3, 2019

lucab commented May 7, 2019

jlebon commented May 7, 2019

miabbott commented May 7, 2019

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters May 21, 2019

Choose a reason for hiding this comment

jlebon May 24, 2019

Choose a reason for hiding this comment

jlebon commented May 24, 2019

jlebon commented May 24, 2019

jlebon commented Jun 11, 2019

jlebon commented Sep 23, 2019

cgwalters commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

jlebon commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

jlebon commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

rh-atomic-bot commented Sep 23, 2019

jlebon commented Sep 24, 2019

rh-atomic-bot commented Sep 24, 2019

rh-atomic-bot commented Sep 24, 2019

jlebon commented Sep 24, 2019

rh-atomic-bot commented Sep 24, 2019

rh-atomic-bot commented Sep 24, 2019

Add new `ex history` command #1813

Add new `ex history` command #1813

jlebon commented Apr 16, 2019 •

edited

Loading