Skip to content

Commit c1ab5cf

Browse files
johnstcnjohnstcn
authored
fix(cli/clistat): better handle cgroups with no limits (#8373)
1 parent f75d497 commit c1ab5cf

File tree

2 files changed

+73
-27
lines changed

2 files changed

+73
-27
lines changed

‎cli/clistat/cgroup.go

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ func (s *Statter) ContainerCPU() (*Result, error) {
6161
if err != nil {
6262
return nil, xerrors.Errorf("get total cpu: %w", err)
6363
}
64-
6564
used1, err := s.cGroupCPUUsed()
6665
if err != nil {
6766
return nil, xerrors.Errorf("get cgroup CPU usage: %w", err)
@@ -86,9 +85,12 @@ func (s *Statter) ContainerCPU() (*Result, error) {
8685
r := &Result{
8786
Unit: "cores",
8887
Used: used2 - used1,
89-
Total: ptr.To(total),
9088
Prefix: PrefixDefault,
9189
}
90+
91+
if total > 0 {
92+
r.Total = ptr.To(total)
93+
}
9294
return r, nil
9395
}
9496

@@ -137,8 +139,12 @@ func (s *Statter) cGroupV2CPUTotal() (total float64, err error) {
137139

138140
quotaUs, err = readInt64SepIdx(s.fs, cgroupV2CPUMax, " ", 0)
139141
if err != nil {
140-
// Fall back to number of cores
141-
quotaUs = int64(s.nproc) * periodUs
142+
if xerrors.Is(err, strconv.ErrSyntax) {
143+
// If the value is not a valid integer, assume it is the string
144+
// 'max' and that there is no limit set.
145+
return -1, nil
146+
}
147+
return 0, xerrors.Errorf("get cpu quota: %w", err)
142148
}
143149

144150
return float64(quotaUs) / float64(periodUs), nil
@@ -156,8 +162,7 @@ func (s *Statter) cGroupV1CPUTotal() (float64, error) {
156162
}
157163

158164
if quotaUs < 0 {
159-
// Fall back to the number of cores
160-
quotaUs = int64(s.nproc) * periodUs
165+
return -1, nil
161166
}
162167

163168
return float64(quotaUs) / float64(periodUs), nil
@@ -199,9 +204,19 @@ func (s *Statter) ContainerMemory(p Prefix) (*Result, error) {
199204
}
200205

201206
func (s *Statter) cGroupV2Memory(p Prefix) (*Result, error) {
207+
r := &Result{
208+
Unit: "B",
209+
Prefix: p,
210+
}
202211
maxUsageBytes, err := readInt64(s.fs, cgroupV2MemoryMaxBytes)
203212
if err != nil {
204-
return nil, xerrors.Errorf("read memory total: %w", err)
213+
if !xerrors.Is(err, strconv.ErrSyntax) {
214+
return nil, xerrors.Errorf("read memory total: %w", err)
215+
}
216+
// If the value is not a valid integer, assume it is the string
217+
// 'max' and that there is no limit set.
218+
} else {
219+
r.Total = ptr.To(float64(maxUsageBytes))
205220
}
206221

207222
currUsageBytes, err := readInt64(s.fs, cgroupV2MemoryUsageBytes)
@@ -214,18 +229,23 @@ func (s *Statter) cGroupV2Memory(p Prefix) (*Result, error) {
214229
return nil, xerrors.Errorf("read memory stats: %w", err)
215230
}
216231

217-
return &Result{
218-
Total: ptr.To(float64(maxUsageBytes)),
219-
Used: float64(currUsageBytes - inactiveFileBytes),
220-
Unit: "B",
221-
Prefix: p,
222-
}, nil
232+
r.Used = float64(currUsageBytes - inactiveFileBytes)
233+
return r, nil
223234
}
224235

225236
func (s *Statter) cGroupV1Memory(p Prefix) (*Result, error) {
237+
r := &Result{
238+
Unit: "B",
239+
Prefix: p,
240+
}
226241
maxUsageBytes, err := readInt64(s.fs, cgroupV1MemoryMaxUsageBytes)
227242
if err != nil {
228-
return nil, xerrors.Errorf("read memory total: %w", err)
243+
if !xerrors.Is(err, strconv.ErrSyntax) {
244+
return nil, xerrors.Errorf("read memory total: %w", err)
245+
}
246+
// I haven't found an instance where this isn't a valid integer.
247+
// Nonetheless, if it is not, assume there is no limit set.
248+
maxUsageBytes = -1
229249
}
230250

231251
// need a space after total_rss so we don't hit something else
@@ -239,13 +259,15 @@ func (s *Statter) cGroupV1Memory(p Prefix) (*Result, error) {
239259
return nil, xerrors.Errorf("read memory stats: %w", err)
240260
}
241261

262+
// If max usage bytes is -1, there is no memory limit set.
263+
if maxUsageBytes > 0 {
264+
r.Total = ptr.To(float64(maxUsageBytes))
265+
}
266+
242267
// Total memory used is usage - total_inactive_file
243-
return &Result{
244-
Total: ptr.To(float64(maxUsageBytes)),
245-
Used: float64(usageBytes - totalInactiveFileBytes),
246-
Unit: "B",
247-
Prefix: p,
248-
}, nil
268+
r.Used = float64(usageBytes - totalInactiveFileBytes)
269+
270+
return r, nil
249271
}
250272

251273
// read an int64 value from path

‎cli/clistat/stat_internal_test.go

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,7 @@ func TestStatter(t *testing.T) {
149149
require.NoError(t, err)
150150
require.NotNil(t, cpu)
151151
assert.Equal(t, 1.0, cpu.Used)
152-
require.NotNil(t, cpu.Total)
153-
assert.Equal(t, 2.0, *cpu.Total)
152+
require.Nil(t, cpu.Total)
154153
assert.Equal(t, "cores", cpu.Unit)
155154
})
156155

@@ -167,6 +166,19 @@ func TestStatter(t *testing.T) {
167166
assert.Equal(t, 1073741824.0, *mem.Total)
168167
assert.Equal(t, "B", mem.Unit)
169168
})
169+
170+
t.Run("ContainerMemory/NoLimit", func(t *testing.T) {
171+
t.Parallel()
172+
fs := initFS(t, fsContainerCgroupV1NoLimit)
173+
s, err := New(WithFS(fs), withNoWait)
174+
require.NoError(t, err)
175+
mem, err := s.ContainerMemory(PrefixDefault)
176+
require.NoError(t, err)
177+
require.NotNil(t, mem)
178+
assert.Equal(t, 268435456.0, mem.Used)
179+
assert.Nil(t, mem.Total)
180+
assert.Equal(t, "B", mem.Unit)
181+
})
170182
})
171183

172184
t.Run("CGroupV2", func(t *testing.T) {
@@ -201,12 +213,11 @@ func TestStatter(t *testing.T) {
201213
require.NoError(t, err)
202214
require.NotNil(t, cpu)
203215
assert.Equal(t, 1.0, cpu.Used)
204-
require.NotNil(t, cpu.Total)
205-
assert.Equal(t, 2.0, *cpu.Total)
216+
require.Nil(t, cpu.Total)
206217
assert.Equal(t, "cores", cpu.Unit)
207218
})
208219

209-
t.Run("ContainerMemory", func(t *testing.T) {
220+
t.Run("ContainerMemory/Limit", func(t *testing.T) {
210221
t.Parallel()
211222
fs := initFS(t, fsContainerCgroupV2)
212223
s, err := New(WithFS(fs), withNoWait)
@@ -219,6 +230,19 @@ func TestStatter(t *testing.T) {
219230
assert.Equal(t, 1073741824.0, *mem.Total)
220231
assert.Equal(t, "B", mem.Unit)
221232
})
233+
234+
t.Run("ContainerMemory/NoLimit", func(t *testing.T) {
235+
t.Parallel()
236+
fs := initFS(t, fsContainerCgroupV2NoLimit)
237+
s, err := New(WithFS(fs), withNoWait)
238+
require.NoError(t, err)
239+
mem, err := s.ContainerMemory(PrefixDefault)
240+
require.NoError(t, err)
241+
require.NotNil(t, mem)
242+
assert.Equal(t, 268435456.0, mem.Used)
243+
assert.Nil(t, mem.Total)
244+
assert.Equal(t, "B", mem.Unit)
245+
})
222246
})
223247
}
224248

@@ -316,7 +340,7 @@ proc /proc/sys proc ro,nosuid,nodev,noexec,relatime 0 0`,
316340
proc /proc/sys proc ro,nosuid,nodev,noexec,relatime 0 0`,
317341
cgroupV2CPUMax: "max 100000",
318342
cgroupV2CPUStat: "usage_usec 0",
319-
cgroupV2MemoryMaxBytes: "1073741824",
343+
cgroupV2MemoryMaxBytes: "max",
320344
cgroupV2MemoryUsageBytes: "536870912",
321345
cgroupV2MemoryStat: "inactive_file 268435456",
322346
}
@@ -338,7 +362,7 @@ proc /proc/sys proc ro,nosuid,nodev,noexec,relatime 0 0`,
338362
cgroupV1CPUAcctUsage: "0",
339363
cgroupV1CFSQuotaUs: "-1",
340364
cgroupV1CFSPeriodUs: "100000",
341-
cgroupV1MemoryMaxUsageBytes: "1073741824",
365+
cgroupV1MemoryMaxUsageBytes: "max", // I have never seen this in the wild
342366
cgroupV1MemoryUsageBytes: "536870912",
343367
cgroupV1MemoryStat: "total_inactive_file 268435456",
344368
}

0 commit comments

Comments
 (0)