Handle unexpected shim kill events

adityaramani · dcantah · commit 3d27bc738a6e · 2023-09-29T19:15:07.000-07:00
When a shim process is unexpectedly killed in a way that was not initiated through containerd - containerd reports the pod as not ready but the containers as running. This results in kubelet repeatedly sending container kill requests that fail since containerd cannot connect to the shim. Changes: - In the container exit handler, treat `err: Unavailable` as if the container has already exited out - When attempting to get a connection to the shim, if the controller isn't available assume that the shim has been killed (needs to be done since we have a separate exit handler that cleans up the reference to the shim controller - before kubelet has the chance to call StopPodSandbox) Signed-off-by: Aditya Ramani <a_ramani@apple.com> (cherry picked from commit 729c97c) Signed-off-by: Danny Canter <danny@dcantah.dev>
diff --git a/pkg/cri/sbserver/events.go b/pkg/cri/sbserver/events.go
@@ -393,7 +393,7 @@ func handleContainerExit(ctx context.Context, e *eventtypes.TaskExit, cntr conta
 		},
 	)
 	if err != nil {
-		if !errdefs.IsNotFound(err) {
+		if !errdefs.IsNotFound(err) && !errdefs.IsUnavailable(err) {
 			return fmt.Errorf("failed to load task for container: %w", err)
 		}
 	} else {
diff --git a/plugins/sandbox/controller.go b/plugins/sandbox/controller.go
@@ -260,6 +260,12 @@ func (c *controllerLocal) Wait(ctx context.Context, sandboxID string) (sandbox.E
 
 func (c *controllerLocal) Status(ctx context.Context, sandboxID string, verbose bool) (sandbox.ControllerStatus, error) {
 	svc, err := c.getSandbox(ctx, sandboxID)
+	if errdefs.IsNotFound(err) {
+		return sandbox.ControllerStatus{
+			SandboxID: sandboxID,
+			ExitedAt:  time.Now(),
+		}, nil
+	}
 	if err != nil {
 		return sandbox.ControllerStatus{}, err
 	}
@@ -286,7 +292,7 @@ func (c *controllerLocal) Status(ctx context.Context, sandboxID string, verbose
 func (c *controllerLocal) getSandbox(ctx context.Context, id string) (runtimeAPI.TTRPCSandboxService, error) {
 	shim, err := c.shims.Get(ctx, id)
 	if err != nil {
-		return nil, errdefs.ErrNotFound
+		return nil, err
 	}
 
 	return sandbox.NewClient(shim.Client())
diff --git a/services/sandbox/controller_service.go b/services/sandbox/controller_service.go
@@ -144,17 +144,21 @@ func (s *controllerService) Status(ctx context.Context, req *api.ControllerStatu
 	if err != nil {
 		return &api.ControllerStatusResponse{}, errdefs.ToGRPC(err)
 	}
+	extra := &anypb.Any{}
+	if cstatus.Extra != nil {
+		extra = &anypb.Any{
+			TypeUrl: cstatus.Extra.GetTypeUrl(),
+			Value:   cstatus.Extra.GetValue(),
+		}
+	}
 	return &api.ControllerStatusResponse{
 		SandboxID: cstatus.SandboxID,
 		Pid:       cstatus.Pid,
 		State:     cstatus.State,
 		Info:      cstatus.Info,
 		CreatedAt: protobuf.ToTimestamp(cstatus.CreatedAt),
 		ExitedAt:  protobuf.ToTimestamp(cstatus.ExitedAt),
-		Extra: &anypb.Any{
-			TypeUrl: cstatus.Extra.GetTypeUrl(),
-			Value:   cstatus.Extra.GetValue(),
-		},
+		Extra:     extra,
 	}, nil
 }
 
diff --git a/services/tasks/local.go b/services/tasks/local.go
@@ -339,7 +339,7 @@ func getProcessState(ctx context.Context, p runtime.Process) (*task.Process, err
 
 	state, err := p.State(ctx)
 	if err != nil {
-		if errdefs.IsNotFound(err) {
+		if errdefs.IsNotFound(err) || errdefs.IsUnavailable(err) {
 			return nil, err
 		}
 		log.G(ctx).WithError(err).Errorf("get state for %s", p.ID())

Original file line number	Diff line number	Diff line change
`@@ -393,7 +393,7 @@ func handleContainerExit(ctx context.Context, e *eventtypes.TaskExit, cntr conta`
`393`	`393`	`},`
`394`	`394`	`)`
`395`	`395`	`if err != nil {`
`396`		`- if !errdefs.IsNotFound(err) {`
	`396`	`+ if !errdefs.IsNotFound(err) && !errdefs.IsUnavailable(err) {`
`397`	`397`	`return fmt.Errorf("failed to load task for container: %w", err)`
`398`	`398`	`}`
`399`	`399`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -339,7 +339,7 @@ func getProcessState(ctx context.Context, p runtime.Process) (*task.Process, err`
`339`	`339`
`340`	`340`	`state, err := p.State(ctx)`
`341`	`341`	`if err != nil {`
`342`		`- if errdefs.IsNotFound(err) {`
	`342`	`+ if errdefs.IsNotFound(err) \|\| errdefs.IsUnavailable(err) {`
`343`	`343`	`return nil, err`
`344`	`344`	`}`
`345`	`345`	`log.G(ctx).WithError(err).Errorf("get state for %s", p.ID())`