Fix review comments

 - Rename *jdisc-container* to *standalone-container* (for files in this PR)
 - Implement stop standalone-container command
 - Avoid e.g. vespa user deletion on upgrade

2 files changed, 107 insertions, 14 deletions

diff --git a/standalone-container/src/main/sh/jdisc-container b/standalone-container/src/main/sh/standalone-container.sh
index 5d023e5de4d..dd0693f6f85 100755
--- a/standalone-container/src/main/sh/jdisc-container
+++ b/standalone-container/src/main/sh/standalone-container.sh
@@ -94,6 +94,11 @@ StartCommand() {
 	Fail "Too many arguments"
     fi
 
+    local service_regex='^[0-9a-zA-Z_-]+$'
+    if ! [[ "$service" =~ $service_regex ]]; then
+	Fail "Service must match regex '$service_regex'"
+    fi
+
     # common setup
     export VESPA_SERVICE_NAME="$service"
 
@@ -110,11 +115,11 @@ StartCommand() {
 
     local vespa_log="$VESPA_HOME/logs/vespa/vespa.log"
     export VESPA_LOG_TARGET="file:$vespa_log"
-    mkdir -p "$(dirname "$vespa_log")"
+    FixDataDirectory "$(dirname "$vespa_log")"
 
     export VESPA_LOG_CONTROL_FILE="$VESPA_HOME/var/db/vespa/logcontrol/$service.logcontrol"
     export VESPA_LOG_CONTROL_DIR="$(dirname "$VESPA_LOG_CONTROL_FILE")"
-    mkdir -p "$VESPA_LOG_CONTROL_DIR"
+    FixDataDirectory "$VESPA_LOG_CONTROL_DIR"
 
     # Does not need fast allocation
     export MALLOC_ARENA_MAX=1
@@ -125,7 +130,7 @@ StartCommand() {
     # class path
     CP="$VESPA_HOME/lib/jars/jdisc_core-jar-with-dependencies.jar"
 
-    mkdir -p "$(dirname "$cfpfile")"
+    FixDataDirectory "$(dirname "$cfpfile")"
     printenv > "$cfpfile"
     FixDataDirectory "$bundlecachedir"
 
@@ -151,18 +156,105 @@ StartCommand() {
         -Djdisc.logger.tag="jdisc/$service" \
         -Dfile.encoding=UTF-8 \
         -cp "$CP" \
-        com.yahoo.jdisc.core.StandaloneMain standalone-container-jar-with-dependencies.jar
+        com.yahoo.jdisc.core.StandaloneMain standalone-container-jar-with-dependencies.jar &
+
+    local pid="$!"
+    echo "$pid" > "$pidfile"
+}
+
+Kill() {
+    local force="$1"
+    local expected_user="$2"
+    local expected_comm="$3" # Executable name only
+    local pid="$4"
+
+    local -i now
+    if ! now=$(date +%s); then
+	Fail "Failed to get the current date in seconds since epoch"
+    fi
+    local -i timeout=$(( now + 300 ))
+
+    local has_killed=false
+
+    while true; do
+	local ps_output=""
+	if ! ps_output=$(ps -p "$pid" -o user= -o comm=); then
+	    # success
+	    return
+	fi
+
+	local user comm
+	read -r user comm <<< "$ps_output"
+
+	if test "$user" != "$expected_user"; then
+	    echo "Warning: Pid collision ($pid): Expected user $expected_user but found $user."
+	    echo "Will assume original process has died."
+	    return
+	fi
+
+	if test "$comm" != "$expected_comm"; then
+	    echo "Warning: Pid collision ($pid): Expected program $expected_comm but found $comm."
+	    echo "Will assume original process has died."
+	    return
+	fi
+
+	if ! "$has_killed"; then
+	    if $force; then
+		if ! kill -KILL "$pid"; then
+		    Fail "Failed to kill $pid"
+		fi
+	    else
+		if ! kill "$pid"; then
+		    Fail "Failed to kill $pid"
+		fi
+	    fi
+
+	    has_killed=true
+	fi
+
+	sleep 1
+
+	now=$(date +%s)
+	if (( now >= timeout )); then
+	    Fail "Process $pid still exists after $timeout seconds, giving up"
+	fi
+    done
 }
 
 StopCommand() {
-    local service="$1"
+    local user="$1"
     shift
 
-    if (( $# > 0 )); then
-	Fail "Too many arguments"
+    local force=false
+    while (( $# > 0 )); do
+	case "$1" in
+	    -f|--force)
+		force=true
+		shift
+		;;
+	    *) break ;;
+	esac
+    done
+
+    if (( $# != 1 )); then
+	Fail "Stop command takes exactly one argument"
+    fi
+
+    local service="$1"
+
+    local pidfile="$VESPA_HOME/var/run/$service.pid"
+    if ! test -r "$pidfile"; then
+	echo "$service is not running"
+	return
+    fi
+
+    local pid=$(< "$pidfile")
+    if ! [[ "$pid" =~ ^[0-9]+$ ]]; then
+	Fail "Pid file '$pidfile' does not contain a valid pid: $pid"
     fi
 
-    Fail "Not implemented"
+    Kill "$force" "$user" java "$pid"
+    rm -f "$pidfile"
 }
 
 Main() {
@@ -205,12 +297,11 @@ Main() {
     elif test "$(id -un)" != "$user"; then
 	Fail "${0##*/} must be started by $user"
     fi
-    export VESPA_USER="$user"
 
     case "$command" in
 	help) Usage ;;
 	start) StartCommand "$service" "$@" ;;
-	stop) StopCommand "$service" "$@" ;;
+	stop) StopCommand "$user" "$service" "$@" ;;
 	*) Fail "Unknown command '$command'" ;;
     esac
 }
diff --git a/standalone-container/vespa-jdisc-container.spec b/standalone-container/vespa-standalone-container.spec
index 87def8881f6..5ab7e9409a5 100644
--- a/standalone-container/vespa-jdisc-container.spec
+++ b/standalone-container/vespa-standalone-container.spec
@@ -6,7 +6,7 @@
 # Hack to speed up jar packing for now. This does not affect the rpm size.
 %define __jar_repack %{nil}
 
-Name:           vespa-jdisc-container
+Name:           vespa-standalone-container
 Version:        %version
 BuildArch:      noarch
 Release:        1%{?dist}
@@ -64,7 +64,7 @@ cp vespajlib/target/vespajlib.jar "$jars_dir"
 
 declare -a libexec_files=(
   vespabase/src/common-env.sh
-  standalone-container/src/main/sh/jdisc-container
+  standalone-container/src/main/sh/standalone-container.sh
 )
 declare libexec_dir=%buildroot%_prefix/libexec/vespa
 mkdir -p "$libexec_dir"
@@ -85,8 +85,10 @@ echo "export VESPA_HOME=%_prefix" >> /etc/profile.d/vespa.sh
 chmod +x /etc/profile.d/vespa.sh
 
 %postun
-rm -f /etc/profile.d/vespa.sh
-userdel vespa
+if [ $1 -eq 0 ]; then # this is an uninstallation
+    rm -f /etc/profile.d/vespa.sh
+    userdel vespa
+fi
 
 %files
 %defattr(-,vespa,vespa,-)