Merge pull request #165 from tgross/gh163_sighup_reload

Fix SIGHUP

core/app.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"os/signal"
 	"sync"
 	"syscall"
 	"time"
@@ -32,20 +31,20 @@ var (
 // after it is run, it can be reloaded and paused with signals.
 type App struct {
 	ServiceBackend discovery.ServiceBackend
-	Services         []*services.Service
-	Backends         []*backends.Backend
-	Tasks            []*tasks.Task
-	Telemetry        *telemetry.Telemetry
-	PreStartCmd      *exec.Cmd
-	PreStopCmd       *exec.Cmd
-	PostStopCmd      *exec.Cmd
-	Command          *exec.Cmd
-	StopTimeout      int
-	QuitChannels     []chan bool
-	maintModeLock    *sync.RWMutex
-	signalLock       *sync.RWMutex
-	paused           bool
-	ConfigFlag       string
+	Services       []*services.Service
+	Backends       []*backends.Backend
+	Tasks          []*tasks.Task
+	Telemetry      *telemetry.Telemetry
+	PreStartCmd    *exec.Cmd
+	PreStopCmd     *exec.Cmd
+	PostStopCmd    *exec.Cmd
+	Command        *exec.Cmd
+	StopTimeout    int
+	QuitChannels   []chan bool
+	maintModeLock  *sync.RWMutex
+	signalLock     *sync.RWMutex
+	paused         bool
+	ConfigFlag     string
 }
 
 // EmptyApp creates an empty application
@@ -102,6 +101,7 @@ func NewApp(configFlag string) (*App, error) {
 	a.Backends = cfg.Backends
 	a.Tasks = cfg.Tasks
 	a.Telemetry = cfg.Telemetry
+	a.ConfigFlag = configFlag
 	return a, nil
 }
 
@@ -228,7 +228,6 @@ func (a *App) Reload() error {
 
 	a.stopPolling()
 	a.forAllServices(deregisterService)
-	signal.Reset()
 
 	a.load(newApp)
 	return nil
@@ -246,7 +245,6 @@ func (a *App) load(newApp *App) {
 	}
 	a.Telemetry = newApp.Telemetry
 	a.Tasks = newApp.Tasks
-	a.handleSignals()
 	a.handlePolling()
 }
 

integration_tests/fixtures/app/Dockerfile

@@ -9,9 +9,12 @@ RUN npm install -g json http-server
 
 COPY build/containerpilot /bin/containerpilot
 COPY app-with-consul.json /app-with-consul.json
+COPY app-with-consul-prestart-sigusr1.json /app-with-consul-prestart-sigusr1.json
+COPY app-with-consul-prestart-sighup.json /app-with-consul-prestart-sighup.json
 COPY app-with-etcd.json /app-with-etcd.json
 COPY reload-app.sh /reload-app.sh
 COPY reload-app-etcd.sh /reload-app-etcd.sh
+COPY reload-app-prestart.sh /reload-app-prestart.sh
 COPY sensor.sh /sensor.sh
 COPY task.sh /task.sh
 

integration_tests/fixtures/app/app-with-consul-prestart-sighup.json

@@ -0,0 +1,45 @@
+{
+  "consul": "consul:8500",
+  "preStart": ["/reload-app-prestart.sh", "HUP"],
+  "logging": {
+    "level": "DEBUG",
+    "format": "text"
+  },
+  "services": [
+    {
+      "name": "app",
+      "port": 8000,
+      "health": "/usr/bin/curl --fail -s -o /dev/null http://localhost:8888",
+      "poll": 1,
+      "ttl": 5,
+      "tags": ["application"]
+    }
+  ],
+  "backends": [
+    {
+      "name": "nginx",
+      "poll": 7,
+      "onChange": "/reload-app.sh"
+    },
+    {
+      "name": "app",
+      "poll": 5,
+      "onChange": "/reload-app.sh",
+      "tag": "application"
+    }
+  ],
+  "telemetry": {
+    "port": 9090,
+    "sensors": [
+       {
+        "namespace": "containerpilot",
+        "subsystem": "app",
+        "name": "some_counter",
+        "help": "help text",
+        "type": "counter",
+        "poll": 1,
+        "check": ["/sensor.sh", "count"]
+       }
+    ]
+  }
+}

integration_tests/fixtures/app/app-with-consul-prestart-sigusr1.json

@@ -0,0 +1,45 @@
+{
+  "consul": "consul:8500",
+  "preStart": ["/reload-app-prestart.sh", "USR1"],
+  "logging": {
+    "level": "DEBUG",
+    "format": "text"
+  },
+  "services": [
+    {
+      "name": "app",
+      "port": 8000,
+      "health": "/usr/bin/curl --fail -s -o /dev/null http://localhost:8000",
+      "poll": 1,
+      "ttl": 5,
+      "tags": ["application"]
+    }
+  ],
+  "backends": [
+    {
+      "name": "nginx",
+      "poll": 7,
+      "onChange": "/reload-app.sh"
+    },
+    {
+      "name": "app",
+      "poll": 5,
+      "onChange": "/reload-app.sh",
+      "tag": "application"
+    }
+  ],
+  "telemetry": {
+    "port": 9090,
+    "sensors": [
+       {
+        "namespace": "containerpilot",
+        "subsystem": "app",
+        "name": "some_counter",
+        "help": "help text",
+        "type": "counter",
+        "poll": 1,
+        "check": ["/sensor.sh", "count"]
+       }
+    ]
+  }
+}

integration_tests/fixtures/app/reload-app-prestart.sh

@@ -0,0 +1,22 @@
+#!/bin/bash
+# Sends a signal to ContainerPilot during the preStart
+
+# wait a few seconds for the Consul container to become available
+n=0
+while true
+do
+    if [ n == 10 ]; then
+        echo "Timed out waiting for Consul"
+        exit 1;
+    fi
+    curl -Ls --fail http://consul:8500/v1/status/leader | grep 8300 && break
+    n=$((n+1))
+    sleep 1
+done
+
+if [[ ${1} == "HUP" ]]; then
+    # Change our config to actually pass the healthcheck
+    sed -i s/8888/8000/ /app-with-consul-prestart-sighup.json
+fi
+
+kill -${1} 1

integration_tests/fixtures/app/reload-app.sh

@@ -1,5 +1,18 @@
 #!/bin/bash
 
+# wait a few seconds for the Consul container to become available
+n=0
+while true
+do
+    if [ n == 10 ]; then
+        echo "Timed out waiting for Consul"
+        exit 1;
+    fi
+    curl -Ls --fail http://consul:8500/v1/status/leader | grep 8300 && break
+    n=$((n+1))
+    sleep 1
+done
+
 # get all the healthy application servers and write the json to file
 curl -s consul:8500/v1/health/service/app?passing | json > /tmp/lastQuery.json
 

integration_tests/fixtures/test_probe/src/docker.go

@@ -13,6 +13,7 @@ const (
 	SigChld = DockerSignal("CHLD")
 	SigUsr1 = DockerSignal("USR1")
 	SigHup  = DockerSignal("HUP")
+	SigAbrt = DockerSignal("ABRT") // for debugging via stack trace dump
 )
 
 // DockerProbe is a test probe for docker

integration_tests/fixtures/test_probe/src/main.go

@@ -14,9 +14,11 @@ var AllTests map[string]TestCommand
 func runTest(testName string, args []string) {
 	// Register Tests
 	AllTests = map[string]TestCommand{
-		"test_sigterm":         TestSigterm,
-		"test_sighup_deadlock": TestSighupDeadlock,
-		"test_discovery":       TestDiscovery,
+		"test_sigterm":          TestSigterm,
+		"test_sighup_deadlock":  TestSighupDeadlock,
+		"test_sigusr1_prestart": TestSigUsr1Prestart,
+		"test_sighup_prestart":  TestSigHupPrestart,
+		"test_discovery":        TestDiscovery,
 	}
 
 	if test := AllTests[testName]; test != nil {

integration_tests/fixtures/test_probe/src/test_sighup_prestart.go

@@ -0,0 +1,25 @@
+package main
+
+import "log"
+
+// The health check in the containerpilot config is intentionally
+// broken. The preStart script will fix the health check and then
+// SIGHUP to perform a config reload.
+func TestSigHupPrestart(args []string) bool {
+	if len(args) != 1 {
+		log.Println("TestSigHupPrestart requires 1 argument")
+		log.Println(" - containerID: docker container to kill")
+		return false
+	}
+
+	consul, err := NewConsulProbe()
+	if err != nil {
+		log.Println(err)
+	}
+	// Wait for 1 healthy 'app' service to be registered with consul
+	if err = consul.WaitForServices("app", "", 1); err != nil {
+		log.Printf("Expected app to be healthy after SIGHUP: %s\n", err)
+		return false
+	}
+	return true
+}

integration_tests/fixtures/test_probe/src/test_sigusr1_prestart.go

@@ -0,0 +1,34 @@
+package main
+
+import "log"
+
+func TestSigUsr1Prestart(args []string) bool {
+	if len(args) != 1 {
+		log.Println("TestSigUsr1Prestart requires 1 argument")
+		log.Println(" - containerID: docker container to kill")
+		return false
+	}
+
+	docker, err := NewDockerProbe()
+	if err != nil {
+		log.Println(err)
+	}
+
+	// Prestart will SIGUSR1 us into maintenance
+	// Send SIGUSR1 to get us back out of maintenance
+	if err = docker.SendSignal(args[0], SigUsr1); err != nil {
+		log.Println(err)
+		return false
+	}
+
+	// Wait for app to be healthy
+	consul, err := NewConsulProbe()
+	if err != nil {
+		log.Println(err)
+	}
+	if err = consul.WaitForServices("app", "", 1); err != nil {
+		log.Printf("Expected app to be healthy after SIGUSR1: %s\n", err)
+		return false
+	}
+	return true
+}

integration_tests/tests/test_sighup_deadlock/docker-compose.yml

@@ -8,6 +8,8 @@ app:
     mem_limit: 512m
     links:
       - consul:consul
+    volumes:
+      - '${CONTAINERPILOT_BIN}:/bin/containerpilot:ro'
 
 test:
     image: "cpfix_test_probe"

integration_tests/tests/test_sighup_prestart/docker-compose.yml

@@ -0,0 +1,23 @@
+consul:
+    image: "cpfix_consul"
+    mem_limit: 256m
+    hostname: consul
+
+app:
+    image: "cpfix_app"
+    environment:
+      - CONTAINERPILOT=file:///app-with-consul-prestart-sighup.json
+    mem_limit: 512m
+    links:
+      - consul:consul
+    volumes:
+      - '${CONTAINERPILOT_BIN}:/bin/containerpilot:ro'
+
+test:
+    image: "cpfix_test_probe"
+    mem_limit: 128m
+    links:
+      - consul:consul
+      - app:app
+    volumes:
+      - '/var/run/docker.sock:/var/run/docker.sock'

integration_tests/tests/test_sighup_prestart/run.sh

@@ -0,0 +1,15 @@
+#!/bin/bash
+
+docker-compose up -d consul app
+APP_ID="$(docker-compose ps -q app)"
+docker-compose run --no-deps test /go/bin/test_probe test_sighup_prestart $APP_ID > /dev/null 2>&1
+result=$?
+TEST_ID=$(docker ps -l -f "ancestor=cpfix_test_probe" --format="{{.ID}}")
+if [ $result -ne 0 ]; then
+  echo "==== TEST LOGS ===="
+  docker logs $TEST_ID
+  echo "==== APP LOGS ===="
+  docker logs $APP_ID
+fi
+docker rm -f $TEST_ID > /dev/null 2>&1
+exit $result