packagekit: Add automatic update configuration for dnf #7709

martinpitt · 2017-09-19T08:59:22Z

At the moment this does not install dnf-automatic on demand - if it's not already installed, automatic updates are not supported and not shown. This will be added in a second iteration.

See UI design.

Only auto-reboot if there were actual updates applied
Add test that controls work if there are updates available
Hide update history when automatic updates are on, as PackageKit doesn't know about these
Fix null is not an object exception on unsupported backends
Fix docker test regressions (#containers-containers-filter button is disabled or somehow not clickable)
Improve grammar in config: [Apply ...] [on Mondays] at [06None] ...

image-refresh fedora-27

change state of on/off button to new state when disabling, less confusing

martinpitt · 2017-09-22T06:57:31Z

I now implemented the last TODO item, so that this is now an useful and working feature. It is currently limited to dnf (i. e. Fedora 26+) and for systems which have the dnf-automatic package already installed; on all others this will just not be shown. Future iterations on that will add on-demand installation of dnf-automatic, and support for apt-based systems.

By default, automatic updates are off:

Once you enable them, they default to "every day at 06:00", which seems like a reasonable default to me (a bit before most people start working and thus can intervene if updates broke something). Automatic reboots are always on and there is no option for it (I don't think automatic updates without automatic reboots would be very useful/advisable, particularly on a server).

You can change the type:

... or day:

... or time:

@garrett, this should be really close to your designs, but I'd still like you to sign off on it. Thanks in advance!

martinpitt · 2017-09-22T06:58:02Z

@mvollmer, can I ask you to give this a code review? Thanks!

garrett

@martinpitt and I talked about it and decided to move the every/on string into the dropdown for better language flow ("every day", "on Mondays", etc.), which is also better for translations. Also, we decided to add "at" between date and time (and it could be translated to an empty string when it's not appropriate).

martinpitt · 2017-09-26T08:46:34Z

Force-pushed to change the auto update config sentence/structure like discussed with @garrett above. I adjusted the screenshots in above comment to the new design.

martinpitt · 2017-09-26T10:01:11Z

fedora-27 image needs dnf-automatic installed.

This does not yet install dnf-automatic on demand - if it's not already installed, automatic updates are not supported and not shown. Closes cockpit-project#7709

cockpituous · 2017-09-26T10:13:23Z

image-refresh in progress on cockpit-tests-4kx0n.
Log: http://fedorapeople.org/groups/cockpit/logs/image-refresh-7709-20170926-101515/

cockpituous · 2017-09-26T10:13:40Z

image-refresh in progress on cockpit-tests-vlgmj.
Log: http://fedorapeople.org/groups/cockpit/logs/image-refresh-7709-20170926-101515/

cockpituous · 2017-09-26T10:13:53Z

image-refresh in progress on cockpit-tests-jttb7.
Log: http://fedorapeople.org/groups/cockpit/logs/image-refresh-7709-20170926-101414/

cockpituous · 2017-09-26T10:54:08Z

image-refresh fedora-27 done: https://github.com/cockpituous/cockpit/commits/image-refresh-fedora-27-20170926-105555

martinpitt · 2017-09-27T11:10:59Z

OS package updates: automatic

martinpitt · 2017-09-27T14:13:29Z

@mvollmer: Some details about the file writing that we do:

The main enabling/disabling happens through enabling/disabling the systemd unit, which IMHO is clean enough. (Only available for Fedora ≥ 26, but I think that's fine)
Switching between "all" and "security" is done in /etc/dnf/automatic.conf - there is no other interface for that.
Configuring time/date is by reading/setting the [Timer] option of the systemd timer unit. Parsing/writing that is a bit involved indeed, but it's the official interface, and we do that on the /system page too. This happens in a drop-in file, so we don't change existing files in the system.
Enabling automatic reboot is the ugly hack. I filed a bug about native support for that, but that might take a while. Question is whether we want to block on this. But at least it's a separate file (unit drop-in) which cockpit cleans up again if you disable the auto-updates again, so not too much chance of messing up an existing file.

mvollmer · 2017-09-28T10:10:39Z

I find the On/Off animation slightly confusing. When toggling On/Off, the button is first disabled while the machine is working, and then switches over to the other position. I think I would prefer it to first switch and then be disabled until the new state sticks.

Once I figured out how the button works, it didn't feel so wrong anymore, but when clicking it the first time and it didn't move immediately confused me quite a bit, and I actually clicked it again a couple of times just to get a feel for how it works.

mvollmer · 2017-09-28T10:26:17Z

pkg/packagekit/updates.jsx

@@ -837,7 +840,8 @@ class OsUpdates extends React.Component {
                        }
                        <UpdatesList updates={this.state.updates} />

-                        { this.state.history
+                        { /* Hide history with automatic updates, as they don't feed their history into PackageKit */


UpdateHistory is instanciated in two more places, AskRestart and when the system is up-to-date. Shouldn't it be hidden there, too?

It's intentionally not hidden for "ask to restart", as there we just did a PackageKit update and thus the history is recent and valid. But indeed, if I forgot to hide it for "up to date", this needs to be fixed.

mvollmer · 2017-09-28T11:04:21Z

pkg/packagekit/autoupdates.jsx

+                // (https://bugzilla.redhat.com/show_bug.cgi?id=1491190)
+                script += "mkdir -p /etc/systemd/system/dnf-automatic-install.service.d; ";
+                script += "printf '[Service]\\nExecStartPost=/bin/sh -ec \"" +
+                          "if systemctl status --no-pager --lines=100 dnf-automatic-install.service| grep -q ===========$$; then " +


Since we want to grep the journal, what about using journalctl? Also, does this really only look at the last run, or does it look at the last 100 lines?

This is exactly why I can't use journalctl for this - it doesn't have an option for "just show me the current run". One can sort of fake it with first doing a systemctl show, grab the started time from it, and plug that into journalctl --since, but overall that's more overhead than just using this. (I asked the systemd guys about that).

Oh, systemctl status only shows the journal for the current run? I didn't know that, nice!

mvollmer · 2017-09-28T11:22:14Z

test/verify/check-packagekit

+        # dial down the random sleep to avoid the test having to wait 5 mins
+        m.execute("sed -i '/random_sleep/ s/=.*$/= 3/' /etc/dnf/automatic.conf")
+        # then manually start the upgrade job like the timer would
+        m.execute("systemctl start dnf-automatic-install.service")


Add a --wait?

No need, it's Type=oneshot thus the start will block until it's actually done. Also, I remember adding --wait not too long ago (late last year), so it might not yet be available on RHEL/CentOS 7.

Roger. Looks like you know a thing or two about systemd... :-)

martinpitt · 2017-09-28T12:18:46Z

Force-pushed to hide the history for "up to date" too, thanks for spotting! That leaves the on/off button behaviour. I'm not sure how to influence this behaviour, it's a standard component - but I'll look into that. I added a TODO item for it for now.

mvollmer · 2017-09-29T08:11:18Z

That leaves the on/off button behaviour. I'm not sure how to influence this behaviour, it's a standard component

Would it work to use enabled ^ pending as the state of the button? E.g., while a change is pending, you show the opposite from what the current state is.

Similar to OnOffSwitch, this allows to gray out a Select component and make it inactive.

Corresponding to commit fb0f7d6 which did it for the older Fedora versions.

This does not yet install dnf-automatic on demand - if it's not already installed, automatic updates are not supported and not shown. Closes cockpit-project#7709

martinpitt · 2017-09-29T10:15:37Z

@mvollmer: The actual implementation is a tad more involved, but in spirit, yes. I did that now and force-pushed the change to keep button mergeability. But for easier review, here is the relative diff to your previous review.

martinpitt force-pushed the dnf-automatic branch from 0109301 to 9800bb0 Compare September 19, 2017 12:23

martinpitt force-pushed the dnf-automatic branch from 9800bb0 to ed8cf5b Compare September 19, 2017 16:55

martinpitt force-pushed the dnf-automatic branch from ed8cf5b to 7636e05 Compare September 19, 2017 20:50

martinpitt force-pushed the dnf-automatic branch from 7636e05 to 0f75960 Compare September 21, 2017 14:30

martinpitt force-pushed the dnf-automatic branch from 0f75960 to b4a3a59 Compare September 22, 2017 06:51

martinpitt changed the title ~~WIP: packagekit: Add automatic update configuration for dnf~~ packagekit: Add automatic update configuration for dnf Sep 22, 2017

martinpitt requested review from garrett and mvollmer September 22, 2017 06:57

garrett approved these changes Sep 26, 2017

View reviewed changes

martinpitt force-pushed the dnf-automatic branch from b4a3a59 to 97ddac3 Compare September 26, 2017 08:42

martinpitt force-pushed the dnf-automatic branch from 97ddac3 to d1063e9 Compare September 26, 2017 09:01

martinpitt added the needswork label Sep 26, 2017

martinpitt force-pushed the dnf-automatic branch from d1063e9 to 7f3bf0e Compare September 26, 2017 10:03

martinpitt added the bot label Sep 26, 2017

cockpituous changed the title ~~packagekit: Add automatic update configuration for dnf~~ WIP: cockpit-tests-4kx0n: packagekit: Add automatic update configuration for dnf Sep 26, 2017

cockpituous changed the title ~~WIP: cockpit-tests-4kx0n: packagekit: Add automatic update configuration for dnf~~ WIP: cockpit-tests-vlgmj: packagekit: Add automatic update configuration for dnf Sep 26, 2017

cockpituous changed the title ~~WIP: cockpit-tests-vlgmj: packagekit: Add automatic update configuration for dnf~~ WIP: cockpit-tests-jttb7: packagekit: Add automatic update configuration for dnf Sep 26, 2017

cockpituous changed the title ~~WIP: cockpit-tests-jttb7: packagekit: Add automatic update configuration for dnf~~ packagekit: Add automatic update configuration for dnf Sep 26, 2017

martinpitt force-pushed the dnf-automatic branch from 7f3bf0e to 48f9d87 Compare September 26, 2017 10:58

martinpitt removed bot needswork labels Sep 26, 2017

mvollmer reviewed Sep 28, 2017

View reviewed changes

martinpitt force-pushed the dnf-automatic branch from 48f9d87 to 0edfd50 Compare September 28, 2017 12:17

martinpitt added 3 commits September 29, 2017 12:06

lib: Add enabled property to Select component

9c1cab2

Similar to OnOffSwitch, this allows to gray out a Select component and make it inactive.

bots: Add dnf-automatic to fedora-27 image

39a026c

Corresponding to commit fb0f7d6 which did it for the older Fedora versions.

packagekit: Add automatic update configuration for dnf

2a49f1a

This does not yet install dnf-automatic on demand - if it's not already installed, automatic updates are not supported and not shown. Closes cockpit-project#7709

martinpitt force-pushed the dnf-automatic branch from 0edfd50 to 2a49f1a Compare September 29, 2017 10:13

mvollmer approved these changes Sep 29, 2017

View reviewed changes

mvollmer closed this in 76b620e Oct 4, 2017

martinpitt deleted the dnf-automatic branch October 4, 2017 10:14

martinpitt mentioned this pull request Oct 4, 2017

Image refresh for fedora-27 #7803

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

packagekit: Add automatic update configuration for dnf #7709

packagekit: Add automatic update configuration for dnf #7709

martinpitt commented Sep 19, 2017 •

edited

martinpitt commented Sep 22, 2017 •

edited

martinpitt commented Sep 22, 2017

garrett left a comment

martinpitt commented Sep 26, 2017

martinpitt commented Sep 26, 2017

cockpituous commented Sep 26, 2017

cockpituous commented Sep 26, 2017

cockpituous commented Sep 26, 2017

cockpituous commented Sep 26, 2017

martinpitt commented Sep 27, 2017

martinpitt commented Sep 27, 2017 •

edited

mvollmer commented Sep 28, 2017

mvollmer Sep 28, 2017

martinpitt Sep 28, 2017

mvollmer Sep 28, 2017

martinpitt Sep 28, 2017

mvollmer Sep 29, 2017

mvollmer Sep 28, 2017

martinpitt Sep 28, 2017

mvollmer Sep 29, 2017

martinpitt commented Sep 28, 2017

mvollmer commented Sep 29, 2017

martinpitt commented Sep 29, 2017

packagekit: Add automatic update configuration for dnf #7709

packagekit: Add automatic update configuration for dnf #7709

Conversation

martinpitt commented Sep 19, 2017 • edited

martinpitt commented Sep 22, 2017 • edited

martinpitt commented Sep 22, 2017

garrett left a comment

Choose a reason for hiding this comment

martinpitt commented Sep 26, 2017

martinpitt commented Sep 26, 2017

cockpituous commented Sep 26, 2017

cockpituous commented Sep 26, 2017

cockpituous commented Sep 26, 2017

cockpituous commented Sep 26, 2017

martinpitt commented Sep 27, 2017

martinpitt commented Sep 27, 2017 • edited

mvollmer commented Sep 28, 2017

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

martinpitt commented Sep 28, 2017

mvollmer commented Sep 29, 2017

martinpitt commented Sep 29, 2017

martinpitt commented Sep 19, 2017 •

edited

martinpitt commented Sep 22, 2017 •

edited

martinpitt commented Sep 27, 2017 •

edited