Merge pull request #13 from FortnoxAB/reapzombies

Only reap zombies after each exec.

docker-compose.yml

@@ -59,11 +59,12 @@ services:
 
   agent-1:
     image: renovate/renovate:39.21.1-full
-    command:
+    entrypoint:
       - /renovator/renovator
       - --loglevel=debug
       - agent
       - --redis-url=redis://redis:6379/0
+      # - --max-process-count=5
     depends_on:
       init-agent-1:
         condition: service_completed_successfully

pkg/agent/agent.go

@@ -2,13 +2,9 @@ package agent
 
 import (
 	"context"
-	"errors"
 	"fmt"
-	"os"
-	"os/signal"
 	"strings"
 	"sync"
-	"syscall"
 	"time"
 
 	"github.com/fortnoxab/renovator/pkg/command"
@@ -52,7 +48,6 @@ func NewAgentFromContext(cCtx *cli.Context) (*Agent, error) {
 }
 
 func (a *Agent) Run(ctx context.Context) {
-	ZombieReaper()
 	reposToProcess := make(chan string)
 
 	wg := &sync.WaitGroup{}
@@ -108,26 +103,3 @@ func (a *Agent) Run(ctx context.Context) {
 	}
 	wg.Wait()
 }
-
-func ZombieReaper() {
-	signals := make(chan os.Signal, 1)
-	signal.Notify(signals, syscall.SIGCHLD)
-
-	go func() {
-		for range signals {
-			for {
-				var wstatus syscall.WaitStatus
-				time.Sleep(1 * time.Second)
-				pid, err := syscall.Wait4(-1, &wstatus, 0, nil)
-				if errors.Is(err, syscall.ECHILD) {
-					break
-				}
-				if err != nil {
-					logrus.Errorf("error waiting for child %d: %s", pid, err)
-					continue
-				}
-				logrus.Debugf("reaped zombie %d %d", pid, wstatus)
-			}
-		}
-	}()
-}

pkg/command/run.go

@@ -1,8 +1,13 @@
 package command
 
 import (
+	"errors"
 	"os"
 	"os/exec"
+	"strings"
+	"syscall"
+
+	"github.com/sirupsen/logrus"
 )
 
 type Commander interface {
@@ -24,6 +29,34 @@ func (e *Exec) RunWithEnv(env []string, head string, parts ...string) (err error
 	cmd.Env = os.Environ()
 	cmd.Env = append(cmd.Env, env...)
 
-	err = cmd.Run()
-	return
+	err = cmd.Start()
+	if err != nil {
+		return err
+	}
+
+	logrus.Debugf("started pid %d, %s", cmd.Process.Pid, head+" "+strings.Join(parts, " "))
+
+	err = cmd.Wait()
+
+	e.reapChildren()
+	return err
+}
+
+func (e *Exec) reapChildren() {
+	for {
+		var wstatus syscall.WaitStatus
+		pid, err := syscall.Wait4(-1, &wstatus, syscall.WNOHANG, nil)
+		if errors.Is(err, syscall.ECHILD) {
+			return
+		}
+		if err != nil {
+			logrus.Errorf("error waiting for child %d: %s", pid, err)
+			continue
+		}
+		if pid == 0 && wstatus == 0 {
+			logrus.Debug("found no pid")
+			return
+		}
+		logrus.Debugf("reaped zombie %d %d", pid, wstatus)
+	}
 }

pkg/master/master.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"sync"
 
-	"github.com/fortnoxab/renovator/pkg/agent"
 	"github.com/fortnoxab/renovator/pkg/command"
 	"github.com/fortnoxab/renovator/pkg/kafka"
 	"github.com/fortnoxab/renovator/pkg/leaderelect"
@@ -67,7 +66,6 @@ func (m *Master) Run(ctx context.Context) error {
 	if m.CronSchedule == nil {
 		return doRun(ctx, m.Candidate, m.RedisClient, m.Renovator, m.LeaderElect)
 	}
-	agent.ZombieReaper()
 
 	if m.RunFirstTime {
 		logrus.Info("running due to --run-first-time")