Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible GetCachedRebaseRpmDiff regression #1250

Closed
jlebon opened this issue Feb 14, 2018 · 3 comments
Closed

Possible GetCachedRebaseRpmDiff regression #1250

jlebon opened this issue Feb 14, 2018 · 3 comments
Assignees
Labels
bug jira for syncing to jira

Comments

@jlebon
Copy link
Member

jlebon commented Feb 14, 2018

Migrated from https://lists.projectatomic.io/projectatomic-archives/atomic-devel/2018-February/msg00037.html.


Hi,

I think we have git some regression in rpm-ostreed on CentOS Atomic in
our Cockpit integration tests:

https://bugs.centos.org/view.php?id=14493

We found this when updating ou continuous-atomic image:

https://github.com/cockpit-project/cockpit/pull/8548
https://fedorapeople.org/groups/cockpit/logs/pull-8548-20180207-130707-8b3ca05f-verify-continuous-atomic/log.html

I would like to make you guys aware of this straight away, in case it is
something interesting for upstream.

Basically, GetCachedRebaseRpmDiff now seems to return the wrong checksum
and version. Maybe this is caused by the way we construct our ostree
test repositories.

Reproducer steps dumped from the bug report:

mkdir -p /var/zrepo
mkdir -p /tmp/rpm-data/usr/share
cp -r /usr/share/rpm /tmp/rpm-data/usr/share/
ostree init --repo /var/zrepo --mode archive-z2
ostree commit --repo=/var/zrepo -b zremote-branch1 --orphan --tree=dir=/tmp/rpm-data --add-metadata-string version=zremote-branch1.1

ostree remote add zremote-test1 http://127.0.0.1:12345 --no-gpg-verify
rpm-ostree reload
<start a trivial webserver that serves /var/zrepo from http://127.0.0.1:12345>

# gdbus call -y -d org.projectatomic.rpmostree1 -o /org/projectatomic/rpmostree1/centos_atomic_host -m org.projectatomic.rpmostree1.OS.DownloadRebaseRpmDiff "zremote-test1:zremote-branch1" "[]"
# gdbus call -y -d org.projectatomic.rpmostree1 -o /org/projectatomic/rpmostree1/centos_atomic_host -m org.projectatomic.rpmostree1.OS.GetCachedRebaseRpmDiff "zremote-test1:zremote-branch1" "[]"
(@a(sua{sv}) [], {'osname': <'centos-atomic-host'>, 'timestamp': <uint64 1518611341>, 'origin': <'zremote-test1:zremote-branch1'>, 'checksum': <'6a5a02b7fe761338591c5a1e28963acbfb391bc50d3a1cd68c42974d34364ebc'>, 'gpg-enabled': <false>, 'version': <'cockpit-base.1'>})

# rpm-ostree status
State: idle; auto updates disabled
Deployments:
* ostree://local:centos-atomic-host/7/x86_64/devel/continuous
                   Version: cockpit-base.1 (2018-02-14 12:29:01)
                    Commit: 6a5a02b7fe761338591c5a1e28963acbfb391bc50d3a1cd68c42974d34364ebc

  ostree://centos-atomic-continuous:centos-atomic-host/7/x86_64/devel/continuous
                   Version: 7.2017.1026 (2018-02-05 16:09:11)
                    Commit: e5e1d67af13843af97dec2774a00cb7ddf5857a413b1f2664b01b294eecf3585

You can see that GetCachedRebaseRpmDiff returns the checksum of the
current deployment instead of the one for zremote-test1:zremote-branch1.

@jlebon jlebon added the bug label Feb 14, 2018
@jlebon jlebon self-assigned this Feb 14, 2018
@jlebon jlebon added the jira for syncing to jira label Feb 14, 2018
jlebon added a commit to jlebon/rpm-ostree that referenced this issue Feb 14, 2018
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: coreos#1250
@jlebon
Copy link
Member Author

jlebon commented Feb 14, 2018

@mvollmer, this should be fixed by #1253. And thanks for tracking the continuous stream. That's awesome! 👍

@mvollmer
Copy link

this should be fixed by #1253.

Nice, thanks!

You call this the "Legacy API". Should we be using something else?

@jlebon
Copy link
Member Author

jlebon commented Feb 15, 2018

Yeah, I should've been more specific. Those D-Bus methods themselves aren't legacy, though the underlying internal APIs they use are. This intersects with the new automatic updates features we've been working on (see #247).

We should discuss at some point how this could be integrated into Cockpit! Basically, switch to a model where Cockpit can control whether automatic updates are turned on/off and display pending updates. (The difference being that users don't even have to click Check for Updates: the rpm-ostree timer will do this at intervals and publish results in the CachedUpdate property). This API still gets you an RPM diff, but with significantly lower costs (in pure OSTree mode at least). For Rebase and Deploy operations though, we can probably just try to make the same Download(Rebase/Deploy)RpmDiff methods more efficient when possible.

Anyway, we can wait on this a bit. I think a rework of that UI would definitely be worth it once we also support an auto-update "pending deployment" mode where users simply need to reboot to update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug jira for syncing to jira
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants