Add pre-stop hooks for containers that expose ports

This pre-stop hook will simply wait for 5 seconds. In order to achieve this in the most general way, a configmap is mounted as a volume that has a very simply binary that sleeps for 5 seconds. It is done this way because this should work for any container images, including scratch. Wait is not applied for stateful containers, jobs, or containers with one replica. Signed-off-by: Donnie Adams <[email protected]>
acorn-io · Oct 26, 2023 · 8c30034 · 8c30034
1 parent 77f8432
commit 8c30034
Show file tree

Hide file tree

Showing 84 changed files with 2,135 additions and 100 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -4,6 +4,7 @@ FROM ghcr.io/acorn-io/images-mirror/tonistiigi/binfmt:qemu-v6.2.0 AS binfmt
 FROM ghcr.io/acorn-io/images-mirror/moby/buildkit:v0.11.6 AS buildkit
 FROM ghcr.io/acorn-io/images-mirror/registry:2.8.1 AS registry
 FROM ghcr.io/acorn-io/images-mirror/rancher/klipper-lb:v0.3.5 AS klipper-lb
+FROM docker.io/thedadams/sleep:five AS sleep
 
 FROM ghcr.io/acorn-io/images-mirror/golang:1.21-alpine AS helper
 WORKDIR /usr/src
@@ -20,7 +21,8 @@ RUN --mount=type=cache,target=/go/pkg --mount=type=cache,target=/root/.cache/go-
 FROM ghcr.io/acorn-io/images-mirror/golang:1.21 AS build
 COPY / /src
 WORKDIR /src
-RUN --mount=type=cache,target=/go/pkg --mount=type=cache,target=/root/.cache/go-build make build
+COPY --from=sleep /sleep /src/pkg/controller/appdefinition/embed/acorn-sleep
+RUN --mount=type=cache,target=/go/pkg --mount=type=cache,target=/root/.cache/go-build GO_TAGS=netgo,image make build
 
 FROM ghcr.io/acorn-io/images-mirror/nginx:1.23.2-alpine AS base
 RUN apk add --no-cache ca-certificates iptables ip6tables fuse3 git openssh pigz xz \

diff --git a/Makefile b/Makefile
@@ -1,5 +1,6 @@
+GO_TAGS ?= netgo
 build:
-	CGO_ENABLED=0 go build -o bin/acorn -tags netgo -ldflags "-s -w" .
+	CGO_ENABLED=0 go build -o bin/acorn -tags "${GO_TAGS}" -ldflags "-s -w" .
 
 tidy:
 	go mod tidy

diff --git a/pkg/controller/appdefinition/deploy.go b/pkg/controller/appdefinition/deploy.go
@@ -2,6 +2,7 @@ package appdefinition
 
 import (
 	"crypto/sha256"
+	_ "embed"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
@@ -205,7 +206,7 @@ func hasContextDir(container v1.Container) bool {
 	return false
 }
 
-func toContainers(app *v1.AppInstance, tag name.Reference, name string, container v1.Container, interpolator *secrets.Interpolator) ([]corev1.Container, []corev1.Container) {
+func toContainers(app *v1.AppInstance, tag name.Reference, name string, container v1.Container, interpolator *secrets.Interpolator, addWait bool) ([]corev1.Container, []corev1.Container) {
 	var (
 		containers     []corev1.Container
 		initContainers []corev1.Container
@@ -226,10 +227,10 @@ func toContainers(app *v1.AppInstance, tag name.Reference, name string, containe
 		})
 	}
 
-	newContainer := toContainer(app, tag, name, container, interpolator)
+	newContainer := toContainer(app, tag, name, container, interpolator, addWait && len(container.Ports) > 0)
 	containers = append(containers, newContainer)
 	for _, entry := range typed.Sorted(container.Sidecars) {
-		newContainer = toContainer(app, tag, entry.Key, entry.Value, interpolator)
+		newContainer = toContainer(app, tag, entry.Key, entry.Value, interpolator, addWait && len(entry.Value.Ports) > 0)
 
 		if entry.Value.Init {
 			initContainers = append(initContainers, newContainer)
@@ -254,7 +255,7 @@ func sanitizeVolumeName(name string) string {
 	return name
 }
 
-func toMounts(app *v1.AppInstance, container v1.Container, interpolation *secrets.Interpolator) (result []corev1.VolumeMount) {
+func toMounts(app *v1.AppInstance, container v1.Container, interpolation *secrets.Interpolator, addWait bool) (result []corev1.VolumeMount) {
 	for _, entry := range typed.Sorted(container.Files) {
 		suffix := ""
 		if volume.NormalizeMode(entry.Value.Mode) != "" {
@@ -297,6 +298,13 @@ func toMounts(app *v1.AppInstance, container v1.Container, interpolation *secret
 			})
 		}
 	}
+
+	if addWait {
+		result = append(result, corev1.VolumeMount{
+			Name:      string(app.UID),
+			MountPath: "/usr/local/bin",
+		})
+	}
 	return
 }
 
@@ -420,7 +428,7 @@ func toProbe(container v1.Container, probeType v1.ProbeType) *corev1.Probe {
 	return nil
 }
 
-func toContainer(app *v1.AppInstance, tag name.Reference, containerName string, container v1.Container, interpolator *secrets.Interpolator) corev1.Container {
+func toContainer(app *v1.AppInstance, tag name.Reference, containerName string, container v1.Container, interpolator *secrets.Interpolator, addWait bool) corev1.Container {
 	containerObject := corev1.Container{
 		Name:           containerName,
 		Image:          images.ResolveTag(tag, container.Image),
@@ -432,13 +440,26 @@ func toContainer(app *v1.AppInstance, tag name.Reference, containerName string,
 		TTY:            container.Interactive,
 		Stdin:          container.Interactive,
 		Ports:          toPorts(container),
-		VolumeMounts:   toMounts(app, container, interpolator),
+		VolumeMounts:   toMounts(app, container, interpolator, addWait),
 		LivenessProbe:  toProbe(container, v1.LivenessProbeType),
 		StartupProbe:   toProbe(container, v1.StartupProbeType),
 		ReadinessProbe: toProbe(container, v1.ReadinessProbeType),
 		Resources:      app.Status.Scheduling[containerName].Requirements,
 	}
 
+	if addWait {
+		// If a container exposes a port, then add a pre-stop lifecycle hook that sleeps for 5 seconds. This should allow the
+		// endpoints controller to remove the pods IP on termination and stop sending traffic to the container.
+		// Note that this requires the acorn-sleep binary to be in the container, via the volume mount.
+		containerObject.Lifecycle = &corev1.Lifecycle{
+			PreStop: &corev1.LifecycleHandler{
+				Exec: &corev1.ExecAction{
+					Command: []string{fmt.Sprintf("/usr/local/bin/%s", app.UID)},
+				},
+			},
+		}
+	}
+
 	return containerObject
 }
 
@@ -636,18 +657,19 @@ func getSecretAnnotations(req router.Request, appInstance *v1.AppInstance, conta
 func toDeployment(req router.Request, appInstance *v1.AppInstance, tag name.Reference, name string, container v1.Container, pullSecrets *PullSecrets, interpolator *secrets.Interpolator) (*appsv1.Deployment, error) {
 	var (
 		stateful = isStateful(appInstance, container)
+		addWait  = !stateful && len(acornSleepBinary) > 0 && z.Dereference(container.Scale) > 1 && !appInstance.Status.GetDevMode()
 	)
 
 	interpolator = interpolator.ForContainer(name)
 
-	containers, initContainers := toContainers(appInstance, tag, name, container, interpolator)
+	containers, initContainers := toContainers(appInstance, tag, name, container, interpolator, addWait)
 
 	secretAnnotations, err := getSecretAnnotations(req, appInstance, container, interpolator)
 	if err != nil {
 		return nil, err
 	}
 
-	volumes, err := toVolumes(appInstance, container, interpolator)
+	volumes, err := toVolumes(appInstance, container, interpolator, addWait)
 	if err != nil {
 		return nil, err
 	}
@@ -685,7 +707,7 @@ func toDeployment(req router.Request, appInstance *v1.AppInstance, tag name.Refe
 					Affinity:                      appInstance.Status.Scheduling[name].Affinity,
 					Tolerations:                   appInstance.Status.Scheduling[name].Tolerations,
 					PriorityClassName:             appInstance.Status.Scheduling[name].PriorityClassName,
-					TerminationGracePeriodSeconds: z.Pointer[int64](5),
+					TerminationGracePeriodSeconds: z.Pointer[int64](10),
 					ImagePullSecrets:              pullSecrets.ForContainer(name, append(containers, initContainers...)),
 					EnableServiceLinks:            new(bool),
 					Containers:                    containers,
@@ -733,6 +755,25 @@ func ToDeployments(req router.Request, appInstance *v1.AppInstance, tag name.Ref
 		if err != nil {
 			return nil, err
 		}
+
+		for _, v := range dep.Spec.Template.Spec.Volumes {
+			if v.Name == string(appInstance.UID) {
+				result = append(result, &corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      string(appInstance.UID),
+						Namespace: appInstance.Status.Namespace,
+						Labels: map[string]string{
+							labels.AcornManaged: "true",
+						},
+					},
+					BinaryData: map[string][]byte{
+						string(appInstance.UID): acornSleepBinary,
+					},
+				})
+				break
+			}
+		}
+
 		perms := v1.FindPermission(containerName, appInstance.Status.Permissions)
 		sa, err := toServiceAccount(req, dep.GetName(), dep.GetLabels(), dep.GetAnnotations(), appInstance, perms)
 		if err != nil {

diff --git a/pkg/controller/appdefinition/jobs.go b/pkg/controller/appdefinition/jobs.go
@@ -109,7 +109,7 @@ func toJob(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSec
 		return nil, nil
 	}
 
-	containers, initContainers := toContainers(appInstance, tag, name, container, interpolator)
+	containers, initContainers := toContainers(appInstance, tag, name, container, interpolator, false)
 
 	containers = append(containers, corev1.Container{
 		Name:            jobs.Helper,
@@ -123,7 +123,7 @@ func toJob(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSec
 		return nil, err
 	}
 
-	volumes, err := toVolumes(appInstance, container, interpolator)
+	volumes, err := toVolumes(appInstance, container, interpolator, false)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/controller/appdefinition/pre_stop_test.go b/pkg/controller/appdefinition/pre_stop_test.go
@@ -0,0 +1,22 @@
+package appdefinition
+
+import (
+	"testing"
+
+	"github.com/acorn-io/baaah/pkg/router/tester"
+	"github.com/acorn-io/runtime/pkg/scheme"
+)
+
+func TestPreStop(t *testing.T) {
+	acornSleepBinary = []byte("acorn-sleep")
+	t.Cleanup(func() {
+		acornSleepBinary = nil
+	})
+
+	tester.DefaultTest(t, scheme.Scheme, "testdata/deployspec/pre-stop/basic", DeploySpec)
+	tester.DefaultTest(t, scheme.Scheme, "testdata/deployspec/pre-stop/stateful", DeploySpec)
+	tester.DefaultTest(t, scheme.Scheme, "testdata/deployspec/pre-stop/job", DeploySpec)
+	tester.DefaultTest(t, scheme.Scheme, "testdata/deployspec/pre-stop/dev", DeploySpec)
+	tester.DefaultTest(t, scheme.Scheme, "testdata/deployspec/pre-stop/no-ports", DeploySpec)
+	tester.DefaultTest(t, scheme.Scheme, "testdata/deployspec/pre-stop/ports-only-sidecar", DeploySpec)
+}
diff --git a/pkg/controller/appdefinition/router.go b/pkg/controller/appdefinition/router.go
@@ -74,7 +74,7 @@ func toRouter(appInstance *v1.AppInstance, routerName string, router v1.Router)
 					Annotations: deploymentAnnotations,
 				},
 				Spec: corev1.PodSpec{
-					TerminationGracePeriodSeconds: z.Pointer[int64](5),
+					TerminationGracePeriodSeconds: z.Pointer[int64](10),
 					EnableServiceLinks:            new(bool),
 					Containers: []corev1.Container{
 						{
@@ -110,6 +110,17 @@ func toRouter(appInstance *v1.AppInstance, routerName string, router v1.Router)
 									},
 								},
 							},
+							Lifecycle: &corev1.Lifecycle{
+								PreStop: &corev1.LifecycleHandler{
+									Exec: &corev1.ExecAction{
+										Command: []string{
+											"/bin/sh",
+											"-c",
+											"sleep 5 && /usr/sbin/nginx -s quit",
+										},
+									},
+								},
+							},
 						},
 					},
 					Volumes: []corev1.Volume{

diff --git a/pkg/controller/appdefinition/sleep_dev.go b/pkg/controller/appdefinition/sleep_dev.go
@@ -0,0 +1,6 @@
+//go:build !image
+
+package appdefinition
+
+// If the binary is being built on its own and not part of an image, then don't worry about this binary.
+var acornSleepBinary []byte
diff --git a/pkg/controller/appdefinition/sleep_image.go b/pkg/controller/appdefinition/sleep_image.go
@@ -0,0 +1,8 @@
+//go:build image
+
+package appdefinition
+
+import _ "embed"
+
+//go:embed embed/acorn-sleep
+var acornSleepBinary []byte
diff --git a/...roller/appdefinition/testdata/computeclass/all-set-overwrite-computeclass/expected.golden b/...roller/appdefinition/testdata/computeclass/all-set-overwrite-computeclass/expected.golden
@@ -95,7 +95,7 @@ spec:
       imagePullSecrets:
       - name: oneimage-pull-1234567890ab
       serviceAccountName: oneimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---

diff --git a/pkg/controller/appdefinition/testdata/computeclass/all-set/expected.golden b/pkg/controller/appdefinition/testdata/computeclass/all-set/expected.golden
@@ -95,7 +95,7 @@ spec:
       imagePullSecrets:
       - name: oneimage-pull-1234567890ab
       serviceAccountName: oneimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---

diff --git a/pkg/controller/appdefinition/testdata/computeclass/container/expected.golden b/pkg/controller/appdefinition/testdata/computeclass/container/expected.golden
@@ -95,7 +95,7 @@ spec:
       imagePullSecrets:
       - name: oneimage-pull-1234567890ab
       serviceAccountName: oneimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---

diff --git a/pkg/controller/appdefinition/testdata/computeclass/different-computeclass/expected.golden b/pkg/controller/appdefinition/testdata/computeclass/different-computeclass/expected.golden
@@ -81,7 +81,7 @@ spec:
       imagePullSecrets:
       - name: oneimage-pull-1234567890ab
       serviceAccountName: oneimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---
@@ -198,7 +198,7 @@ spec:
       imagePullSecrets:
       - name: twoimage-pull-1234567890ab
       serviceAccountName: twoimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---

diff --git a/...ller/appdefinition/testdata/computeclass/overwrite-acornfile-computeclass/expected.golden b/...ller/appdefinition/testdata/computeclass/overwrite-acornfile-computeclass/expected.golden
@@ -95,7 +95,7 @@ spec:
       imagePullSecrets:
       - name: oneimage-pull-1234567890ab
       serviceAccountName: oneimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---

diff --git a/pkg/controller/appdefinition/testdata/computeclass/two-containers/expected.golden b/pkg/controller/appdefinition/testdata/computeclass/two-containers/expected.golden
@@ -81,7 +81,7 @@ spec:
       imagePullSecrets:
       - name: oneimage-pull-1234567890ab
       serviceAccountName: oneimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---
@@ -184,7 +184,7 @@ spec:
       imagePullSecrets:
       - name: twoimage-pull-1234567890ab
       serviceAccountName: twoimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---

diff --git a/...ontroller/appdefinition/testdata/computeclass/with-acornfile-computeclass/expected.golden b/...ontroller/appdefinition/testdata/computeclass/with-acornfile-computeclass/expected.golden
@@ -95,7 +95,7 @@ spec:
       imagePullSecrets:
       - name: oneimage-pull-1234567890ab
       serviceAccountName: oneimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---

diff --git a/pkg/controller/appdefinition/testdata/depends-ready/expected.golden b/pkg/controller/appdefinition/testdata/depends-ready/expected.golden
@@ -61,7 +61,7 @@ spec:
       imagePullSecrets:
       - name: container-name-pull-1234567890ab
       serviceAccountName: container-name
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---
@@ -158,7 +158,7 @@ spec:
       imagePullSecrets:
       - name: web-pull-1234567890ab
       serviceAccountName: web
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---

diff --git a/pkg/controller/appdefinition/testdata/depends/expected.golden b/pkg/controller/appdefinition/testdata/depends/expected.golden
@@ -61,7 +61,7 @@ spec:
       imagePullSecrets:
       - name: container-name-pull-1234567890ab
       serviceAccountName: container-name
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---
@@ -162,7 +162,7 @@ spec:
       imagePullSecrets:
       - name: web-pull-1234567890ab
       serviceAccountName: web
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---

diff --git a/pkg/controller/appdefinition/testdata/deployspec/basic/expected.golden b/pkg/controller/appdefinition/testdata/deployspec/basic/expected.golden
@@ -61,7 +61,7 @@ spec:
       imagePullSecrets:
       - name: buildimage-pull-1234567890ab
       serviceAccountName: buildimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---
@@ -173,7 +173,7 @@ spec:
       imagePullSecrets:
       - name: oneimage-pull-1234567890ab
       serviceAccountName: oneimage
-      terminationGracePeriodSeconds: 5
+      terminationGracePeriodSeconds: 10
 status: {}
 
 ---