From a3e1fec8814f304bfb395824241ccd83c3711ada Mon Sep 17 00:00:00 2001 From: ramr Date: Mon, 14 Aug 2017 01:19:07 -0700 Subject: [PATCH 1/2] Rework lib to add another exported entry point to pass down wait4 specific configuration and also test with bypassing pid 1 checks. --- reaper.go | 61 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/reaper.go b/reaper.go index e6a2175..ab5bce7 100644 --- a/reaper.go +++ b/reaper.go @@ -8,6 +8,10 @@ import "os" import "os/signal" import "syscall" +type Config struct { + Pid int + Options int +} // Handle death of child (SIGCHLD) messages. Pushes the signal onto the // notifications channel if there is a waiter. @@ -16,9 +20,9 @@ func sigChildHandler(notifications chan os.Signal) { signal.Notify(sigs, syscall.SIGCHLD) for { - var sig = <- sigs + var sig = <-sigs select { - case notifications <-sig: /* published it. */ + case notifications <- sig: /* published it. */ default: /* * Notifications channel full - drop it to the @@ -29,15 +33,17 @@ func sigChildHandler(notifications chan os.Signal) { } } -} /* End of function sigChildHandler. */ - +} /* End of function sigChildHandler. */ // Be a good parent - clean up behind the children. -func reapChildren() { +func reapChildren(config Config) { var notifications = make(chan os.Signal, 1) go sigChildHandler(notifications) + pid := config.Pid + opts := config.Options + for { var sig = <-notifications fmt.Printf(" - Received signal %v\n", sig) @@ -48,9 +54,9 @@ func reapChildren() { * Reap 'em, so that zombies don't accumulate. * Plants vs. Zombies!! */ - pid, err := syscall.Wait4(-1, &wstatus, 0, nil) + pid, err := syscall.Wait4(pid, &wstatus, opts, nil) for syscall.EINTR == err { - pid, err = syscall.Wait4(-1, &wstatus, 0, nil) + pid, err = syscall.Wait4(pid, &wstatus, opts, nil) } if syscall.ECHILD == err { @@ -63,9 +69,7 @@ func reapChildren() { } } -} /* End of function reapChildren. */ - - +} /* End of function reapChildren. */ /* * ====================================================================== @@ -73,21 +77,38 @@ func reapChildren() { * ====================================================================== */ -// Entry point for the reaper code. Start reaping children in the +// Normal entry point for the reaper code. Start reaping children in the // background inside a goroutine. func Reap() { /* * Only reap processes if we are taking over init's duties aka * we are running as pid 1 inside a docker container. */ - if 1 == os.Getpid() { - /* - * Ok, we are the grandma of 'em all, so we get to play - * the grim reaper. - * You will be missed, Terry Pratchett!! RIP - */ - go reapChildren() - } + if 1 == os.Getpid() { + config := Config{Pid: -1, Options: 0} + + /* + * Ok, we are the grandma of 'em all, so we get to play + * the grim reaper. + * You will be missed, Terry Pratchett!! RIP + */ + go reapChildren(config) + } + +} /* End of [exported] function Reap. */ -} /* End of [exported] function Reap. */ +// Entry point for invoking the reaper code bypassing pid 1 checks and +// with a specific configuration. Starts reaping children in the background +// inside a goroutine. +func Start(config Config) { + /* + * Start the Reaper with configuration options. This allows you to + * reap processes even if the current pid isn't running as pid 1. + * So ... use with caution!! + * + * In most cases, you are better off just using Reap() as that + * checks if we are running as Pid 1. + */ + go reapChildren(config) +} /* End of [exported] function Start. */ From 456483b4399aac58b4d008df5ef772b7fd65865b Mon Sep 17 00:00:00 2001 From: ramr Date: Mon, 14 Aug 2017 01:47:36 -0700 Subject: [PATCH 2/2] Add new tests for the Start api entry point and rework the tests so that we can run with different docker files. Yeah --mount would have been so easy to use but a few environments I tested in were running older docker versions. --- test/.gitignore | 1 + test/Makefile | 14 ++- test/config/reaper.json | 4 + test/{ => docker-files/no-config}/Dockerfile | 0 test/runtests.sh | 16 +++- test/testpid1.go | 94 ++++++++++++++++---- 6 files changed, 106 insertions(+), 23 deletions(-) create mode 100644 test/.gitignore create mode 100644 test/config/reaper.json rename test/{ => docker-files/no-config}/Dockerfile (100%) diff --git a/test/.gitignore b/test/.gitignore new file mode 100644 index 0000000..9414382 --- /dev/null +++ b/test/.gitignore @@ -0,0 +1 @@ +Dockerfile diff --git a/test/Makefile b/test/Makefile index dfe0b7b..911a799 100644 --- a/test/Makefile +++ b/test/Makefile @@ -1,11 +1,17 @@ #!/usr/bin/env make +TEST_IMAGE = "reaper/test" +CONFIG_TEST_IMAGE = "reaper/config-test" + all: tests test: tests tests: - (go build testpid1.go; docker build -t reaper/test .; ./runtests.sh) - - - + (go build testpid1.go; \ + cp docker-files/no-config/Dockerfile .; \ + docker build -t $(TEST_IMAGE) .; \ + ./runtests.sh; \ + cp docker-files/json-config/Dockerfile .; \ + docker build -t $(CONFIG_TEST_IMAGE) .; \ + ./runtests.sh "$(CONFIG_TEST_IMAGE)" ) diff --git a/test/config/reaper.json b/test/config/reaper.json new file mode 100644 index 0000000..829627d --- /dev/null +++ b/test/config/reaper.json @@ -0,0 +1,4 @@ +{ + "Pid": 0, + "Options": 0 +} diff --git a/test/Dockerfile b/test/docker-files/no-config/Dockerfile similarity index 100% rename from test/Dockerfile rename to test/docker-files/no-config/Dockerfile diff --git a/test/runtests.sh b/test/runtests.sh index 6de864a..64a8b51 100755 --- a/test/runtests.sh +++ b/test/runtests.sh @@ -4,6 +4,8 @@ readonly MAX_SLEEP_TIME=$((5 + 2)) readonly IMAGE="reaper/test" +logfile="/tmp/reaper_test.log" + function get_sleepers() { ps -ef -p $1 | grep sleep | grep -v grep @@ -32,14 +34,24 @@ function check_orphans() { function terminate_container() { + docker logs "$1" > "$logfile" + echo " - Container logs saved to $logfile" + echo " - Terminated container $(docker rm -f "$1")" } # End of function terminate_container. function run_tests() { - echo " - Starting docker container running image $IMAGE ..." - local elcid=$(docker run -dit $IMAGE) + local image=${1:-"${IMAGE}"} + + logfile="/tmp/$(echo ${image} | sed 's#/#_#g').log" + + echo " - Removing any existing log file $logfile ... " + rm -f "$logfile" + + echo " - Starting docker container running image ${image} ..." + local elcid=$(docker run -dit "${image}") echo " - Docker container name=$elcid" local pid1=$(docker inspect --format '{{.State.Pid}}' $elcid) diff --git a/test/testpid1.go b/test/testpid1.go index 07bf12a..e1f3de5 100644 --- a/test/testpid1.go +++ b/test/testpid1.go @@ -1,17 +1,53 @@ package main +import "encoding/json" import "fmt" import "os" import "os/signal" import "os/exec" import "path/filepath" import "syscall" +import "time" import reaper "github.com/ramr/go-reaper" +const NWORKERS = 3 +const REAPER_JSON_CONFIG = "/reaper/config/reaper.json" -const NWORKERS=3 +func sleeper_test(set_proc_attributes bool) { + fmt.Printf(" - Set process attributes: %+v\n", set_proc_attributes) + cmd := exec.Command("sleep", "1") + if set_proc_attributes { + cmd.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, + Pgid: 0, + } + } + + err := cmd.Start() + if err != nil { + fmt.Printf(" - Error starting sleep command: %s\n", err) + return + } + + fmt.Printf("Set proc attributes: %+v\n", set_proc_attributes) + + // Sleep for a wee bit longer to allow the reaper to reap the + // command on a slow system. + time.Sleep(4 * time.Second) + + err = cmd.Wait() + if err != nil { + if set_proc_attributes { + fmt.Printf(" - Error waiting for command: %s\n", + err) + } else { + fmt.Printf(" - Expected wait failure: %s\n", err) + } + } + +} /* End of function sleeper_test. */ func start_workers() { // Starts up workers - which in turn start up kids that get @@ -28,34 +64,58 @@ func start_workers() { fmt.Printf(" - Error getting script - %s\n", scriptFile) return } - + var args = fmt.Sprintf("%d", NWORKERS) var cmd = exec.Command(script, args) cmd.Start() fmt.Printf(" - Started worker: %s %s\n", script, args) -} /* End of function start_workers. */ +} /* End of function start_workers. */ +func main() { + sig := make(chan os.Signal, 1) + signal.Notify(sig, syscall.SIGUSR1) + + useConfig := false + config := reaper.Config{} + + configFile, err := os.Open(REAPER_JSON_CONFIG) + if err == nil { + decoder := json.NewDecoder(configFile) + err = decoder.Decode(&config) + if err == nil { + useConfig = true + } else { + fmt.Printf(" - Error: Invalid json config: %s", err) + fmt.Printf(" - Using defaults ... ") + } + } + /* Start the grim reaper ... */ + if useConfig { + go reaper.Start(config) -func main() { - sig := make(chan os.Signal, 1) - signal.Notify(sig, syscall.SIGUSR1) + /* Run the sleeper test setting the process attributes. */ + go sleeper_test(true) - /* Start the grim reaper ... */ - go reaper.Reap() + /* And run test without setting process attributes. */ + go sleeper_test(false) - /* Start the initial set of workers ... */ - start_workers() + } else { + go reaper.Reap() + } - for { - select { - case <-sig: - fmt.Println(" - Got SIGUSR1, starting more workers ...") + /* Start the initial set of workers ... */ start_workers() - } - } /* End of while doomsday ... */ + for { + select { + case <-sig: + fmt.Println(" - Got SIGUSR1, adding workers ...") + start_workers() + } + + } /* End of while doomsday ... */ -} /* End of function main. */ +} /* End of function main. */