#811 datestamp in FAW treefile causes no-op updates
Closed: Fixed Opened by walters.

rpm-ostree (unlike basically every other image build system we have) has a concept of whether or not its inputs changed; it stores a hash in rpmostree.inputhash in the resulting OSTree commit. Today that's defined by checksumming:

  • The input treefile JSON (and things like add-files which we don't use in fedora)
  • The depsolved package set (including the final rpm-md sha256).

Recently for Fedora Atomic Workstation we keep getting no-op updates. See these two runs for example:

  • https://kojipkgs.fedoraproject.org/compose/updates/Fedora-27-updates-20171202.1/logs/x86_64/Everything/ostree-4/create-ostree-repo.log
  • https://kojipkgs.fedoraproject.org/compose/updates/Fedora-27-updates-20171202.3/logs/x86_64/Everything/ostree-4/create-ostree-repo.log

Doing some sed on the pkgset (and we know we didn't change the treefile in git), there's no difference.

But it looks like the problem is that the generated treefile has the compose datestamp in it. So my proposal is that we drop that datestamp.


There's a bigger picture issue here in that AFAIK pungi itself doesn't have a concept of "nothing changed", because for every other image type, from lorax to cloud image builds, the build system doesn't have the intelligence to express it.

Also re the repo datestamp, see this commit - after the next rpm-ostree release we'll be emitting that metadata into the ostree commit, so there's no information lost about the input timestamps.

The repo files that pungi generates and injects into the treefile contain timestamp to make them somewhat unique. That seems to be the problem here.

There are two kinds of repos: external ones (outside of compose) and pointing to the current compose. The external ones can drop the timestamp without any issues (IMHO). For the compose repos if we drop the timestamp (and compose ID) from the name, there will be similarly named repo every time but pointing to a different location. Is that okay?

The per-compose .repo files end up in temporary work directories right? Like https://kojipkgs.fedoraproject.org/compose/updates/Fedora-27-updates-20171202.3/work/ostree-4/config_repo/ ?

I can't think of any problem on the rpm-ostree side by having non-datestamped inputs that point to varying content; it's after all how the traditional client side works! With fedora and updates.

BTW see also https://github.com/projectatomic/rpm-ostree/issues/774 - we really want versioning data inside the rpm-md.

Ack, they are always in a clone of the configuration repo, so the only possibility of conflict on file name level is with something there. This is not really a concern.

Oh it looks like this code is specific to the rpm-ostree phase? So something like this?

diff --git a/pungi/ostree/utils.py b/pungi/ostree/utils.py
index c832df3..d46d1cf 100644
--- a/pungi/ostree/utils.py
+++ b/pungi/ostree/utils.py
@@ -84,21 +84,17 @@ def tweak_treeconf(treeconf, source_repos=None, keep_original_sources=False):
     Update tree config file by adding new repos, and remove existing repos
     from the tree config file if 'keep_original_sources' is not enabled.
     """
-    # add this timestamp to repo name to get unique repo filename and repo name
-    # should be safe enough
-    time = datetime.datetime.now().strftime("%Y%m%d%H%M%S")
-
     treeconf_dir = os.path.dirname(treeconf)
     with open(treeconf, 'r') as f:
         treeconf_content = json.load(f)
     # backup the old tree config
-    os.rename(treeconf, '%s.%s.bak' % (treeconf, time))
+    os.rename(treeconf, '{}.bak'.format(treeconf))
     repos = []
     if source_repos:
         for repo in source_repos:
-            name = "%s-%s" % (repo['name'], time)
+            name = repo['name']
             _write_repofile("%s/%s.repo" % (treeconf_dir, name), name, repo)
             repos.append(name)

?

Yeah, I think that should be pretty much it. I think the name of the backup file can remain with the timestamp (although it does not really matter either).

There's a bigger picture issue here in that AFAIK pungi itself doesn't have a concept of "nothing changed", because for every other image type, from lorax to cloud image builds, the build system doesn't have the intelligence to express it.

AFAICT what you want is for two sequential composes that have the same content to not create the 2nd commit, but rather exit with a message that says "nothing changed". That's perfectly reasonable.

Specifically for Atomic Host the only problem I see, which you touched on above, is that media still gets created as part of the pungi compose. This normally wouldn't be a problem, but we are moving (hopefully very soon) to a model where we have version numbers like 27.20171204.0 in our ostrees and this will match the names of the media. This normally wouldn't be a problem unless we happen to have a compose A where ostree succeeded and qcow failed, and then we have compose B where ostree doesn't create a new commit because no packages changed, but then the media gets created with B in the name but version A ostree embedded in the media.

I have previously opened an RFE for use output of ostree step to determine if later steps should be done. Specifically take a look at the 3rd bullet point in the description.

PR in https://pagure.io/pungi/pull-request/815

@dustymabe - right, the interaction with media gets very interesting. But for "update" composes we aren't generating media right? At least for FAW there's no need to ship an updated ISO each day.

Anyways we need to fix this in order to have "use output of ostree step" to work at all.

PR in https://pagure.io/pungi/pull-request/815
@dustymabe - right, the interaction with media gets very interesting. But for "update" composes we aren't generating media right?

We aren't currently, but we're planning on creating media as part of the updates runs soon. This is good because we'll get ostree versions that match the media name AND we don't pay time delays from when an ostree compose was done and when we have media for it.

At least for FAW there's no need to ship an updated ISO each day.

Agree, this is only a concern for FAH. We don't create updated FAW ISOs.

Anyways we need to fix this in order to have "use output of ostree step" to work at all.

Can we make this configurable on the pungi side? i.e. add a argument for force new commit.

That already exists of course:
--force-nocache Always create a new OSTree commit, even if nothing appears to have changed

That already exists of course:
--force-nocache Always create a new OSTree commit, even if nothing appears to have changed

I know it exists in rpm-ostree. I'm asking if we can add hooks into pungi so we can use it. Your patch presumably will change the behavior back to what it should have been, but I've illustrated we may need to rely on that behavior for FAH until #677 is worked out.

I'm asking if we can add hooks into pungi so we can use it.

or maybe hooks already exist and I don't know about it, but a grep -R nocache in the pungi repo doesn't turn anything up.

but I've illustrated we may need to rely on that behavior for FAH until #677 is worked out.

Ugh...can you elaborate on this? I don't quite understand the link.

Also, PR for this in https://pagure.io/pungi/pull-request/815

This normally wouldn't be a problem unless we happen to have a compose A where ostree succeeded and qcow failed, and then we have compose B where ostree doesn't create a new commit because no packages changed, but then the media gets created with B in the name but version A ostree embedded in the media.

Oh I see. Yeah it may be that there are directly conflicting goals here; we can't have the ostree and media versioned the same unless they succeed/fail atomically, but we can't make use of rpm-ostree's change detection unless we support them differing.

That said though it should be straightforward to spin a patch to have pungi use --force-nocache only for the FAH composes say?

This doesn't impact FAH very much because we already have the two-week cadence for all updates, but it really is pretty awful for FAW.

Oh I see. Yeah it may be that there are directly conflicting goals here; we can't have the ostree and media versioned the same unless they succeed/fail atomically, but we can't make use of rpm-ostree's change detection unless we support them differing.

right. Another thing to think about, with multi-arch we could theoretically have a one tree (like aarch64) have an updated package (one that isn't in the other two arches) and it would get an update but the other two wouldn't.

That said though it should be straightforward to spin a patch to have pungi use --force-nocache only for the FAH composes say?

yep. that would be the ideal case. that way we can has FAH composes --force-nocache and FAW composes not. If we merge #815 I think we'd need to have this for FAH.

This doesn't impact FAH very much because we already have the two-week cadence for all updates, but it really is pretty awful for FAW.

yeah I agree it's not ideal for FAW, but I also doubt people update every day (if they did they would have to reboot every day, which I suspect is uncommon).

I also doubt people update every day (if they did they would have to reboot every day, which I suspect is uncommon).

A lot of the point though is that I can request an update every day - if nothing changed, then nothing should happen! And the update is queued, I'm not forced to reboot immediately (it's my computer after all 😉). But no-op updates undermine all that, at least if one's not looking closely and noticing there's no rpmdb diff.

Commit 51cd3590 relates to this ticket

Commit 3427d6ab fixes this issue

well. this isn't good. we need pungi to support --force-nocache before we can start using this change. See earlier comment: https://pagure.io/pungi/issue/811#comment-482800

@dustymabe: that option is already available: https://pagure.io/pungi/c/161b7f974b2dae625d2286d9f37ec7f01b7debd8?branch=master

@dustymabe: that option is already available: https://pagure.io/pungi/c/161b7f974b2dae625d2286d9f37ec7f01b7debd8?branch=master

Nice. I missed that. Thanks!

@dustymabe: Let's file a new ticket for the FAH issue that comes as a followup to this?

@dustymabe: Let's file a new ticket for the FAH issue that comes as a followup to this?

The FAH issue being?? that we need to force nocache?

Let's chat about this in https://pagure.io/pungi/issue/677 since it's quite related.

Metadata Update from @onosek:
- Issue tagged with: 4.1.22

This issue has been migrated to Fedora Forge:
https://forge.fedoraproject.org/pungi/pungi/issues/811

Please continue any further discussion there.

Metadata