Add support for reading and writing lockfiles #553

jlebon · 2019-06-11T19:38:22Z

First, add manifest-lock.json to the list of magical files in the
source config which we check for.

Then, teach fetch and build to respect the lockfile by default, but
add an --update-lockfile option to the former allow bumping the
lockfile.

Finally, always generate an output lockfile from the compose and put in
the build dir as manifest-lock.generated.json.

This is the first step towards actually being able to make use of
lockfiles in the FCOS pipeline.

cgwalters · 2019-06-12T13:33:03Z

src/cmd-build

@@ -153,9 +158,13 @@ prepare_git_artifacts "${configdir_gitrepo}" "${PWD}/coreos-assembler-config.tar

 # These need to be absolute paths right now for rpm-ostree
 composejson=${PWD}/tmp/compose.json
+# Put this under tmprepo so it gets automatically chown'ed if needed
+lockfile_out=${tmprepo}/tmp/lockfile.json


How about manifest-lock.json?

It would be nice to have the output lockfile name indicate it is the output of the compose.. Something like manifest-lockfile-out.json. In practice we can then use manifest-lockfile-in.json as the input to the compose that we store in our fedora-coreos-configs git repo

Not strongly against, though isn't where the lockfile located enough context already? E.g. if it's in the build dir, it's clearly an output from cosa, and if it's in the source config, it's clearly meant to be an input, right? And the commands will respect that by default (unless you explicitly pass e.g. --update-lockfile as suggested below).

directory context does give you a lot, but if you copy things to different locations, discuss them in bug reports or on IRC, etc, you will lose that context. What if I want to copy two lockfiles to the same directory and then vimdiff them? All of these things can be done with them being the same filename and their directory setting the context, but I think It's way easier if we have explicit naming.

Why would you copy them to the same directory?

It sounds like you just want them named differently, but not necessarily explicitly labeled "input" and "output", which I don't particularly like since the format of the file is the exact same in either case. And e.g. a lockfile output can itself be used as an input.

How about manifest-lock.json for the one in the source config, and ostree-lock.json for the output one? That way the former is close to manifest.yaml, and the latter is close to the other ostree- objects.

It sounds like you just want them named differently

yes

but not necessarily explicitly labeled "input" and "output"

i'd prefer it over arbitrary names. Of course the format is similar and one could use an output lockfile as input, but would be nice if the filename indicated where it came from or what it was used for. Looks like i'm not being very convincing. One final attempt 😄 lockfile.out.json and lockfile.in.json

another option.. the lockfile in the configs repo can just be lockfile.json but the generated output lockfile is lockfile-out.json or lockfile-generated.json or something that indicates it was an output artifact.

I went with manifest-lock.json for input to follow the convention of using the same name as the manifest itself (could also match cargo/glide and do manifest.lock though the .json is helpful for e.g. web viewing), and manifest-lock.generated.json for the output so it's obvious that it's the same file format as the input one, but that it was generated from scratch.

thanks - that works for me!

cgwalters · 2019-06-12T13:36:22Z

Hmm. Wouldn't we want a semantic more like where we auto-find the lockfile in the config git repo?
And something like cosa fetch would update it?

jlebon · 2019-06-12T14:42:43Z

Wouldn't we want a semantic more like where we auto-find the lockfile in the config git repo?

Yeah, make sense to me. I mean, ideally the lockfile is stored in the same git repo as the config (and that'll be the case for FCOS) so it makes sense to codify that.

And something like cosa fetch would update it?

I think we need both ways. E.g. in FCOS, the mechanical pipelines will always be fetching the latest, so we don't need a lockfile input to cosa build. OTOH, devel and prod pipelines will always be building with an input lockfile, so the cosa fetch pipeline just needs to fetch what's in the lockfile.

Higher-level though, it doesn't seem like just bumping the lockfile (without doing a build & basic test) would be a common workflow, right? Let's make it a switch instead? So e.g.

cosa fetch without lockfile in repo --> status quo; fetch the latest from yum repos
cosa fetch with lockfile in repo --> fetch locked pkgs from yum repos
cosa fetch --update-lockfile --> fetch the latest from yum repos and create/update lockfile

dustymabe · 2019-06-12T15:12:21Z

cosa fetch without lockfile in repo --> status quo; fetch the latest from yum repos

cosa fetch with lockfile in repo --> fetch locked pkgs from yum repos

cosa fetch --update-lockfile --> fetch the latest from yum repos and create/update lockfile

seems reasonable to me, though I will note that we probably will need some other instrumentation around the lockfile. For example, I just hit an issue because the fedora-coreos-overlay rpm from one run (i.e. output lockfile) was in the lockfile I then used as input to another run and the run barfed at me because it couldn't find an rpm with the same sha256sum.

I have a few other questions:

what happens when an rpm (dep) is missing in a lockfile, but needed for the build? i'm thinking the build should error, but not sure.
when we input a lockfile - is the manifest used at all? i.e. whats the real source of truth?
could we add a semantic to the lockfile that says an rpm should be part of the build but we don't restrict the version
could we outfit the lockfile to handle multi-arch so we don't need n = number of arches branches for each stream?

dustymabe · 2019-06-12T15:25:35Z

could we add a semantic to the lockfile that says an rpm should be part of the build but we don't restrict the version

along that same vein, if we have something like this then it would be cool to have an output artifact that represents the difference between the input and output lockfiles

cgwalters · 2019-06-12T17:44:22Z

For example, I just hit an issue because the fedora-coreos-overlay rpm from one run (i.e. output lockfile) was in the lockfile I then used as input to another run and the run barfed at me because it couldn't find an rpm with the same sha256sum.

Right, I think we need to maintain an rpm-md repo that has multiple versions of the base fedora content too.

dustymabe · 2019-06-12T18:50:35Z

Right, I think we need to maintain an rpm-md repo that has multiple versions of the base fedora content too.

do we have that today with coreos-pool? In the latest we have multiple versions of the kernel:

So now that we have OSTree layer capabilities in rpm-ostree (coreos/rpm-ostree#1830), I think it makes more sense overall to make the overlay an OSTree layer. The fact that it *is* an RPM, but differs from all the other RPMs in various ways will (and already has) come up multiple times when we talk about signing, changelogs, lockfiles, overrides, etc... (See [1] and [2] for some already filed issues). Switching to an OSTree layer sidesteps many of those issues. Of course, we do lose some things from moving away from RPM: 1. it's harder to determine from where the overlay files come from on a host system (see [3] related to this) 2. invisible to `rpm-ostree db diff` (but then again, the diff we currently get isn't helpful) 3. not possible to issue an advisory for it to be consumed like the rest of the advisories (though with the current approach, making use of this without going through Bodhi would've required some non-trivial amount of work too) Overall though, I think those are things we could (and should strive to) fix in rpm-ostree rather than avoiding layers entirely. [1] coreos#544 [2] coreos#553 (comment) [3] coreos/rpm-ostree#1789

jlebon · 2019-06-12T21:26:10Z

what happens when an rpm (dep) is missing in a lockfile, but needed for the build? i'm thinking the build should error, but not sure.

Yup, that's the correct behaviour. That's the "lock" part in the name. :)

when we input a lockfile - is the manifest used at all? i.e. whats the real source of truth?

Right, we should probably write up some docs around the exact semantics. The TL;DR is: the lockfile merely constrains the pool of NEVRAs from which to pick, it doesn't dictate what goes into the compose itself.

could we outfit the lockfile to handle multi-arch so we don't need n = number of arches branches for each stream?

Could just be separate lockfiles in the same branch though, right? This is a tricky topic, but yeah, we definitely need to figure out the lockfile + multi-arch + overrides strategy, though not necessarily here. We can rework the format to accommodate whatever we decide.

Right, I think we need to maintain an rpm-md repo that has multiple versions of the base fedora content too.

do we have that today with coreos-pool?

Right, exactly. Then all the prod & devel streams will source from there, using the lockfile to narrow down versions.

For example, I just hit an issue because the fedora-coreos-overlay rpm from one run (i.e. output lockfile) was in the lockfile I then used as input to another run and the run barfed at me because it couldn't find an rpm with the same sha256sum.

So this comment prompted me to think more deeply about what we're doing with fedora-coreos-overlay right now and overall I think we should either (a) make it an OSTree layer, or (b) make it a proper RPM in Fedora. I think (a) is cleaner and less work, so I opened #555.

dustymabe · 2019-06-13T13:17:02Z

what happens when an rpm (dep) is missing in a lockfile, but needed for the build? i'm thinking the build should error, but not sure.

Yup, that's the correct behaviour. That's the "lock" part in the name. :)

hmm. that's not exactly what I am seeing. In #553 (comment) I mention hitting an issue with thefedora-coreos-overlay rpm. I took the output lockfile that included it and removed it from the input lockfile for the next compose. Then the next compose succeeded. From your statement above it would make me think that because it was included in the manifest, but was missing from the lockfile I should have seen an error.

So this comment prompted me to think more deeply about what we're doing with fedora-coreos-overlay right now and overall I think we should either (a) make it an OSTree layer, or (b) make it a proper RPM in Fedora. I think (a) is cleaner and less work, so I opened #555.

cool, will discuss over there.

jlebon · 2019-06-13T14:35:05Z

Yup, that's the correct behaviour. That's the "lock" part in the name. :)

hmm. that's not exactly what I am seeing

Ahh apologies, totally misread your original question. So, this is mostly answered in the second bit of #553 (comment), but to be more explicit: right now, a package name that isn't in the lockfile can still get pulled into the compose. This means one can e.g. add a package to the manifest while keeping everything else frozen, or manually bump a locked package that brings in a new dependency.

You're right though that there could be a "strict" mode in which the lockfile is the canonical source of packages (e.g. something like --lockfile-only?). In practice, feeding the same 3-tuple input of (manifest, lockfile, rpm-md) to rpm-ostree compose tree should result in the same output, but e.g. changing the available packages in the repo could in theory result in libsolv picking a different final solution. (E.g. imagine a repo with a new package which obsoletes or provides one of our requests... we could do the modification I proposed here of only supporting pure pkgnames, but it still wouldn't cover dependencies).

The downside of this approach is that it makes manipulating lockfiles (e.g. bumping/adding/removing packages) manually trickier since we'd have to always provide the full dependency tree... which we'd probably use rpm-ostree in "non-strict mode" to do as well, so it doesn't really get us out of sanity-checking the diff anyway. So overall, I'm leaning more towards rolling with the current approach for now and see how it pans out in practice?

jlebon · 2019-06-13T16:28:59Z

OK, updated! ⬆️

You'll want to test this with #555 though.

src/cmd-build

src/cmd-fetch

First, add `manifest-lock.json` to the list of magical files in the source config which we check for. Then, teach `fetch` and `build` to respect the lockfile by default, but add an `--update-lockfile` option to the former allow bumping the lockfile. Finally, always generate an output lockfile from the compose and put in the build dir as `manifest-lock.generated.json`. This is the first step towards actually being able to make use of lockfiles in the FCOS pipeline.

So now that we have OSTree layer capabilities in rpm-ostree (coreos/rpm-ostree#1830), I think it makes more sense overall to make the overlay an OSTree layer. The fact that it *is* an RPM, but differs from all the other RPMs in various ways will (and already has) come up multiple times when we talk about signing, changelogs, lockfiles, overrides, etc... (See [1] and [2] for some already filed issues). Switching to an OSTree layer sidesteps many of those issues. Of course, we do lose some things from moving away from RPM: 1. it's harder to determine from where the overlay files come from on a host system (see [3] related to this) 2. invisible to `rpm-ostree db diff` (but then again, the diff we currently get isn't helpful) 3. not possible to issue an advisory for it to be consumed like the rest of the advisories (though with the current approach, making use of this without going through Bodhi would've required some non-trivial amount of work too) Overall though, I think those are things we could (and should strive to) fix in rpm-ostree rather than avoiding layers entirely. [1] coreos#544 [2] coreos#553 (comment) [3] coreos/rpm-ostree#1789

dustymabe

LGTM. We'll discuss strict mode more in a separate issue

See discussions in coreos/coreos-assembler#553, especially coreos/coreos-assembler#553 (comment). Essentially, there is a use case for making the lockfile *the* canonical source of packages to install. And in fact, it seems like that should be the default mode in which `--ex-lockfile` operates. This patch then adds a `--ex-lockfile-relaxed` mode which allows packages to be added or removed (current behaviour). Just whipped this up while I still had the context in my head. Haven't thouroughly thought it through or tested it out yet.

jlebon · 2019-06-14T18:50:14Z

Strict mode: coreos/rpm-ostree#1858

So now that we have OSTree layer capabilities in rpm-ostree (coreos/rpm-ostree#1830), I think it makes more sense overall to make the overlay an OSTree layer. The fact that it *is* an RPM, but differs from all the other RPMs in various ways will (and already has) come up multiple times when we talk about signing, changelogs, lockfiles, overrides, etc... (See [1] and [2] for some already filed issues). Switching to an OSTree layer sidesteps many of those issues. Of course, we do lose some things from moving away from RPM: 1. it's harder to determine from where the overlay files come from on a host system (see [3] related to this) 2. invisible to `rpm-ostree db diff` (but then again, the diff we currently get isn't helpful) 3. not possible to issue an advisory for it to be consumed like the rest of the advisories (though with the current approach, making use of this without going through Bodhi would've required some non-trivial amount of work too) Overall though, I think those are things we could (and should strive to) fix in rpm-ostree rather than avoiding layers entirely. [1] #544 [2] #553 (comment) [3] coreos/rpm-ostree#1789

jlebon mentioned this pull request Jun 11, 2019

Work towards creating streams coreos/fedora-coreos-config#100

Closed

8 tasks

dustymabe self-requested a review June 11, 2019 20:58

cgwalters reviewed Jun 12, 2019

View reviewed changes

jlebon added the WIP PR still being worked on label Jun 12, 2019

jlebon mentioned this pull request Jun 12, 2019

cmdlib.sh: Switch to using OSTree layers for overlay/ #555

Merged

jlebon force-pushed the pr/lockfile branch from 7958a03 to 9f15a9a Compare June 13, 2019 16:27

jlebon changed the title ~~cmd-build: Add lockfile support~~ Add support for reading and writing lockfiles Jun 13, 2019

jlebon removed the WIP PR still being worked on label Jun 13, 2019

jlebon force-pushed the pr/lockfile branch from 9f15a9a to 5612077 Compare June 13, 2019 16:31

dustymabe reviewed Jun 14, 2019

View reviewed changes

src/cmd-build Outdated Show resolved Hide resolved

dustymabe reviewed Jun 14, 2019

View reviewed changes

src/cmd-fetch Outdated Show resolved Hide resolved

jlebon force-pushed the pr/lockfile branch from 5612077 to 2dedd46 Compare June 14, 2019 14:06

dustymabe approved these changes Jun 14, 2019

View reviewed changes

jlebon mentioned this pull request Jun 14, 2019

compose: Add --ex-lockfile-strict coreos/rpm-ostree#1858

Merged

jlebon merged commit cbe1c98 into coreos:master Jun 14, 2019

jlebon mentioned this pull request Nov 26, 2019

[testing] Add coreos-keep-cgroup-v1.service coreos/fedora-coreos-config#238

Merged

jlebon mentioned this pull request Apr 8, 2020

Use strict lockfile semantics for FCOS coreos/fedora-coreos-tracker#454

Closed

jlebon deleted the pr/lockfile branch July 6, 2020 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for reading and writing lockfiles #553

Add support for reading and writing lockfiles #553

jlebon commented Jun 11, 2019 •

edited

Loading

cgwalters Jun 12, 2019

dustymabe Jun 12, 2019

jlebon Jun 12, 2019

dustymabe Jun 12, 2019

cgwalters Jun 12, 2019

jlebon Jun 12, 2019

dustymabe Jun 12, 2019

dustymabe Jun 13, 2019

jlebon Jun 13, 2019

dustymabe Jun 13, 2019

cgwalters commented Jun 12, 2019

jlebon commented Jun 12, 2019

dustymabe commented Jun 12, 2019 •

edited

Loading

dustymabe commented Jun 12, 2019

cgwalters commented Jun 12, 2019

dustymabe commented Jun 12, 2019

jlebon commented Jun 12, 2019

dustymabe commented Jun 13, 2019

jlebon commented Jun 13, 2019

jlebon commented Jun 13, 2019

dustymabe left a comment

jlebon commented Jun 14, 2019

Add support for reading and writing lockfiles #553

Add support for reading and writing lockfiles #553

Conversation

jlebon commented Jun 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgwalters commented Jun 12, 2019

jlebon commented Jun 12, 2019

dustymabe commented Jun 12, 2019 • edited Loading

dustymabe commented Jun 12, 2019

cgwalters commented Jun 12, 2019

dustymabe commented Jun 12, 2019

jlebon commented Jun 12, 2019

dustymabe commented Jun 13, 2019

jlebon commented Jun 13, 2019

jlebon commented Jun 13, 2019

dustymabe left a comment

Choose a reason for hiding this comment

jlebon commented Jun 14, 2019

jlebon commented Jun 11, 2019 •

edited

Loading

dustymabe commented Jun 12, 2019 •

edited

Loading