Skip to content

Commit

Permalink
refactor: cri exec
Browse files Browse the repository at this point in the history
  • Loading branch information
xzchaoo committed Mar 28, 2024
1 parent f65f217 commit 4ccc292
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 21 deletions.
5 changes: 3 additions & 2 deletions pkg/cri/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ type (
WorkingDir string `json:"workingDir"`
Input io.Reader
// User is the user passed to docker exec, defaults to 'root'
User string
FixOut bool
User string
FixOut bool
NoWrapCmdWithTimeout bool
}
)

Expand Down
3 changes: 2 additions & 1 deletion pkg/cri/criutils/zombies.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import (

func CountZombies(i cri.Interface, ctx context.Context, c *cri.Container) (int, error) {
r, err := i.Exec(ctx, c, cri.ExecRequest{
Cmd: []string{core.HelperToolPath, "countZombies"},
Cmd: []string{core.HelperToolPath, "countZombies"},
NoWrapCmdWithTimeout: true,
})
if err != nil {
return 0, err
Expand Down
11 changes: 8 additions & 3 deletions pkg/cri/impl/default_cri.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,11 @@ func (e *defaultCri) Exec(ctx context.Context, c *cri.Container, req cri.ExecReq
return invalidResult, errContainerIsNil
}

if !req.NoWrapCmdWithTimeout {
req.Cmd = wrapTimeout(c, req.Cmd)
}
req.Env = wrapEnv(req.Env)

begin := time.Now()
defer func() {
cost := time.Now().Sub(begin)
Expand Down Expand Up @@ -635,11 +640,11 @@ func (e *defaultCri) buildCriContainer(criPod *cri.Pod, dc *cri.EngineDetailCont
alreadyExists := false
copyCount := 0

if err := e.ensureHelperCopied(ctx, criContainer, core.HelperToolLocalPath, core.HelperToolPath); err == nil {
if err := e.ensureHelperCopied(ctx, criContainer, core.BusyboxLocalPath, core.BusyboxPath); err == nil {
copyCount++
}

if err := e.ensureHelperCopied(ctx, criContainer, core.BusyboxLocalPath, core.BusyboxPath); err == nil {
if err := e.ensureHelperCopied(ctx, criContainer, core.HelperToolLocalPath, core.HelperToolPath); err == nil {
copyCount++
}

Expand Down Expand Up @@ -1080,7 +1085,7 @@ func (e *defaultCri) updateZombieCheck(c *cri.Container) {
return
}

if _, err := e.Exec(ctx, c, cri.ExecRequest{Cmd: []string{core.BusyboxPath, "timeout", "1", "true"}}); err != nil {
if _, err := e.Exec(ctx, c, cri.ExecRequest{Cmd: []string{core.BusyboxPath, "timeout", "1", "true"}, NoWrapCmdWithTimeout: true}); err != nil {
logger.Errorz("check timeout error", zap.String("cid", c.ShortID()), zap.Error(err))
return
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/cri/impl/engine/containerd_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,13 +284,13 @@ func (e *ContainerdContainerEngine) Exec(ctx context.Context, c *cri.Container,

pspec := spec.Process
pspec.Terminal = false
pspec.Args = wrapTimeout(c, req.Cmd)
pspec.Args = req.Cmd
if req.WorkingDir != "" {
pspec.Cwd = req.WorkingDir
}

// Append user specified env
pspec.Env = wrapEnv(append(pspec.Env, req.Env...))
pspec.Env = append(pspec.Env, req.Env...)

task, err := container.Task(ctx, nil)
if err != nil {
Expand Down Expand Up @@ -425,13 +425,13 @@ func (e *ContainerdContainerEngine) ExecAsync(ctx context.Context, c *cri.Contai

pspec := spec.Process
pspec.Terminal = false
pspec.Args = wrapTimeout(c, req.Cmd)
pspec.Args = req.Cmd
if req.WorkingDir != "" {
pspec.Cwd = req.WorkingDir
}

// Append user specified env
pspec.Env = wrapEnv(append(pspec.Env, req.Env...))
pspec.Env = req.Env

task, err := container.Task(ctx, nil)
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions pkg/cri/impl/engine/docker_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ func (e *DockerContainerEngine) Exec(ctx context.Context, c *cri.Container, req
AttachStdout: true,
Detach: false,
DetachKeys: "",
Env: wrapEnv(req.Env),
Env: req.Env,
WorkingDir: req.WorkingDir,
Cmd: wrapTimeout(c, req.Cmd),
Cmd: req.Cmd,
})
if err != nil {
return invalidResult, err
Expand Down Expand Up @@ -240,9 +240,9 @@ func (e *DockerContainerEngine) ExecAsync(ctx context.Context, c *cri.Container,
AttachStdout: true,
Detach: false,
DetachKeys: "",
Env: wrapEnv(req.Env),
Env: req.Env,
WorkingDir: req.WorkingDir,
Cmd: wrapTimeout(c, hackedCmd),
Cmd: hackedCmd,
})
if err != nil {
return invalidResult, err
Expand Down
10 changes: 3 additions & 7 deletions pkg/cri/impl/engine/utils.go → pkg/cri/impl/wrap.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
/*
* Copyright 2022 Holoinsight Project Authors. Licensed under Apache-2.0.
*/

package engine
package impl

import (
"github.com/spf13/cast"
Expand All @@ -29,9 +25,9 @@ func wrapTimeout(c *cri.Container, cmd []string) []string {
// Different busybox versions have different timeout command formats
// In alpined based container, timeout will generate zombie processes
// timeout -s KILL <seconds> cmd...
// return append([]string{"timeout", "-s", "KILL", timeout}, cmd...)
// return append([]string{"timeout", "-s", "SIGKILL", timeout}, cmd...)
if c.Pid1CanRecycleZombieProcesses {
return append([]string{core.BusyboxPath, "timeout", "-s", "KILL", timeout}, cmd...)
return append([]string{core.BusyboxPath, "timeout", "-s", "SIGKILL", timeout}, cmd...)
}
return cmd
}
Expand Down

0 comments on commit 4ccc292

Please sign in to comment.