From patchwork Tue Jul 16 23:46:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Jakob L. Kreuze" X-Patchwork-Id: 14699 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 52BA217234; Wed, 17 Jul 2019 00:47:10 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTP id 767BA17232 for ; Wed, 17 Jul 2019 00:47:08 +0100 (BST) Received: from localhost ([::1]:52930 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hnXA3-0003yp-Vi for patchwork@mira.cbaines.net; Tue, 16 Jul 2019 19:47:07 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:36998) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hnXA0-0003yd-8J for guix-patches@gnu.org; Tue, 16 Jul 2019 19:47:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hnX9y-00067o-SK for guix-patches@gnu.org; Tue, 16 Jul 2019 19:47:04 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:42511) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hnX9y-00067h-PA for guix-patches@gnu.org; Tue, 16 Jul 2019 19:47:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hnX9y-00041N-Kw for guix-patches@gnu.org; Tue, 16 Jul 2019 19:47:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#36555] [PATCH v3 0/3] Refactor out common behavior for system reconfiguration. Resent-From: zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 16 Jul 2019 23:47:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36555 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 36555@debbugs.gnu.org Received: via spool by 36555-submit@debbugs.gnu.org id=B36555.156332079515424 (code B ref 36555); Tue, 16 Jul 2019 23:47:02 +0000 Received: (at 36555) by debbugs.gnu.org; 16 Jul 2019 23:46:35 +0000 Received: from localhost ([127.0.0.1]:51332 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hnX9V-00040g-HF for submit@debbugs.gnu.org; Tue, 16 Jul 2019 19:46:33 -0400 Received: from mx.sdf.org ([205.166.94.20]:54788) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hnX9R-00040U-LL for 36555@debbugs.gnu.org; Tue, 16 Jul 2019 19:46:32 -0400 Received: from Upsilon (mobile-107-107-59-57.mycingular.net [107.107.59.57]) (authenticated (0 bits)) by mx.sdf.org (8.15.2/8.14.5) with ESMTPSA id x6GNkKf9028996 (using TLSv1.2 with cipher AES256-GCM-SHA384 (256 bits) verified NO); Tue, 16 Jul 2019 23:46:25 GMT From: zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) References: <87imsci9sj.fsf@sdf.lonestar.org> <87ef30i9fl.fsf@sdf.lonestar.org> <87y3129qsn.fsf@gnu.org> <87sgr9bziq.fsf@sdf.lonestar.org> <87pnmc7nt1.fsf@gnu.org> <8736j7nwcb.fsf@sdf.lonestar.org> <87muhfjm14.fsf@gnu.org> <87ftn63l7d.fsf@sdf.lonestar.org> Date: Tue, 16 Jul 2019 19:46:16 -0400 In-Reply-To: <87ftn63l7d.fsf@sdf.lonestar.org> (Jakob L. Kreuze's message of "Mon, 15 Jul 2019 19:57:26 -0400") Message-ID: <87v9w1zgon.fsf_-_@sdf.lonestar.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches Hi, all. Submitting this reroll to ask for some further feedback. Here's a summary of the more significant changes since v2: - All of the system tests for the reconfiguration procedures have been implemented. - 'upgrade-services-program' has been completely reimplemented; '(gnu machine ssh)' is now capable of (partially) serializing the objects returned by 'current-services', so we can use 'shepherd-service-upgrade' to traverse the service dependency graph. - Procedures in '(guix scripts system reconfigure)' now use 'program-file' instead of 'gexp->script'. I hadn't realized the difference, but this makes invocations of 'remote-eval' a bit cleaner. - Thanks to Ludovic's patches to '(guix remote)', the reconfiguration procedures no longer need to capture output from the activation/installation scripts. - I've removed my awful hack of a solution for handling Shepherd errors in 'upgrade-services-program' in favor of handling exceptions on the host side. I have some questions about this. - 'upgrade-services-program' comes after 'install-bootloader-program' in 'guix deploy' and 'guix system reconfigure' now, as it's the procedure most likely to fail trivially. I still need to handle failed deployments in 'guix deploy'. I suspect that, for now, it would make sense to implement remote roll-backs and just roll-back the system on failure, at least until we've have some dialog about the proper way to do atomic deployments. My biggest concern at the moment is error handling reporting in the new 'guix system reconfigure'. I'd like to emulate what was done with the previous version, but I'm at somewhat of a loss for how I'd go about that, since the error reporting was mixed with the reconfiguration code. So I'd like to ask for some suggestions: is the best way to catch errors in '%store-monad' to do what 'with-shepherd-error-handling' does, and then 'leave' on failure? Ludovic suggested guarding against 'message-condition' and having the expression I send to 'remote-eval' return either ('error message) or ('success). Would it make sense to just do this in all of the reconfiguration procedures? Or is raising exceptions in the reconfiguration procedures and catching them in the scripts' code the way to go? There's also a slight bug in the new 'guix system reconfigure' that I'll need to figure out. At the moment, it installs a bootloader entry for all but the newest generation. Jakob L. Kreuze writes: > Noted. That should be a relatively small change, so I'll see about > tackling that in my next revision for this series. Oh, how naïve I was four days ago. This reroll doesn't address this. Having the procedures "parameterized by an evaluation procedure" can be done in so many ways, and I think it would be best I put some serious thought into which of those ways would be the best. A 'local-eval' would clearly be much better than what I'm doing at the present in 'system.scm', but the solution I came up with today involved three layers of 'primitive-load', which I doubt is the way to go about it. I had the idea to parameterize on a procedure that takes a '' rather than a G-Expression as I was making dinner tonight, which seems to me like a sound idea, but we'll see if it works tomorrow when I try to implement it. Also, it hit me today that the safety checks done in 'guix system reconfigure' -- 'check-mapped-devices', 'check-file-system-availability', and 'check-initrd-modules' -- should also be done in 'guix deploy'. It might make sense for me to submit that change as a separate patch series so the code review for this doesn't get too complicated, but since we're on the topic of unifying the code between 'guix deploy' and 'guix system reconfigure', should I perhaps reimplement those procedures as '' objects like everything else in '(guix scripts system reconfigure)'? They aren't really effectful, but they concern system reconfiguration. And, on the same note, should I go ahead and refactor the rest of the reconfiguration code in 'system.scm' out into '(guix scripts system reconfigure)'? I mean, this will probably be a separate patch series for the same reason that the safety checks would be a separate patch series, and I'll likely do this _after_ I come up with a decent way to parameterize on an evaluation procedure, but I'd like to know if it's a good idea or not before going ahead and ripping apart 'system.scm'. Regards, and TYIA for reviewing this. Jakob Jakob L. Kreuze (3): guix system: Add 'reconfigure' module. guix system: Reimplement 'reconfigure'. tests: Add reconfigure system test. Makefile.am | 1 + gnu/local.mk | 1 + gnu/machine/ssh.scm | 266 ++++++++++----------------- gnu/services/herd.scm | 6 + gnu/tests/reconfigure.scm | 268 ++++++++++++++++++++++++++++ guix/scripts/system.scm | 152 +++++----------- guix/scripts/system/reconfigure.scm | 122 +++++++++++++ tests/services.scm | 4 - 8 files changed, 538 insertions(+), 282 deletions(-) create mode 100644 gnu/tests/reconfigure.scm create mode 100644 guix/scripts/system/reconfigure.scm