Bug 1161501 - [Build 20200120] Update notification applet does not like locked DB
[Build 20200120] Update notification applet does not like locked DB
Status: RESOLVED FIXED
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: KDE Applications
Current
Other Other
: P5 - None : Normal (vote)
: ---
Assigned To: E-Mail List
E-mail List
https://openqa.opensuse.org/tests/115...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2020-01-22 09:08 UTC by Dominique Leuenberger
Modified: 2020-02-24 15:28 UTC (History)
6 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dominique Leuenberger 2020-01-22 09:08:01 UTC
## Observation

openQA test in scenario opensuse-Tumbleweed-DVD-x86_64-install_only@64bit-multipath fails in
[first_boot](https://openqa.opensuse.org/tests/1151232/modules/first_boot/steps/2)

This happened due to the rewrite of the service purge-kernles-service being enabled, which runs:
ExecStart=/usr/bin/zypper purge-kernels


The Updater applet takes this as an indication to warn the user about 'packagemanager locked' - which can happen at any time and should, imho, not be told to the user, but the action be retried later


## Test suite description
set HDDSIZE=40 as required for ppc64le (failed w/o it on https://openqa.opensuse.org/tests/390330#step/install_and_reboot/21


## Reproducible

Fails since (at least) Build [20200120](https://openqa.opensuse.org/tests/1150886)


## Expected result

Last good: [20200119](https://openqa.opensuse.org/tests/1149615) (or more recent)


## Further details

Always latest result in this scenario: [latest](https://openqa.opensuse.org/tests/latest?arch=x86_64&distri=opensuse&flavor=DVD&machine=64bit-multipath&test=install_only&version=Tumbleweed)
Comment 1 Dominique Leuenberger 2020-01-22 09:09:18 UTC
As a quick workaround for Snapshot 0121 I removed the service again from the pattern - meaning the kernel purge service won't be enabled and thus the PMnot be locked. This should get 0121 at least to pass openQA again, But that is merely fighting the symptom.
Comment 2 Fabian Vogt 2020-01-22 12:03:47 UTC
[10:02] <DimStar> I think that actual fix woul dbe for QtPK not to complain right away if the PM is locked and retry later - a locked DB can happen at any time and an automatic update notifier should not consider that a problem

The "system management is locked" error maps to PackageKit::Transaction::ErrorFailedInitialization, with no other way to tell for sure that it was a locking issue (the message is translated).
So I don't think ignoring this particular error is a good idea, as it might also catch important errors.

Instead, I made a patch which does this:

+    KNotification::NotificationFlags flags = KNotification::Persistent;
+
+    const static std::set<PackageKit::Transaction::Error> transientErrors
+                           {PackageKit::Transaction::ErrorFailedInitialization, PackageKit::Transaction::ErrorNoNetwork,
+                            PackageKit::Transaction::ErrorInternalError};
+    if(!m_isManualCheck && transientErrors.count(error) > 0) {
+        qDebug(PLASMA_PK_UPDATES) << "Making notification not persisting for likely transient error during automatic check";
+        flags = KNotification::CloseOnTimeout;
+    }

So if it was an automatic "refresh" operation and one of those errors occured, the notification is not persistent.

Stefan: What do you think?
Comment 3 Stefan Brüns 2020-01-22 15:37:35 UTC
On the DBUS level, I get the following:

signal time=1579705625.780028 sender=:1.1025 -> destination=(null destination) serial=46 path=/1_ccccdaea; interface=org.freedesktop.PackageKit.Transaction; member=ErrorCode
   uint32 22
   string "Die Systemverwaltung ist gesperrt durch die Anwendung mit PID 11278 (zypper).
Schließen Sie diese Anwendung, bevor Sie es erneut versuchen."


ErrorCode=22:
{PK_ERROR_ENUM_FAILED_INITIALIZATION,	"failed-initialization"},
https://github.com/hughsie/PackageKit/blob/16a85736ff7986bc42c690fb9bee42473ac24cae/lib/packagekit-glib2/pk-enum.c#L157

originating here:
https://github.com/hughsie/PackageKit/blob/master/backends/zypp/pk-backend-zypp.cpp#L607

due to the ZyppFactoryException thrown here:
https://github.com/openSUSE/libzypp/blob/5eb3dec91f218bfe0b0874804558fea622b2624a/zypp/ZYppFactory.cc#L404

{PK_ERROR_ENUM_CANNOT_GET_LOCK,	"cannot-get-lock"} would have been more appropriate for the exception, but as _FAILED_INITIALIZATION is only used when aquiring the lock it is still usable.
Comment 4 Fabian Vogt 2020-01-22 15:54:17 UTC
(In reply to Stefan Brüns from comment #3)
> ErrorCode=22:
> {PK_ERROR_ENUM_FAILED_INITIALIZATION,	"failed-initialization"},

Yes, that's exactly what I meant by

> The "system management is locked" error maps to PackageKit::Transaction::ErrorFailedInitialization, with no other way to tell for sure that it was a locking issue (the message is translated).
Comment 5 Stefan Brüns 2020-01-22 18:59:09 UTC
I wanted to point out you *can* rely on PackageKit::Transaction::ErrorFailedInitialization to detect the locked database case currently.

What do you think about making pk-backend-zypp.cpp return PK_ERROR_ENUM_CANNOT_GET_LOCK instead?

Why is PackageKit::Transaction::ErrorInternalError supposed to be transient?
Comment 6 Fabian Vogt 2020-01-23 10:17:42 UTC
(In reply to Stefan Brüns from comment #5)
> I wanted to point out you *can* rely on
> PackageKit::Transaction::ErrorFailedInitialization to detect the locked
> database case currently.

So should the message just not be shown and the refresh/get-updates attempt skipped completely? The issue is that there's no way to see that something failed previously in the applet, only the "Last refresh: 42 days ago" would show that.
Implementing something like "show an error if the second try failed" could be done as well, but needs slightly more work.

> What do you think about making pk-backend-zypp.cpp return
> PK_ERROR_ENUM_CANNOT_GET_LOCK instead?

Fits perfectly. I assumed it would be for some PK internal lock only, but apt uses that one as well.

> Why is PackageKit::Transaction::ErrorInternalError supposed to be transient?

On a second look, it seems like PK doesn't even use that anymore. So I'd drop that before submission.
Comment 7 Fabian Vogt 2020-02-15 14:24:32 UTC
Submitted a patch: https://phabricator.kde.org/D27423
Comment 8 Fabian Vogt 2020-02-24 15:28:09 UTC
(In reply to Fabian Vogt from comment #7)
> Submitted a patch: https://phabricator.kde.org/D27423

Submitted to oS:F, let's hope for the best.