Skip to content

Commit

Permalink
Kill exec PIDs after main container exited
Browse files Browse the repository at this point in the history
Before applying this patch we killed the exec PIDs right away on
container stop which leads into the failing e2e test:

```
[sig-node] [NodeFeature:SidecarContainers] Containers Lifecycle should terminate sidecars simultaneously if prestop doesn't exit
```

This regression is now fixed by killing the exec PIDs after the main
container as well as in the same thread.

Fixes kubernetes/kubernetes#124743
Follow-up on cri-o#7937

Needs a cherry-pick since the enhancement got already backported into
supported release branches.

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
  • Loading branch information
saschagrunert authored and openshift-cherrypick-robot committed May 15, 2024
1 parent 9b807d6 commit 84fc117
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions internal/oci/runtime_oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,6 @@ func (r *runtimeOCI) StopLoopForContainer(c *Container, bm kwait.BackoffManager)

startTime := time.Now()

go c.KillExecPIDs()

// Allow for SIGINT to correctly interrupt the stop loop, especially
// when CRI-O is run directly in the foreground in the terminal.
ctx, stop := signal.NotifyContext(ctx, os.Interrupt)
Expand Down Expand Up @@ -935,6 +933,10 @@ func (r *runtimeOCI) StopLoopForContainer(c *Container, bm kwait.BackoffManager)
}
}, bm, true, ctx.Done())

// Kill the exec PIDs after the main container to avoid pod lifecycle regressions:
// Ref: https://github.com/kubernetes/kubernetes/issues/124743
c.KillExecPIDs()

c.state.Finished = time.Now()
c.opLock.Unlock()
c.SetAsDoneStopping()
Expand Down

0 comments on commit 84fc117

Please sign in to comment.