Skip to content

Commit 5cfbc2a

Browse files
kiashokKirtana Ashok
kiashok
and
Kirtana Ashok
authored
wcow: support graceful termination of servercore containers (#1416)
* This commit includes the changes to enable graceful termination of WCOW containers Signed-off-by: Kirtana Ashok <[email protected]> * Added regression tests for nanoserver and servercore base images Signed-off-by: Kirtana Ashok <[email protected]> * Worked on Kevin's review comments Signed-off-by: Kirtana Ashok <[email protected]> * Fixed lint failures Fixed lint errors caused by spelling mistakes in hcsdoc_wcow.go and stopcontainer_test.go Signed-off-by: Kirtana Ashok <[email protected]> * Addresses Kevin's review comments Signed-off-by: Kirtana Ashok <[email protected]> Signed-off-by: Kirtana Ashok <[email protected]> Signed-off-by: Kirtana Ashok <[email protected]> Co-authored-by: Kirtana Ashok <[email protected]>
1 parent 6547959 commit 5cfbc2a

9 files changed

Lines changed: 316 additions & 39 deletions

File tree

‎cmd/containerd-shim-runhcs-v1/exec_hcs.go‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,32 @@ func (he *hcsExec) Kill(ctx context.Context, signal uint32) error {
290290
}
291291
var delivered bool
292292
if supported && options != nil {
293-
delivered, err = he.p.Process.Signal(ctx, options)
293+
if he.isWCOW {
294+
// Servercore images block on signaling and wait until the target process
295+
// is terminated to return to the caller. This causes issues when graceful
296+
// termination of containers is requested (Bug36689012).
297+
// To fix this, we deliver the signal to the target process in a separate background
298+
// thread so that the caller can wait for the desired timeout before sending
299+
// a SIGKILL to the process.
300+
// TODO: We can get rid of these changes once the fix to support graceful termination is
301+
// made in windows.
302+
go func() {
303+
signalDelivered, deliveryErr := he.p.Process.Signal(ctx, options)
304+
305+
if deliveryErr != nil {
306+
if !hcs.IsAlreadyStopped(deliveryErr) {
307+
// Process is not already stopped and there was a signal delivery error to this process
308+
log.G(ctx).WithField("err", deliveryErr).Errorf("Error in delivering signal %d, to pid: %d", signal, he.pid)
309+
}
310+
}
311+
if !signalDelivered {
312+
log.G(ctx).Errorf("Error: NotFound; exec: '%s' in task: '%s' not found", he.id, he.tid)
313+
}
314+
}()
315+
delivered, err = true, nil
316+
} else {
317+
delivered, err = he.p.Process.Signal(ctx, options)
318+
}
294319
} else {
295320
// legacy path before signals support OR if WCOW with signals
296321
// support needs to issue a terminate.

‎internal/hcs/process.go‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,39 @@ func (process *Process) Kill(ctx context.Context) (bool, error) {
167167
return true, nil
168168
}
169169

170-
resultJSON, err := vmcompute.HcsTerminateProcess(ctx, process.handle)
170+
// HCS serializes the signals sent to a target pid per compute system handle.
171+
// To avoid SIGKILL being serialized behind other signals, we open a new compute
172+
// system handle to deliver the kill signal.
173+
// If the calls to opening a new compute system handle fail, we forcefully
174+
// terminate the container itself so that no container is left behind
175+
hcsSystem, err := OpenComputeSystem(ctx, process.system.id)
176+
if err != nil {
177+
// log error and force termination of container
178+
log.G(ctx).WithField("err", err).Error("OpenComputeSystem() call failed")
179+
err = process.system.Terminate(ctx)
180+
// if the Terminate() call itself ever failed, log and return error
181+
if err != nil {
182+
log.G(ctx).WithField("err", err).Error("Terminate() call failed")
183+
return false, err
184+
}
185+
process.system.Close()
186+
return true, nil
187+
}
188+
defer hcsSystem.Close()
189+
190+
newProcessHandle, err := hcsSystem.OpenProcess(ctx, process.Pid())
191+
if err != nil {
192+
// Return true only if the target process has either already
193+
// exited, or does not exist.
194+
if IsAlreadyStopped(err) {
195+
return true, nil
196+
} else {
197+
return false, err
198+
}
199+
}
200+
defer newProcessHandle.Close()
201+
202+
resultJSON, err := vmcompute.HcsTerminateProcess(ctx, newProcessHandle.handle)
171203
if err != nil {
172204
// We still need to check these two cases, as processes may still be killed by an
173205
// external actor (human operator, OOM, random script etc).
@@ -191,9 +223,9 @@ func (process *Process) Kill(ctx context.Context) (bool, error) {
191223
}
192224
}
193225
events := processHcsResult(ctx, resultJSON)
194-
delivered, err := process.processSignalResult(ctx, err)
226+
delivered, err := newProcessHandle.processSignalResult(ctx, err)
195227
if err != nil {
196-
err = makeProcessError(process, operation, err, events)
228+
err = makeProcessError(newProcessHandle, operation, err, events)
197229
}
198230

199231
process.killSignalDelivered = delivered

‎internal/hcsoci/hcsdoc_wcow.go‎

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@ import (
77
"context"
88
"errors"
99
"fmt"
10+
"math"
1011
"path/filepath"
1112
"regexp"
13+
"strconv"
1214
"strings"
1315

1416
specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -402,6 +404,31 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter
402404
dumpPath = specDumpPath
403405
}
404406

407+
// Servercore images block on signaling and wait until the target process
408+
// is terminated to return to its caller. By default, servercore waits for
409+
// 5 seconds (default value of 'WaitToKillServiceTimeout') before sending
410+
// a SIGKILL to terminate the process. This causes issues when graceful
411+
// termination of containers is requested (Bug36689012).
412+
// The regkey 'WaitToKillServiceTimeout' value is overridden here to help
413+
// honor graceful termination of containers by waiting for the requested
414+
// amount of time before stopping the container.
415+
// More details on the implementation of this fix can be found in the Kill()
416+
// function of exec_hcs.go
417+
418+
// 'WaitToKillServiceTimeout' reg key value is arbitrarily chosen and set to a
419+
// value that is long enough that no one will want to wait longer
420+
registryAdd := []hcsschema.RegistryValue{
421+
{
422+
Key: &hcsschema.RegistryKey{
423+
Hive: "System",
424+
Name: "ControlSet001\\Control",
425+
},
426+
Name: "WaitToKillServiceTimeout",
427+
StringValue: strconv.Itoa(math.MaxInt32),
428+
Type_: "String",
429+
},
430+
}
431+
405432
if dumpPath != "" {
406433
dumpType, err := parseDumpType(coi.Spec.Annotations)
407434
if err != nil {
@@ -410,30 +437,31 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter
410437

411438
// Setup WER registry keys for local process dump creation if specified.
412439
// https://docs.microsoft.com/en-us/windows/win32/wer/collecting-user-mode-dumps
413-
v2Container.RegistryChanges = &hcsschema.RegistryChanges{
414-
AddValues: []hcsschema.RegistryValue{
415-
{
416-
Key: &hcsschema.RegistryKey{
417-
Hive: "Software",
418-
Name: "Microsoft\\Windows\\Windows Error Reporting\\LocalDumps",
419-
},
420-
Name: "DumpFolder",
421-
StringValue: dumpPath,
422-
Type_: "String",
440+
registryAdd = append(registryAdd, []hcsschema.RegistryValue{
441+
{
442+
Key: &hcsschema.RegistryKey{
443+
Hive: "Software",
444+
Name: "Microsoft\\Windows\\Windows Error Reporting\\LocalDumps",
423445
},
424-
{
425-
Key: &hcsschema.RegistryKey{
426-
Hive: "Software",
427-
Name: "Microsoft\\Windows\\Windows Error Reporting\\LocalDumps",
428-
},
429-
Name: "DumpType",
430-
DWordValue: dumpType,
431-
Type_: "DWord",
446+
Name: "DumpFolder",
447+
StringValue: dumpPath,
448+
Type_: "String",
449+
},
450+
{
451+
Key: &hcsschema.RegistryKey{
452+
Hive: "Software",
453+
Name: "Microsoft\\Windows\\Windows Error Reporting\\LocalDumps",
432454
},
455+
Name: "DumpType",
456+
DWordValue: dumpType,
457+
Type_: "DWord",
433458
},
434-
}
459+
}...)
435460
}
436461

462+
v2Container.RegistryChanges = &hcsschema.RegistryChanges{
463+
AddValues: registryAdd,
464+
}
437465
return v1, v2Container, nil
438466
}
439467

‎test/cri-containerd/main_test.go‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,22 +48,24 @@ const (
4848
testVMServiceAddress = "C:\\ContainerPlat\\vmservice.sock"
4949
testVMServiceBinary = "C:\\Containerplat\\vmservice.exe"
5050

51-
lcowRuntimeHandler = "runhcs-lcow"
52-
imageLcowK8sPause = "mcr.microsoft.com/oss/kubernetes/pause:3.1"
53-
imageLcowAlpine = "mcr.microsoft.com/mirror/docker/library/alpine:3.16"
54-
imageLcowAlpineCoreDump = "cplatpublic.azurecr.io/stackoverflow-alpine:latest"
55-
imageLcowCosmos = "cosmosarno/spark-master:2.4.1_2019-04-18_8e864ce"
56-
imageLcowCustomUser = "cplatpublic.azurecr.io/linux_custom_user:latest"
57-
imageWindowsProcessDump = "cplatpublic.azurecr.io/crashdump:latest"
58-
imageWindowsArgsEscaped = "cplatpublic.azurecr.io/argsescaped:latest"
59-
imageWindowsTimezone = "cplatpublic.azurecr.io/timezone:latest"
60-
imageJobContainerHNS = "cplatpublic.azurecr.io/jobcontainer_hns:latest"
61-
imageJobContainerETW = "cplatpublic.azurecr.io/jobcontainer_etw:latest"
62-
imageJobContainerVHD = "cplatpublic.azurecr.io/jobcontainer_vhd:latest"
63-
imageJobContainerCmdline = "cplatpublic.azurecr.io/jobcontainer_cmdline:latest"
64-
imageJobContainerWorkDir = "cplatpublic.azurecr.io/jobcontainer_workdir:latest"
65-
alpineAspNet = "mcr.microsoft.com/dotnet/core/aspnet:3.1-alpine3.11"
66-
alpineAspnetUpgrade = "mcr.microsoft.com/dotnet/core/aspnet:3.1.2-alpine3.11"
51+
lcowRuntimeHandler = "runhcs-lcow"
52+
imageLcowK8sPause = "mcr.microsoft.com/oss/kubernetes/pause:3.1"
53+
imageLcowAlpine = "mcr.microsoft.com/mirror/docker/library/alpine:3.16"
54+
imageLcowAlpineCoreDump = "cplatpublic.azurecr.io/stackoverflow-alpine:latest"
55+
imageLcowCosmos = "cosmosarno/spark-master:2.4.1_2019-04-18_8e864ce"
56+
imageLcowCustomUser = "cplatpublic.azurecr.io/linux_custom_user:latest"
57+
imageWindowsProcessDump = "cplatpublic.azurecr.io/crashdump:latest"
58+
imageWindowsArgsEscaped = "cplatpublic.azurecr.io/argsescaped:latest"
59+
imageWindowsTimezone = "cplatpublic.azurecr.io/timezone:latest"
60+
imageJobContainerHNS = "cplatpublic.azurecr.io/jobcontainer_hns:latest"
61+
imageJobContainerETW = "cplatpublic.azurecr.io/jobcontainer_etw:latest"
62+
imageJobContainerVHD = "cplatpublic.azurecr.io/jobcontainer_vhd:latest"
63+
imageJobContainerCmdline = "cplatpublic.azurecr.io/jobcontainer_cmdline:latest"
64+
imageJobContainerWorkDir = "cplatpublic.azurecr.io/jobcontainer_workdir:latest"
65+
alpineAspNet = "mcr.microsoft.com/dotnet/core/aspnet:3.1-alpine3.11"
66+
alpineAspnetUpgrade = "mcr.microsoft.com/dotnet/core/aspnet:3.1.2-alpine3.11"
67+
gracefulTerminationServercore = "cplatpublic.azurecr.io/servercore-gracefultermination-repro:latest"
68+
gracefulTerminationNanoserver = "cplatpublic.azurecr.io/nanoserver-gracefultermination-repro:latest"
6769
// Default account name for use with GMSA related tests. This will not be
6870
// present/you will not have access to the account on your machine unless
6971
// your environment is configured properly.

‎test/cri-containerd/stopcontainer_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package cri_containerd
66
import (
77
"context"
88
"testing"
9+
"time"
910

1011
runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
1112
)
@@ -170,3 +171,78 @@ func Test_StopContainer_ReusePod_LCOW(t *testing.T) {
170171
containerID = createContainer(t, client, ctx, request)
171172
runContainerLifetime(t, client, ctx, containerID)
172173
}
174+
175+
// This test runs a container with an image that waits for sigterm and then
176+
// prints for loop counter down from 60 till the container is stopped with
177+
// a timeout of 15 seconds. This is done to mimic graceful termination
178+
// behavior and to ensure that the containers are killed only after 15 second
179+
// timeout specified via the stop container command.
180+
func Test_GracefulTermination(t *testing.T) {
181+
for name, tc := range map[string]struct {
182+
features []string
183+
runtimeHandler string
184+
image string
185+
}{
186+
"WCOWProcessNanoserver": {
187+
features: []string{featureWCOWProcess},
188+
runtimeHandler: wcowProcessRuntimeHandler,
189+
image: gracefulTerminationNanoserver,
190+
},
191+
"WCOWProcessServercore": {
192+
features: []string{featureWCOWProcess},
193+
runtimeHandler: wcowProcessRuntimeHandler,
194+
image: gracefulTerminationServercore,
195+
},
196+
"WCOWHypervisorNanoserver": {
197+
features: []string{featureWCOWHypervisor},
198+
runtimeHandler: wcowHypervisorRuntimeHandler,
199+
image: gracefulTerminationNanoserver,
200+
},
201+
"WCOWHypervisorServercore": {
202+
features: []string{featureWCOWHypervisor},
203+
runtimeHandler: wcowHypervisorRuntimeHandler,
204+
image: gracefulTerminationServercore,
205+
},
206+
} {
207+
t.Run(name, func(t *testing.T) {
208+
requireFeatures(t, tc.features...)
209+
pullRequiredImages(t, []string{tc.image})
210+
client := newTestRuntimeClient(t)
211+
ctx, cancel := context.WithCancel(context.Background())
212+
defer cancel()
213+
sandboxRequest := getRunPodSandboxRequest(t, tc.runtimeHandler)
214+
podID := runPodSandbox(t, client, ctx, sandboxRequest)
215+
defer removePodSandbox(t, client, ctx, podID)
216+
defer stopPodSandbox(t, client, ctx, podID)
217+
request := &runtime.CreateContainerRequest{
218+
PodSandboxId: podID,
219+
Config: &runtime.ContainerConfig{
220+
Metadata: &runtime.ContainerMetadata{},
221+
Image: &runtime.ImageSpec{
222+
Image: tc.image,
223+
},
224+
},
225+
SandboxConfig: sandboxRequest.Config,
226+
}
227+
containerID := createContainer(t, client, ctx, request)
228+
defer removeContainer(t, client, ctx, containerID)
229+
230+
startContainer(t, client, ctx, containerID)
231+
// Wait few seconds for the container to be completely initialized
232+
time.Sleep(5 * time.Second)
233+
assertContainerState(t, client, ctx, containerID, runtime.ContainerState_CONTAINER_RUNNING)
234+
235+
startTimeOfContainer := time.Now()
236+
// stop container with timeout of 15 seconds
237+
stopContainerWithTimeout(t, client, ctx, containerID, 15)
238+
assertContainerState(t, client, ctx, containerID, runtime.ContainerState_CONTAINER_EXITED)
239+
// get time elapsed before and after container stop command was issued
240+
elapsedTime := time.Since(startTimeOfContainer)
241+
// Ensure that the container has stopped after approx 15 seconds.
242+
// We are giving it a buffer of +/- 1 second
243+
if elapsedTime < 14*time.Second || elapsedTime > 16*time.Second {
244+
t.Fatalf("Container did not shutdown gracefully \n")
245+
}
246+
})
247+
}
248+
}

‎test/cri-containerd/test-images/nanoserver-gracefultermination-repro/latest/Dockerfile‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
FROM golang:latest as build
2+
ENV GOOS=windows
3+
ENV GOARCH=amd64
4+
ENV GO111MODULE=off
5+
WORKDIR /app
6+
COPY ./delayed-shutdown.go ./
7+
RUN go build -o delayed-shutdown.exe
8+
9+
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022
10+
WORKDIR /app
11+
COPY --from=build /app/delayed-shutdown.exe .
12+
ENTRYPOINT ["delayed-shutdown.exe"]

‎test/cri-containerd/test-images/nanoserver-gracefultermination-repro/latest/delayed-shutdown.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"os/signal"
7+
"syscall"
8+
"time"
9+
)
10+
11+
func main() {
12+
fmt.Println("Waiting for OS signal...")
13+
14+
signalChannel := make(chan os.Signal)
15+
wait := make(chan int, 1)
16+
17+
signal.Notify(signalChannel, syscall.SIGHUP)
18+
signal.Notify(signalChannel, syscall.SIGINT)
19+
signal.Notify(signalChannel, syscall.SIGTERM)
20+
21+
go func() {
22+
sig := <-signalChannel
23+
switch sig {
24+
case syscall.SIGHUP:
25+
fmt.Println("SIGHUP")
26+
wait <- 1
27+
case syscall.SIGTERM:
28+
fmt.Println("SIGTERM")
29+
wait <- 1
30+
case syscall.SIGINT:
31+
fmt.Println("SIGINT")
32+
wait <- 1
33+
}
34+
}()
35+
36+
<-wait
37+
38+
fmt.Println("Exiting in 60 seconds...")
39+
for i := 60; i > 0; i-- {
40+
fmt.Printf("%d\n", i)
41+
time.Sleep(1 * time.Second)
42+
}
43+
44+
fmt.Println("Goodbye")
45+
}

‎test/cri-containerd/test-images/servercore-gracefultermination-repro/latest/Dockerfile‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
FROM golang:latest as build
2+
ENV GOOS=windows
3+
ENV GOARCH=amd64
4+
ENV GO111MODULE=off
5+
WORKDIR /app
6+
COPY ./delayed-shutdown.go ./
7+
RUN go build -o delayed-shutdown.exe
8+
9+
FROM mcr.microsoft.com/windows/servercore:ltsc2022
10+
WORKDIR /app
11+
COPY --from=build /app/delayed-shutdown.exe .
12+
ENTRYPOINT ["delayed-shutdown.exe"]

0 commit comments

Comments
 (0)