Merge pull request #184 from Scalingo/fix/183/cpu-monitoring-stop

fix(cpu/monitoring) Correctly handle error when CPU monitoring fails because cgroup has been deleted (container stopped)

Author: leo-scalingo

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## To Be Released
 
+## v2.0.2 - 2025-09-25
+
+* Correctly handle error when CPU monitoring fails because cgroup has been deleted (container stopped)
+
 ## v2.0.1 - 2025-09-09
 
 * dist(cgo) Disable CGO to build binaries

README.md

@@ -1,4 +1,4 @@
-# Acadock Monitoring - Docker container monitoring v2.0.1
+# Acadock Monitoring - Docker container monitoring v2.0.2
 
 This webservice provides live data on Docker containers. It takes
 data from the Linux kernel control groups and from the namespace of
@@ -88,7 +88,7 @@ Bump new version number in:
 Commit, tag and create a new release:
 
 ```sh
-version="2.0.1"
+version="2.0.2"
 
 git switch --create release/${version}
 git add CHANGELOG.md README.md

cgroup/stats.go

@@ -39,10 +39,14 @@ func (e StatsReaderError) Unwrap() error {
 	return e.err
 }
 
+func NewStatsReaderError(err error) StatsReaderError {
+	return StatsReaderError{err: err}
+}
+
 func (r *StatsReaderImpl) GetStats(ctx context.Context, containerID string) (Stats, error) {
 	manager, err := NewManager(ctx, containerID)
 	if err != nil {
-		return Stats{}, errors.Wrap(ctx, err, "create cgroup manager")
+		return Stats{}, NewStatsReaderError(errors.Wrap(ctx, err, "create cgroup manager"))
 	}
 	var stats Stats
 	if manager.IsV2() {
@@ -51,7 +55,7 @@ func (r *StatsReaderImpl) GetStats(ctx context.Context, containerID string) (Sta
 		stats, err = r.getCgroupV1Stats(ctx, manager)
 	}
 	if err != nil {
-		return Stats{}, StatsReaderError{err: errors.Wrap(ctx, err, "get cgroup stats")}
+		return Stats{}, NewStatsReaderError(errors.Wrap(ctx, err, "get cgroup stats"))
 	}
 	return stats, nil
 }

cpu/cpu.go

@@ -56,7 +56,7 @@ func (m *CPUUsageMonitor) Start(ctx context.Context) {
 		case docker.ContainerActionStart:
 			ctx, cancel := context.WithCancel(ctx)
 			cancels[event.ContainerID] = cancel
-			log.Infof("Start monitoring CPU")
+			log.Info("Start monitoring CPU")
 			go m.monitorContainerCPU(ctx, event.ContainerID)
 		case docker.ContainerActionStop:
 			log.Info("Stop monitoring CPU")
@@ -122,6 +122,7 @@ func (m *CPUUsageMonitor) monitorContainerCPU(ctx context.Context, id string) {
 			if errors.As(err, &cgroupStatsErr) {
 				log.WithError(err).Infof("Stop monitoring CPU with error")
 				m.cleanMonitoringData(id)
+				return
 			} else if err != nil {
 				// No Error logging to prevent spamming
 				log.WithError(err).Info("Fail to update container CPU usage")

cpu/cpu_test.go

@@ -2,6 +2,7 @@ package cpu
 
 import (
 	"context"
+	"errors"
 	"sync"
 	"testing"
 	"time"
@@ -68,3 +69,44 @@ func TestCPUUsageMonitor_Start(t *testing.T) {
 	cancel()
 	wg.Wait()
 }
+
+func TestCPUUsageMonitor_Start_Shutdown(t *testing.T) {
+	ctrl := gomock.NewController(t)
+
+	cgroupStatsReader := cgroupmock.NewMockStatsReader(ctrl)
+	cpuStatsReader := procfs.NewMockCPUStat(ctrl)
+	containerRepository := dockermock.NewMockContainerRepository(ctrl)
+	dockerID := "1"
+	config.RefreshTime = 100 * time.Millisecond
+
+	cpuStatsReader.EXPECT().Read(gomock.Any()).Return(procfs.CPUStats{}, nil).AnyTimes()
+	err := errors.New("cgroup error")
+	cgroupStatsReader.EXPECT().GetStats(gomock.Any(), dockerID).Return(cgroup.Stats{}, cgroup.NewStatsReaderError(err)).MaxTimes(1)
+
+	eventsChan := make(chan docker.ContainerEvent)
+	containerRepository.EXPECT().RegisterToContainersStream(gomock.Any()).Return(eventsChan)
+	go func() {
+		eventsChan <- docker.ContainerEvent{ContainerID: dockerID, Action: docker.ContainerActionStart}
+		close(eventsChan)
+	}()
+
+	monitor := NewCPUUsageMonitor(containerRepository, cpuStatsReader, cgroupStatsReader)
+
+	ctx, cancel := context.WithCancel(t.Context())
+	wg := &sync.WaitGroup{}
+	wg.Add(1)
+	go func() {
+		monitor.Start(ctx)
+		wg.Done()
+	}()
+
+	// After 2 cycles it must have accurate CPU information
+	time.Sleep(2 * config.RefreshTime)
+	usage, err := monitor.GetContainerUsage(dockerID)
+	require.NoError(t, err)
+	require.Zero(t, usage)
+
+	// Check that the context does stop the Monitor
+	cancel()
+	wg.Wait()
+}