Skip to content

Commit a4c67cc

Browse files
xw19xw19
committed
Fix: If cgroup manager is cgroupfs then allow conmon_cgroup to be empty
conmon_cgroup is deprecated and is replaced with RuntimeHandler monitor_cgroup and we prepopulate with default 'system.slice' Signed-off-by: Sourav Moitra <[email protected]>
1 parent b49a38f commit a4c67cc

File tree

2 files changed

+85
-48
lines changed

2 files changed

+85
-48
lines changed

‎pkg/config/config.go‎

Lines changed: 70 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717

1818
"github.com/BurntSushi/toml"
1919
"github.com/containers/common/pkg/hooks"
20-
conmonconfig "github.com/containers/conmon/runner/config"
2120
"github.com/containers/image/v5/pkg/sysregistriesv2"
2221
"github.com/containers/image/v5/types"
2322
"github.com/containers/storage"
@@ -929,46 +928,7 @@ func DefaultConfig() (*Config, error) {
929928
GRPCMaxSendMsgSize: defaultGRPCMaxMsgSize,
930929
GRPCMaxRecvMsgSize: defaultGRPCMaxMsgSize,
931930
},
932-
RuntimeConfig: RuntimeConfig{
933-
AllowedDevices: []string{"/dev/fuse", "/dev/net/tun"},
934-
DecryptionKeysPath: "/etc/crio/keys/",
935-
DefaultRuntime: DefaultRuntime,
936-
Runtimes: Runtimes{
937-
DefaultRuntime: defaultRuntimeHandler(),
938-
},
939-
SELinux: selinuxEnabled(),
940-
ApparmorProfile: apparmor.DefaultProfile,
941-
BlockIOConfigFile: DefaultBlockIOConfigFile,
942-
BlockIOReload: DefaultBlockIOReload,
943-
IrqBalanceConfigFile: DefaultIrqBalanceConfigFile,
944-
RdtConfigFile: rdt.DefaultRdtConfigFile,
945-
CgroupManagerName: cgroupManager.Name(),
946-
PidsLimit: DefaultPidsLimit,
947-
ContainerExitsDir: containerExitsDir,
948-
ContainerAttachSocketDir: conmonconfig.ContainerAttachSocketDir,
949-
MinimumMappableUID: -1,
950-
MinimumMappableGID: -1,
951-
LogSizeMax: DefaultLogSizeMax,
952-
CtrStopTimeout: defaultCtrStopTimeout,
953-
DefaultCapabilities: capabilities.Default(),
954-
LogLevel: "info",
955-
HooksDir: []string{hooks.DefaultDir},
956-
CDISpecDirs: cdi.DefaultSpecDirs,
957-
NamespacesDir: defaultNamespacesDir,
958-
DropInfraCtr: true,
959-
IrqBalanceConfigRestoreFile: DefaultIrqBalanceConfigRestoreFile,
960-
seccompConfig: seccomp.New(),
961-
apparmorConfig: apparmor.New(),
962-
blockioConfig: blockio.New(),
963-
cgroupManager: cgroupManager,
964-
deviceConfig: device.New(),
965-
namespaceManager: nsmgr.New(defaultNamespacesDir, ""),
966-
rdtConfig: rdt.New(),
967-
ulimitsConfig: ulimits.New(),
968-
HostNetworkDisableSELinux: true,
969-
DisableHostPortMapping: false,
970-
EnableCriuSupport: true,
971-
},
931+
RuntimeConfig: *DefaultRuntimeConfig(cgroupManager),
972932
ImageConfig: ImageConfig{
973933
DefaultTransport: "docker://",
974934
PauseImage: DefaultPauseImage,
@@ -995,6 +955,50 @@ func DefaultConfig() (*Config, error) {
995955
}, nil
996956
}
997957

958+
// DefaultRuntimeConfig returns the default Runtime configs.
959+
func DefaultRuntimeConfig(cgroupManager cgmgr.CgroupManager) *RuntimeConfig {
960+
return &RuntimeConfig{
961+
AllowedDevices: []string{"/dev/fuse", "/dev/net/tun"},
962+
DecryptionKeysPath: "/etc/crio/keys/",
963+
DefaultRuntime: DefaultRuntime,
964+
Runtimes: Runtimes{
965+
DefaultRuntime: defaultRuntimeHandler(cgroupManager.IsSystemd()),
966+
},
967+
SELinux: selinuxEnabled(),
968+
ApparmorProfile: apparmor.DefaultProfile,
969+
BlockIOConfigFile: DefaultBlockIOConfigFile,
970+
BlockIOReload: DefaultBlockIOReload,
971+
IrqBalanceConfigFile: DefaultIrqBalanceConfigFile,
972+
RdtConfigFile: rdt.DefaultRdtConfigFile,
973+
CgroupManagerName: cgroupManager.Name(),
974+
PidsLimit: DefaultPidsLimit,
975+
ContainerExitsDir: containerExitsDir,
976+
ContainerAttachSocketDir: ContainerAttachSocketDir,
977+
MinimumMappableUID: -1,
978+
MinimumMappableGID: -1,
979+
LogSizeMax: DefaultLogSizeMax,
980+
CtrStopTimeout: defaultCtrStopTimeout,
981+
DefaultCapabilities: capabilities.Default(),
982+
LogLevel: "info",
983+
HooksDir: []string{hooks.DefaultDir},
984+
CDISpecDirs: cdi.DefaultSpecDirs,
985+
NamespacesDir: defaultNamespacesDir,
986+
DropInfraCtr: true,
987+
IrqBalanceConfigRestoreFile: DefaultIrqBalanceConfigRestoreFile,
988+
seccompConfig: seccomp.New(),
989+
apparmorConfig: apparmor.New(),
990+
blockioConfig: blockio.New(),
991+
cgroupManager: cgroupManager,
992+
deviceConfig: device.New(),
993+
namespaceManager: nsmgr.New(defaultNamespacesDir, ""),
994+
rdtConfig: rdt.New(),
995+
ulimitsConfig: ulimits.New(),
996+
HostNetworkDisableSELinux: true,
997+
DisableHostPortMapping: false,
998+
EnableCriuSupport: true,
999+
}
1000+
}
1001+
9981002
// Validate is the main entry point for library configuration validation.
9991003
// The parameter `onExecution` specifies if the validation should include
10001004
// execution checks. It returns an `error` on validation failure, otherwise
@@ -1323,15 +1327,26 @@ func (c *RuntimeConfig) ValidateDefaultRuntime() error {
13231327
// The default config sets crun and its path in the runtimes map, so check for that
13241328
// first. If it does not exist then we add runc + its path to the runtimes map.
13251329
if _, ok := c.Runtimes[DefaultRuntime]; !ok {
1326-
c.Runtimes[DefaultRuntime] = defaultRuntimeHandler()
1330+
c.Runtimes[DefaultRuntime] = defaultRuntimeHandler(c.cgroupManager.IsSystemd())
13271331
}
13281332
// Set the DefaultRuntime to runc so we don't fail further along in the code
13291333
c.DefaultRuntime = DefaultRuntime
13301334

13311335
return nil
13321336
}
13331337

1334-
func defaultRuntimeHandler() *RuntimeHandler {
1338+
// getDefaultMonitorGroup checks which defaultmonitor group to use
1339+
// for cgroupfs it is empty.
1340+
func getDefaultMonitorGroup(isSystemd bool) string {
1341+
monitorGroup := ""
1342+
if isSystemd {
1343+
monitorGroup = defaultMonitorCgroup
1344+
}
1345+
1346+
return monitorGroup
1347+
}
1348+
1349+
func defaultRuntimeHandler(isSystemd bool) *RuntimeHandler {
13351350
return &RuntimeHandler{
13361351
RuntimeType: DefaultRuntimeType,
13371352
RuntimeRoot: DefaultRuntimeRoot,
@@ -1343,7 +1358,7 @@ func defaultRuntimeHandler() *RuntimeHandler {
13431358
"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
13441359
},
13451360
ContainerMinMemory: units.BytesSize(defaultContainerMinMemoryCrun),
1346-
MonitorCgroup: defaultMonitorCgroup,
1361+
MonitorCgroup: getDefaultMonitorGroup(isSystemd),
13471362
}
13481363
}
13491364

@@ -1477,19 +1492,26 @@ func (c *RuntimeConfig) TranslateMonitorFieldsForHandler(handler *RuntimeHandler
14771492
if len(c.ConmonEnv) != 0 {
14781493
handler.MonitorEnv = c.ConmonEnv
14791494
}
1480-
// If empty, assume default
1481-
if handler.MonitorCgroup == "" {
1495+
1496+
// If systemd and empty, assume default
1497+
if c.cgroupManager.IsSystemd() && handler.MonitorCgroup == "" {
14821498
handler.MonitorCgroup = defaultMonitorCgroup
14831499
}
14841500

14851501
if onExecution {
14861502
if err := c.ValidateConmonPath("conmon", handler); err != nil {
14871503
return err
14881504
}
1489-
1505+
// if cgroupManager is cgroupfs
14901506
if !c.cgroupManager.IsSystemd() {
1507+
// handler.MonitorCgroup having value "" is valid
1508+
// but the default value system.slice is not
1509+
if handler.MonitorCgroup == defaultMonitorCgroup {
1510+
handler.MonitorCgroup = ""
1511+
}
1512+
14911513
if handler.MonitorCgroup != utils.PodCgroupName && handler.MonitorCgroup != "" {
1492-
return errors.New("cgroupfs manager conmon cgroup should be 'pod' or empty")
1514+
return fmt.Errorf("cgroupfs manager conmon cgroup should be 'pod' or empty, but got: '%s'", handler.MonitorCgroup)
14931515
}
14941516

14951517
return nil

‎pkg/config/config_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
. "github.com/onsi/ginkgo/v2"
1212
. "github.com/onsi/gomega"
1313

14+
"github.com/cri-o/cri-o/internal/config/cgmgr"
1415
crioann "github.com/cri-o/cri-o/pkg/annotations"
1516
"github.com/cri-o/cri-o/pkg/config"
1617
"github.com/cri-o/cri-o/utils/cmdrunner"
@@ -535,6 +536,20 @@ var _ = t.Describe("Config", func() {
535536
// Then
536537
Expect(err).To(HaveOccurred())
537538
})
539+
It("should not fail on cgroupfs as cgroup manager and conmon_cgroup as empty", func() {
540+
handler := &config.RuntimeHandler{}
541+
542+
// Given
543+
cgm, _ := cgmgr.SetCgroupManager("cgroupfs") //nolint:errcheck
544+
runtimeConfig := *config.DefaultRuntimeConfig(cgm)
545+
546+
// When
547+
err := runtimeConfig.TranslateMonitorFieldsForHandler(handler, true)
548+
549+
// Then
550+
Expect(handler.MonitorCgroup).To(Equal(""))
551+
Expect(err).ToNot(HaveOccurred())
552+
})
538553

539554
It("should fail on invalid InfraCtrCPUSet", func() {
540555
// Given

0 commit comments

Comments
 (0)