Skip to content

Commit eb558b8

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request cri-o#6153 from haircommander/delegate-always
oci: specify --systemd-cgroup for all runtime commands
2 parents a0ffc30 + 7076f72 commit eb558b8

File tree

3 files changed

+44
-70
lines changed

3 files changed

+44
-70
lines changed

‎internal/oci/runtime_oci.go‎

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"os/exec"
1111
"path/filepath"
1212
"strconv"
13+
"strings"
1314
"syscall"
1415
"time"
1516

@@ -275,9 +276,7 @@ func (r *runtimeOCI) StartContainer(ctx context.Context, c *Container) error {
275276
return nil
276277
}
277278

278-
if _, err := utils.ExecCmd(
279-
r.handler.RuntimePath, rootFlag, r.root, "start", c.ID(),
280-
); err != nil {
279+
if _, err := r.runtimeCmd("start", c.ID()); err != nil {
281280
return err
282281
}
283282
c.state.Started = time.Now()
@@ -359,8 +358,8 @@ func (r *runtimeOCI) ExecContainer(ctx context.Context, c *Container, cmd []stri
359358
}
360359
defer os.RemoveAll(processFile)
361360

362-
args := []string{rootFlag, r.root, "exec"}
363-
args = append(args, "--process", processFile, c.ID())
361+
args := r.defaultRuntimeArgs()
362+
args = append(args, "exec", "--process", processFile, c.ID())
364363
execCmd := cmdrunner.Command(r.handler.RuntimePath, args...) // nolint: gosec
365364
if v, found := os.LookupEnv("XDG_RUNTIME_DIR"); found {
366365
execCmd.Env = append(execCmd.Env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", v))
@@ -494,6 +493,9 @@ func (r *runtimeOCI) ExecSyncContainer(ctx context.Context, c *Container, comman
494493
if timeout > 0 {
495494
args = append(args, "-T", fmt.Sprintf("%d", timeout))
496495
}
496+
if r.config.CgroupManager().IsSystemd() {
497+
args = append(args, "-s")
498+
}
497499

498500
processFile, err := prepareProcessExec(c, command, c.terminal)
499501
if err != nil {
@@ -839,9 +841,7 @@ func (r *runtimeOCI) StopContainer(ctx context.Context, c *Container, timeout in
839841
}
840842

841843
if timeout > 0 {
842-
if _, err := utils.ExecCmd(
843-
r.handler.RuntimePath, rootFlag, r.root, "kill", c.ID(), c.GetStopSignal(),
844-
); err != nil {
844+
if _, err := r.runtimeCmd("kill", c.ID(), c.GetStopSignal()); err != nil {
845845
checkProcessGone(c)
846846
}
847847
err := WaitContainerStop(ctx, c, time.Duration(timeout)*time.Second, true)
@@ -851,9 +851,7 @@ func (r *runtimeOCI) StopContainer(ctx context.Context, c *Container, timeout in
851851
log.Warnf(ctx, "Stopping container %v with stop signal timed out: %v", c.ID(), err)
852852
}
853853

854-
if _, err := utils.ExecCmd(
855-
r.handler.RuntimePath, rootFlag, r.root, "kill", c.ID(), "KILL",
856-
); err != nil {
854+
if _, err := r.runtimeCmd("kill", c.ID(), "KILL"); err != nil {
857855
checkProcessGone(c)
858856
}
859857

@@ -877,7 +875,7 @@ func (r *runtimeOCI) DeleteContainer(ctx context.Context, c *Container) error {
877875
return nil
878876
}
879877

880-
_, err := utils.ExecCmd(r.handler.RuntimePath, rootFlag, r.root, "delete", "--force", c.ID())
878+
_, err := r.runtimeCmd("delete", "--force", c.ID())
881879
return err
882880
}
883881

@@ -918,11 +916,7 @@ func (r *runtimeOCI) UpdateContainerStatus(ctx context.Context, c *Container) er
918916
}
919917

920918
stateCmd := func() (*ContainerState, bool, error) {
921-
cmd := cmdrunner.Command(r.handler.RuntimePath, rootFlag, r.root, "state", c.ID()) // nolint: gosec
922-
if v, found := os.LookupEnv("XDG_RUNTIME_DIR"); found {
923-
cmd.Env = append(cmd.Env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", v))
924-
}
925-
out, err := cmd.Output()
919+
out, err := r.runtimeCmd("state", c.ID())
926920
if err != nil {
927921
// there are many code paths that could lead to have a bad state in the
928922
// underlying runtime.
@@ -943,7 +937,7 @@ func (r *runtimeOCI) UpdateContainerStatus(ctx context.Context, c *Container) er
943937
return nil, true, nil
944938
}
945939
state := *c.state
946-
if err := json.NewDecoder(bytes.NewBuffer(out)).Decode(&state); err != nil {
940+
if err := json.NewDecoder(strings.NewReader(out)).Decode(&state); err != nil {
947941
return &state, false, fmt.Errorf("failed to decode container status for %s: %s", c.ID(), err)
948942
}
949943
return &state, false, nil
@@ -1024,7 +1018,7 @@ func (r *runtimeOCI) PauseContainer(ctx context.Context, c *Container) error {
10241018
return nil
10251019
}
10261020

1027-
_, err := utils.ExecCmd(r.handler.RuntimePath, rootFlag, r.root, "pause", c.ID())
1021+
_, err := r.runtimeCmd("pause", c.ID())
10281022
return err
10291023
}
10301024

@@ -1037,7 +1031,7 @@ func (r *runtimeOCI) UnpauseContainer(ctx context.Context, c *Container) error {
10371031
return nil
10381032
}
10391033

1040-
_, err := utils.ExecCmd(r.handler.RuntimePath, rootFlag, r.root, "resume", c.ID())
1034+
_, err := r.runtimeCmd("resume", c.ID())
10411035
return err
10421036
}
10431037

@@ -1066,17 +1060,13 @@ func (r *runtimeOCI) SignalContainer(ctx context.Context, c *Container, sig sysc
10661060

10671061
func (r *runtimeOCI) signalContainer(c *Container, sig syscall.Signal, all bool) error {
10681062
args := []string{
1069-
rootFlag,
1070-
r.root,
10711063
"kill",
10721064
}
10731065
if all {
10741066
args = append(args, "-a")
10751067
}
10761068
args = append(args, c.ID(), strconv.Itoa(int(sig)))
1077-
_, err := utils.ExecCmd(
1078-
r.handler.RuntimePath, args...,
1079-
)
1069+
_, err := r.runtimeCmd(args...)
10801070
return err
10811071
}
10821072

@@ -1357,3 +1347,32 @@ func prepareProcessExec(c *Container, cmd []string, tty bool) (processFile strin
13571347
func (c *Container) conmonPidFilePath() string {
13581348
return filepath.Join(c.bundlePath, "conmon-pidfile")
13591349
}
1350+
1351+
// runtimeCmd executes a command with args and returns its output as a string along
1352+
// with an error, if any
1353+
func (r *runtimeOCI) runtimeCmd(args ...string) (string, error) {
1354+
runtimeArgs := append(r.defaultRuntimeArgs(), args...)
1355+
cmd := cmdrunner.Command(r.handler.RuntimePath, runtimeArgs...)
1356+
var stdout bytes.Buffer
1357+
var stderr bytes.Buffer
1358+
cmd.Stdout = &stdout
1359+
cmd.Stderr = &stderr
1360+
if v, found := os.LookupEnv("XDG_RUNTIME_DIR"); found {
1361+
cmd.Env = append(cmd.Env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", v))
1362+
}
1363+
1364+
err := cmd.Run()
1365+
if err != nil {
1366+
return "", fmt.Errorf("`%v %v` failed: %v %v: %w", r.handler.RuntimePath, strings.Join(runtimeArgs, " "), stderr.String(), stdout.String(), err)
1367+
}
1368+
1369+
return stdout.String(), nil
1370+
}
1371+
1372+
func (r *runtimeOCI) defaultRuntimeArgs() []string {
1373+
args := []string{rootFlag, r.root}
1374+
if r.config.CgroupManager().IsSystemd() {
1375+
args = append(args, "--systemd-cgroup")
1376+
}
1377+
return args
1378+
}

‎utils/utils.go‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package utils
22

33
import (
4-
"bytes"
54
"crypto/rand"
65
"encoding/hex"
76
"errors"
@@ -11,12 +10,10 @@ import (
1110
"path/filepath"
1211
"runtime"
1312
"strconv"
14-
"strings"
1513
"time"
1614

1715
"github.com/containers/podman/v4/pkg/lookup"
1816
"github.com/cri-o/cri-o/internal/dbusmgr"
19-
"github.com/cri-o/cri-o/utils/cmdrunner"
2017
securejoin "github.com/cyphar/filepath-securejoin"
2118
"github.com/opencontainers/runc/libcontainer/user"
2219
"github.com/sirupsen/logrus"
@@ -28,26 +25,6 @@ import (
2825
"github.com/godbus/dbus/v5"
2926
)
3027

31-
// ExecCmd executes a command with args and returns its output as a string along
32-
// with an error, if any
33-
func ExecCmd(name string, args ...string) (string, error) {
34-
cmd := cmdrunner.Command(name, args...)
35-
var stdout bytes.Buffer
36-
var stderr bytes.Buffer
37-
cmd.Stdout = &stdout
38-
cmd.Stderr = &stderr
39-
if v, found := os.LookupEnv("XDG_RUNTIME_DIR"); found {
40-
cmd.Env = append(cmd.Env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", v))
41-
}
42-
43-
err := cmd.Run()
44-
if err != nil {
45-
return "", fmt.Errorf("`%v %v` failed: %v %v: %w", name, strings.Join(args, " "), stderr.String(), stdout.String(), err)
46-
}
47-
48-
return stdout.String(), nil
49-
}
50-
5128
// StatusToExitCode converts wait status code to an exit code
5229
func StatusToExitCode(status int) int {
5330
return ((status) & 0xff00) >> 8

‎utils/utils_test.go‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,28 +25,6 @@ func (m *errorReaderWriter) Read(p []byte) (int, error) {
2525

2626
// The actual test suite
2727
var _ = t.Describe("Utils", func() {
28-
t.Describe("ExecCmd", func() {
29-
It("should succeed", func() {
30-
// Given
31-
// When
32-
res, err := utils.ExecCmd("ls")
33-
34-
// Then
35-
Expect(err).To(BeNil())
36-
Expect(res).NotTo(BeEmpty())
37-
})
38-
39-
It("should fail on wrong command", func() {
40-
// Given
41-
// When
42-
res, err := utils.ExecCmd("not-existing")
43-
44-
// Then
45-
Expect(err).NotTo(BeNil())
46-
Expect(res).To(BeEmpty())
47-
})
48-
})
49-
5028
t.Describe("StatusToExitCode", func() {
5129
It("should succeed", func() {
5230
// Given

0 commit comments

Comments
 (0)