From 7471a05d49240ea1163c0886a1c349744f050a1b Mon Sep 17 00:00:00 2001 From: MinJae Kwon Date: Thu, 3 Nov 2022 12:10:55 +0900 Subject: [PATCH] fix(cgroupv2): handle undefined CPU quota (#9) * fix(cgroupv2): handle undefined CPU quota * refactor: use os.IsNotExist instead of errors.Is() * test: add loadCPUQuota tests * docs: update README.md --- README.md | 6 ++-- autopprof.go | 22 ++++++++++++- autopprof_test.go | 79 +++++++++++++++++++++++++++++++++++++++++++++++ cgroups.go | 2 ++ cgroups_mock.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++ cgroupv2.go | 10 +++++- error.go | 1 + 7 files changed, 193 insertions(+), 5 deletions(-) create mode 100644 cgroups_mock.go diff --git a/README.md b/README.md index 8c3ed1d..03b0aeb 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,8 @@ ![Run tests](https://github.com/daangn/autopprof/workflows/Run%20tests/badge.svg) [![Release](https://img.shields.io/github/v/tag/daangn/autopprof?label=Release)](https://github.com/daangn/autopprof/releases) -Automatically profile the Go applications when CPU or memory utilization crosses -threshold levels. +Automatically profile the Go applications when CPU or memory utilization crosses specific +threshold levels against the Linux container CPU quota and memory limit. Once you start the autopprof, the autopprof process will periodically check the CPU and memory utilization of the Go applications. If the resource utilization crosses the @@ -61,7 +61,7 @@ func main() { } ``` -> You can create the custom reporter by implementing the `report.Reporter` interface. +> You can create a custom reporter by implementing the `report.Reporter` interface. ## Benchmark diff --git a/autopprof.go b/autopprof.go index 3bfc7f1..ba3ca31 100644 --- a/autopprof.go +++ b/autopprof.go @@ -89,7 +89,7 @@ func Start(opt Option) error { ap.memThreshold = opt.MemThreshold } if !ap.disableCPUProf { - if err := ap.queryer.setCPUQuota(); err != nil { + if err := ap.loadCPUQuota(); err != nil { return err } } @@ -106,6 +106,26 @@ func Stop() { } } +func (ap *autoPprof) loadCPUQuota() error { + err := ap.queryer.setCPUQuota() + if err == nil { + return nil + } + + // If memory profiling is disabled and CPU quota isn't set, + // returns an error immediately. + if ap.disableMemProf { + return err + } + // If memory profiling is enabled, just logs the error and + // disables the cpu profiling. + log.Println( + "autopprof: disable the cpu profiling due to the CPU quota isn't set", + ) + ap.disableCPUProf = true + return nil +} + func (ap *autoPprof) watch() { go ap.watchCPUUsage() go ap.watchMemUsage() diff --git a/autopprof_test.go b/autopprof_test.go index 6415388..585286a 100644 --- a/autopprof_test.go +++ b/autopprof_test.go @@ -146,6 +146,85 @@ func TestStop(t *testing.T) { } } +func TestAutoPprof_loadCPUQuota(t *testing.T) { + testCases := []struct { + name string + newAp func() *autoPprof + wantDisableCPUProfFlag bool + wantErr error + }{ + { + name: "cpu quota is set", + newAp: func() *autoPprof { + ctrl := gomock.NewController(t) + + mockQueryer := NewMockqueryer(ctrl) + mockQueryer.EXPECT(). + setCPUQuota(). + Return(nil) // Means that the quota is set correctly. + + return &autoPprof{ + queryer: mockQueryer, + disableCPUProf: false, + disableMemProf: false, + } + }, + wantDisableCPUProfFlag: false, + wantErr: nil, + }, + { + name: "cpu quota isn't set and memory profiling is enabled", + newAp: func() *autoPprof { + ctrl := gomock.NewController(t) + + mockQueryer := NewMockqueryer(ctrl) + mockQueryer.EXPECT(). + setCPUQuota(). + Return(ErrV2CPUQuotaUndefined) + + return &autoPprof{ + queryer: mockQueryer, + disableCPUProf: false, + disableMemProf: false, + } + }, + wantDisableCPUProfFlag: true, + wantErr: nil, + }, + { + name: "cpu quota isn't set and memory profiling is disabled", + newAp: func() *autoPprof { + ctrl := gomock.NewController(t) + + mockQueryer := NewMockqueryer(ctrl) + mockQueryer.EXPECT(). + setCPUQuota(). + Return(ErrV2CPUQuotaUndefined) + + return &autoPprof{ + queryer: mockQueryer, + disableCPUProf: false, + disableMemProf: true, + } + }, + wantDisableCPUProfFlag: false, + wantErr: ErrV2CPUQuotaUndefined, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ap := tc.newAp() + err := ap.loadCPUQuota() + if !errors.Is(err, tc.wantErr) { + t.Errorf("loadCPUQuota() = %v, want %v", err, tc.wantErr) + } + if ap.disableCPUProf != tc.wantDisableCPUProfFlag { + t.Errorf("disableCPUProf = %v, want %v", ap.disableCPUProf, tc.wantDisableCPUProfFlag) + } + }) + } +} + func TestAutoPprof_watchCPUUsage(t *testing.T) { ctrl := gomock.NewController(t) diff --git a/cgroups.go b/cgroups.go index 369380e..4068fa6 100644 --- a/cgroups.go +++ b/cgroups.go @@ -7,6 +7,8 @@ import ( "github.com/containerd/cgroups" ) +//go:generate mockgen -source=cgroups.go -destination=cgroups_mock.go -package=autopprof + type queryer interface { cpuUsage() (float64, error) memUsage() (float64, error) diff --git a/cgroups_mock.go b/cgroups_mock.go new file mode 100644 index 0000000..2f51c06 --- /dev/null +++ b/cgroups_mock.go @@ -0,0 +1,78 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: cgroups.go + +// Package autopprof is a generated GoMock package. +package autopprof + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// Mockqueryer is a mock of queryer interface. +type Mockqueryer struct { + ctrl *gomock.Controller + recorder *MockqueryerMockRecorder +} + +// MockqueryerMockRecorder is the mock recorder for Mockqueryer. +type MockqueryerMockRecorder struct { + mock *Mockqueryer +} + +// NewMockqueryer creates a new mock instance. +func NewMockqueryer(ctrl *gomock.Controller) *Mockqueryer { + mock := &Mockqueryer{ctrl: ctrl} + mock.recorder = &MockqueryerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *Mockqueryer) EXPECT() *MockqueryerMockRecorder { + return m.recorder +} + +// cpuUsage mocks base method. +func (m *Mockqueryer) cpuUsage() (float64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "cpuUsage") + ret0, _ := ret[0].(float64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// cpuUsage indicates an expected call of cpuUsage. +func (mr *MockqueryerMockRecorder) cpuUsage() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "cpuUsage", reflect.TypeOf((*Mockqueryer)(nil).cpuUsage)) +} + +// memUsage mocks base method. +func (m *Mockqueryer) memUsage() (float64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "memUsage") + ret0, _ := ret[0].(float64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// memUsage indicates an expected call of memUsage. +func (mr *MockqueryerMockRecorder) memUsage() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "memUsage", reflect.TypeOf((*Mockqueryer)(nil).memUsage)) +} + +// setCPUQuota mocks base method. +func (m *Mockqueryer) setCPUQuota() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "setCPUQuota") + ret0, _ := ret[0].(error) + return ret0 +} + +// setCPUQuota indicates an expected call of setCPUQuota. +func (mr *MockqueryerMockRecorder) setCPUQuota() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "setCPUQuota", reflect.TypeOf((*Mockqueryer)(nil).setCPUQuota)) +} diff --git a/cgroupv2.go b/cgroupv2.go index 4851bf8..ab9540b 100644 --- a/cgroupv2.go +++ b/cgroupv2.go @@ -18,7 +18,9 @@ import ( const ( cgroupV2MountPoint = "/sys/fs/cgroup" - cgroupV2CPUMaxFile = "cpu.max" + + cgroupV2CPUMaxFile = "cpu.max" + cgroupV2CPUMaxQuotaMax = "max" cgroupV2CPUMaxDefaultPeriod = 100000 ) @@ -46,6 +48,9 @@ func (c *cgroupV2) setCPUQuota() error { f, err := os.Open( path.Join(c.mountPoint, c.cpuMaxFile), ) + if os.IsNotExist(err) { + return ErrV2CPUQuotaUndefined + } if err != nil { return err } @@ -58,6 +63,9 @@ func (c *cgroupV2) setCPUQuota() error { "autopprof: invalid cpu.max format", ) } + if fields[0] == cgroupV2CPUMaxQuotaMax { + return ErrV2CPUQuotaUndefined + } max, err := strconv.Atoi(fields[0]) if err != nil { diff --git a/error.go b/error.go index d394348..edba05f 100644 --- a/error.go +++ b/error.go @@ -16,6 +16,7 @@ var ( ) ErrNilReporter = fmt.Errorf("autopprof: Reporter can't be nil") ErrDisableAllProfiling = fmt.Errorf("autopprof: all profiling is disabled") + ErrV2CPUQuotaUndefined = fmt.Errorf("autopprof: v2 cpu quota is undefined") ErrV2CPUMaxEmpty = fmt.Errorf("autopprof: v2 cpu.max is empty") ErrV1CPUSubsystemEmpty = fmt.Errorf("autopprof: v1 cpu subsystem is empty") )