From 4ccc2929f159f554dbefbc44f7a9008afa57eb5e Mon Sep 17 00:00:00 2001 From: xzchaoo Date: Thu, 28 Mar 2024 17:12:48 +0800 Subject: [PATCH] refactor: cri exec --- pkg/cri/api.go | 5 +++-- pkg/cri/criutils/zombies.go | 3 ++- pkg/cri/impl/default_cri.go | 11 ++++++++--- pkg/cri/impl/engine/containerd_engine.go | 8 ++++---- pkg/cri/impl/engine/docker_engine.go | 8 ++++---- pkg/cri/impl/{engine/utils.go => wrap.go} | 10 +++------- 6 files changed, 24 insertions(+), 21 deletions(-) rename pkg/cri/impl/{engine/utils.go => wrap.go} (78%) diff --git a/pkg/cri/api.go b/pkg/cri/api.go index b1712cc..a0fc9b1 100644 --- a/pkg/cri/api.go +++ b/pkg/cri/api.go @@ -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 } ) diff --git a/pkg/cri/criutils/zombies.go b/pkg/cri/criutils/zombies.go index 922e267..f8eee97 100644 --- a/pkg/cri/criutils/zombies.go +++ b/pkg/cri/criutils/zombies.go @@ -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 diff --git a/pkg/cri/impl/default_cri.go b/pkg/cri/impl/default_cri.go index 6bd969a..6a829a7 100644 --- a/pkg/cri/impl/default_cri.go +++ b/pkg/cri/impl/default_cri.go @@ -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) @@ -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++ } @@ -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 } diff --git a/pkg/cri/impl/engine/containerd_engine.go b/pkg/cri/impl/engine/containerd_engine.go index 48ab122..7055a00 100644 --- a/pkg/cri/impl/engine/containerd_engine.go +++ b/pkg/cri/impl/engine/containerd_engine.go @@ -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 { @@ -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 { diff --git a/pkg/cri/impl/engine/docker_engine.go b/pkg/cri/impl/engine/docker_engine.go index 923b13c..be83a26 100644 --- a/pkg/cri/impl/engine/docker_engine.go +++ b/pkg/cri/impl/engine/docker_engine.go @@ -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 @@ -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 diff --git a/pkg/cri/impl/engine/utils.go b/pkg/cri/impl/wrap.go similarity index 78% rename from pkg/cri/impl/engine/utils.go rename to pkg/cri/impl/wrap.go index 3a4831d..ed59d17 100644 --- a/pkg/cri/impl/engine/utils.go +++ b/pkg/cri/impl/wrap.go @@ -1,8 +1,4 @@ -/* - * Copyright 2022 Holoinsight Project Authors. Licensed under Apache-2.0. - */ - -package engine +package impl import ( "github.com/spf13/cast" @@ -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 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 }