Skip to content

Commit a187c84

Browse files
author
Mrunal Patel
authored
Merge pull request #3626 from thaJeztah/more_idiomatic
libcontainer/cgroups: return concrete types
2 parents 56b01e7 + 04389ae commit a187c84

File tree

6 files changed

+74
-75
lines changed

6 files changed

+74
-75
lines changed

libcontainer/cgroups/fs/fs.go

+16-16
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ type subsystem interface {
5353
Set(path string, r *configs.Resources) error
5454
}
5555

56-
type manager struct {
56+
type Manager struct {
5757
mu sync.Mutex
5858
cgroups *configs.Cgroup
5959
paths map[string]string
6060
}
6161

62-
func NewManager(cg *configs.Cgroup, paths map[string]string) (cgroups.Manager, error) {
62+
func NewManager(cg *configs.Cgroup, paths map[string]string) (*Manager, error) {
6363
// Some v1 controllers (cpu, cpuset, and devices) expect
6464
// cgroups.Resources to not be nil in Apply.
6565
if cg.Resources == nil {
@@ -77,7 +77,7 @@ func NewManager(cg *configs.Cgroup, paths map[string]string) (cgroups.Manager, e
7777
}
7878
}
7979

80-
return &manager{
80+
return &Manager{
8181
cgroups: cg,
8282
paths: paths,
8383
}, nil
@@ -104,7 +104,7 @@ func isIgnorableError(rootless bool, err error) bool {
104104
return false
105105
}
106106

107-
func (m *manager) Apply(pid int) (err error) {
107+
func (m *Manager) Apply(pid int) (err error) {
108108
m.mu.Lock()
109109
defer m.mu.Unlock()
110110

@@ -138,19 +138,19 @@ func (m *manager) Apply(pid int) (err error) {
138138
return nil
139139
}
140140

141-
func (m *manager) Destroy() error {
141+
func (m *Manager) Destroy() error {
142142
m.mu.Lock()
143143
defer m.mu.Unlock()
144144
return cgroups.RemovePaths(m.paths)
145145
}
146146

147-
func (m *manager) Path(subsys string) string {
147+
func (m *Manager) Path(subsys string) string {
148148
m.mu.Lock()
149149
defer m.mu.Unlock()
150150
return m.paths[subsys]
151151
}
152152

153-
func (m *manager) GetStats() (*cgroups.Stats, error) {
153+
func (m *Manager) GetStats() (*cgroups.Stats, error) {
154154
m.mu.Lock()
155155
defer m.mu.Unlock()
156156
stats := cgroups.NewStats()
@@ -166,7 +166,7 @@ func (m *manager) GetStats() (*cgroups.Stats, error) {
166166
return stats, nil
167167
}
168168

169-
func (m *manager) Set(r *configs.Resources) error {
169+
func (m *Manager) Set(r *configs.Resources) error {
170170
if r == nil {
171171
return nil
172172
}
@@ -201,7 +201,7 @@ func (m *manager) Set(r *configs.Resources) error {
201201

202202
// Freeze toggles the container's freezer cgroup depending on the state
203203
// provided
204-
func (m *manager) Freeze(state configs.FreezerState) error {
204+
func (m *Manager) Freeze(state configs.FreezerState) error {
205205
path := m.Path("freezer")
206206
if path == "" {
207207
return errors.New("cannot toggle freezer: cgroups not configured for container")
@@ -217,25 +217,25 @@ func (m *manager) Freeze(state configs.FreezerState) error {
217217
return nil
218218
}
219219

220-
func (m *manager) GetPids() ([]int, error) {
220+
func (m *Manager) GetPids() ([]int, error) {
221221
return cgroups.GetPids(m.Path("devices"))
222222
}
223223

224-
func (m *manager) GetAllPids() ([]int, error) {
224+
func (m *Manager) GetAllPids() ([]int, error) {
225225
return cgroups.GetAllPids(m.Path("devices"))
226226
}
227227

228-
func (m *manager) GetPaths() map[string]string {
228+
func (m *Manager) GetPaths() map[string]string {
229229
m.mu.Lock()
230230
defer m.mu.Unlock()
231231
return m.paths
232232
}
233233

234-
func (m *manager) GetCgroups() (*configs.Cgroup, error) {
234+
func (m *Manager) GetCgroups() (*configs.Cgroup, error) {
235235
return m.cgroups, nil
236236
}
237237

238-
func (m *manager) GetFreezerState() (configs.FreezerState, error) {
238+
func (m *Manager) GetFreezerState() (configs.FreezerState, error) {
239239
dir := m.Path("freezer")
240240
// If the container doesn't have the freezer cgroup, say it's undefined.
241241
if dir == "" {
@@ -245,15 +245,15 @@ func (m *manager) GetFreezerState() (configs.FreezerState, error) {
245245
return freezer.GetState(dir)
246246
}
247247

248-
func (m *manager) Exists() bool {
248+
func (m *Manager) Exists() bool {
249249
return cgroups.PathExists(m.Path("devices"))
250250
}
251251

252252
func OOMKillCount(path string) (uint64, error) {
253253
return fscommon.GetValueByKey(path, "memory.oom_control", "oom_kill")
254254
}
255255

256-
func (m *manager) OOMKillCount() (uint64, error) {
256+
func (m *Manager) OOMKillCount() (uint64, error) {
257257
c, err := OOMKillCount(m.Path("memory"))
258258
// Ignore ENOENT when rootless as it couldn't create cgroup.
259259
if err != nil && m.cgroups.Rootless && os.IsNotExist(err) {

libcontainer/cgroups/fs2/fs2.go

+18-18
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
type parseError = fscommon.ParseError
1515

16-
type manager struct {
16+
type Manager struct {
1717
config *configs.Cgroup
1818
// dirPath is like "/sys/fs/cgroup/user.slice/user-1001.slice/session-1.scope"
1919
dirPath string
@@ -25,7 +25,7 @@ type manager struct {
2525
// NewManager creates a manager for cgroup v2 unified hierarchy.
2626
// dirPath is like "/sys/fs/cgroup/user.slice/user-1001.slice/session-1.scope".
2727
// If dirPath is empty, it is automatically set using config.
28-
func NewManager(config *configs.Cgroup, dirPath string) (cgroups.Manager, error) {
28+
func NewManager(config *configs.Cgroup, dirPath string) (*Manager, error) {
2929
if dirPath == "" {
3030
var err error
3131
dirPath, err = defaultDirPath(config)
@@ -34,14 +34,14 @@ func NewManager(config *configs.Cgroup, dirPath string) (cgroups.Manager, error)
3434
}
3535
}
3636

37-
m := &manager{
37+
m := &Manager{
3838
config: config,
3939
dirPath: dirPath,
4040
}
4141
return m, nil
4242
}
4343

44-
func (m *manager) getControllers() error {
44+
func (m *Manager) getControllers() error {
4545
if m.controllers != nil {
4646
return nil
4747
}
@@ -62,7 +62,7 @@ func (m *manager) getControllers() error {
6262
return nil
6363
}
6464

65-
func (m *manager) Apply(pid int) error {
65+
func (m *Manager) Apply(pid int) error {
6666
if err := CreateCgroupPath(m.dirPath, m.config); err != nil {
6767
// Related tests:
6868
// - "runc create (no limits + no cgrouppath + no permission) succeeds"
@@ -84,15 +84,15 @@ func (m *manager) Apply(pid int) error {
8484
return nil
8585
}
8686

87-
func (m *manager) GetPids() ([]int, error) {
87+
func (m *Manager) GetPids() ([]int, error) {
8888
return cgroups.GetPids(m.dirPath)
8989
}
9090

91-
func (m *manager) GetAllPids() ([]int, error) {
91+
func (m *Manager) GetAllPids() ([]int, error) {
9292
return cgroups.GetAllPids(m.dirPath)
9393
}
9494

95-
func (m *manager) GetStats() (*cgroups.Stats, error) {
95+
func (m *Manager) GetStats() (*cgroups.Stats, error) {
9696
var errs []error
9797

9898
st := cgroups.NewStats()
@@ -128,7 +128,7 @@ func (m *manager) GetStats() (*cgroups.Stats, error) {
128128
return st, nil
129129
}
130130

131-
func (m *manager) Freeze(state configs.FreezerState) error {
131+
func (m *Manager) Freeze(state configs.FreezerState) error {
132132
if m.config.Resources == nil {
133133
return errors.New("cannot toggle freezer: cgroups not configured for container")
134134
}
@@ -139,15 +139,15 @@ func (m *manager) Freeze(state configs.FreezerState) error {
139139
return nil
140140
}
141141

142-
func (m *manager) Destroy() error {
142+
func (m *Manager) Destroy() error {
143143
return cgroups.RemovePath(m.dirPath)
144144
}
145145

146-
func (m *manager) Path(_ string) string {
146+
func (m *Manager) Path(_ string) string {
147147
return m.dirPath
148148
}
149149

150-
func (m *manager) Set(r *configs.Resources) error {
150+
func (m *Manager) Set(r *configs.Resources) error {
151151
if r == nil {
152152
return nil
153153
}
@@ -213,7 +213,7 @@ func setDevices(dirPath string, r *configs.Resources) error {
213213
return cgroups.DevicesSetV2(dirPath, r)
214214
}
215215

216-
func (m *manager) setUnified(res map[string]string) error {
216+
func (m *Manager) setUnified(res map[string]string) error {
217217
for k, v := range res {
218218
if strings.Contains(k, "/") {
219219
return fmt.Errorf("unified resource %q must be a file name (no slashes)", k)
@@ -239,29 +239,29 @@ func (m *manager) setUnified(res map[string]string) error {
239239
return nil
240240
}
241241

242-
func (m *manager) GetPaths() map[string]string {
242+
func (m *Manager) GetPaths() map[string]string {
243243
paths := make(map[string]string, 1)
244244
paths[""] = m.dirPath
245245
return paths
246246
}
247247

248-
func (m *manager) GetCgroups() (*configs.Cgroup, error) {
248+
func (m *Manager) GetCgroups() (*configs.Cgroup, error) {
249249
return m.config, nil
250250
}
251251

252-
func (m *manager) GetFreezerState() (configs.FreezerState, error) {
252+
func (m *Manager) GetFreezerState() (configs.FreezerState, error) {
253253
return getFreezer(m.dirPath)
254254
}
255255

256-
func (m *manager) Exists() bool {
256+
func (m *Manager) Exists() bool {
257257
return cgroups.PathExists(m.dirPath)
258258
}
259259

260260
func OOMKillCount(path string) (uint64, error) {
261261
return fscommon.GetValueByKey(path, "memory.events", "oom_kill")
262262
}
263263

264-
func (m *manager) OOMKillCount() (uint64, error) {
264+
func (m *Manager) OOMKillCount() (uint64, error) {
265265
c, err := OOMKillCount(m.dirPath)
266266
if err != nil && m.config.Rootless && os.IsNotExist(err) {
267267
err = nil

libcontainer/cgroups/systemd/devices.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717
// (unlike our fs driver, they will happily write deny-all rules to running
1818
// containers). So we have to freeze the container to avoid the container get
1919
// an occasional "permission denied" error.
20-
func (m *legacyManager) freezeBeforeSet(unitName string, r *configs.Resources) (needsFreeze, needsThaw bool, err error) {
20+
func (m *LegacyManager) freezeBeforeSet(unitName string, r *configs.Resources) (needsFreeze, needsThaw bool, err error) {
2121
// Special case for SkipDevices, as used by Kubernetes to create pod
2222
// cgroups with allow-all device policy).
2323
if r.SkipDevices {

libcontainer/cgroups/systemd/freeze_test.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,9 @@ func TestFreezeBeforeSet(t *testing.T) {
139139
t.Fatal(err)
140140
}
141141
defer m.Destroy() //nolint:errcheck
142-
lm := m.(*legacyManager)
143142

144143
// Checks for a non-existent unit.
145-
freeze, thaw, err := lm.freezeBeforeSet(getUnitName(tc.cg), tc.cg.Resources)
144+
freeze, thaw, err := m.freezeBeforeSet(getUnitName(tc.cg), tc.cg.Resources)
146145
if err != nil {
147146
t.Fatal(err)
148147
}
@@ -176,7 +175,7 @@ func TestFreezeBeforeSet(t *testing.T) {
176175
return // no more checks
177176
}
178177
}
179-
freeze, thaw, err = lm.freezeBeforeSet(getUnitName(tc.cg), tc.cg.Resources)
178+
freeze, thaw, err = m.freezeBeforeSet(getUnitName(tc.cg), tc.cg.Resources)
180179
if err != nil {
181180
t.Error(err)
182181
return // no more checks

0 commit comments

Comments
 (0)