Merge lp:~morphis/core-snap/add-configure-hook-sshd-service into lp:core-snap

[Simon Fels]

Add configure hook which only accepts a single option service.ssh.disable for now

The purpose of the service.ssh.disable option is to either enable or disable the ssh service on the system. A management snap can then use the snapd-control interface to change the configuration and enable/disable ssh depending on its needs.

[Michael Vogt (mvo)]

Looks good!

review: Approve

[Simon Fels (morphis)]

The hook needs plugs added to allow access to /etc/ssh and restarting services via systemd. See https://github.com/snapcore/snapd/pull/2683 and https://github.com/snapcore/snapd/pull/2687

[Simon Fels (morphis)]

Using now the new 'core-support' interface which was explicitly introduced for things the core has to do like this configure hook. See https://github.com/snapcore/snapd/pull/2706 for more details.

[Simon Fels (morphis)]

Spread test added to snapd with https://github.com/snapcore/snapd/pull/2723

[Michael Vogt (mvo)]

:+1:

review: Approve

[Oliver Grawert (ogra)]

looks fine to me

review: Approve

[Simon Fels (morphis)]

Switched to stop/disable and enable/start as suggested by Gustavo on Telegram. Status is back to work-in-progress as I am using this MP as a base to generate a new core snap on launchpad.

Also needs https://github.com/snapcore/snapd/pull/2726/files

[John Lenton (chipaca)]

This is broken as is, unless i'm very mistaken.

'let' is a bashism

and

while $n -lt 10 $wait_attempts ! is_ssh_unit_enabled

is missing an &&

[John Lenton (chipaca)]

review: Needs Fixing

[lp:~morphis/core-snap/add-configure-hook-sshd-service]

56. By Simon Fels on 2017-01-26

Fix leftover invalid shell syntax

[lp:~morphis/core-snap/add-configure-hook-sshd-service]

57. By Simon Fels on 2017-01-26

Switch stop+disable to disable+stop

[lp:~morphis/core-snap/add-configure-hook-sshd-service]

58. By Simon Fels on 2017-01-26

Fix condition

[Simon Fels (morphis)]

Fixed all review comments.

[Oliver Grawert (ogra)]

added some inline comments

[lp:~morphis/core-snap/add-configure-hook-sshd-service]

59. By Simon Fels on 2017-01-26

Respect review comments

60. By Simon Fels on 2017-01-26

Add missing brackets

[Michael Vogt (mvo)]

Some nitpick.

[lp:~morphis/core-snap/add-configure-hook-sshd-service]

61. By Simon Fels on 2017-01-27

Rework hook again to use plain systemctl

[Simon Fels (morphis)]

After some more discussion this was now reworked to use systemctl again. See https://github.com/snapcore/snapd/pull/2726 for the required changes to the core-support interface.

Name of the configure option is now 'service.ssh.disable' following Gustavo's suggestion.

Also note that proper documentation for this was added with https://github.com/CanonicalLtd/ubuntu-core-docs/pull/7 which now needs a change to respect the changed name.

[lp:~morphis/core-snap/add-configure-hook-sshd-service]

62. By Simon Fels on 2017-01-27

Correct configure hook implementation

63. By Simon Fels on 2017-01-27

disable with --now doesn't work well for ssh.service

64. By Simon Fels on 2017-01-27

Only enable/disable when really needed

[Michael Vogt (mvo)]

Some nitpick

[lp:~morphis/core-snap/add-configure-hook-sshd-service]

65. By Simon Fels on 2017-01-27

Rework logic again

66. By Simon Fels on 2017-01-27

Don't do anything when no value is set

=== added directory 'hooks'
=== added file 'hooks/configure'
--- hooks/configure	1970-01-01 00:00:00 +0000
+++ hooks/configure	2017-01-27 16:22:54 +0000
@@ -0,0 +1,47 @@
+#!/bin/sh -e
+#
+# Copyright (C) 2017 Canonical Ltd
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+switch_service_ssh() {
+	case "$1" in
+		false)
+			# When the unit is already enabled but not active a call with
+			# --now does start the unit so we have to check for that  case
+			# and explicitly start the unit.
+			if [ "$(systemctl is-enabled ssh.service)" = "disabled" ]; then
+				systemctl enable ssh.service
+			fi
+			if [ "$(systemctl is-active ssh.service)" = "inactive" ]; then
+				systemctl start ssh.service
+			fi
+			;;
+		true)
+			# A simple `systemctl disable --now ssh.service` doesn't work
+			# and fails with an error message. Because of that we're going
+			# in two steps here and disable first and then stopping the
+			# service unit.
+			systemctl disable ssh.service
+			systemctl stop ssh.service
+			;;
+		*)
+			echo "ERROR: Invalid value '$1' provided for option service.ssh.disable"
+			exit 1
+	esac
+}
+
+value=$(snapctl get service.ssh.disable)
+if [ -n "$value" ]; then
+	switch_service_ssh $value
+fi

=== modified file 'snapcraft.yaml'
--- snapcraft.yaml	2016-10-04 16:29:23 +0000
+++ snapcraft.yaml	2017-01-27 16:22:54 +0000
@@ -6,7 +6,18 @@
 type: os
 grade: stable
 
+hooks:
+  configure:
+    plugs:
+      - core-support
+
 parts:
+  hooks:
+    plugin: dump
+    source: hooks
+    organize:
+      configure: meta/hooks/configure
+
   livebuild:
     source: .
     plugin: make