From d31bb31072853e5f855601678cf0c7284c9eba8f Mon Sep 17 00:00:00 2001
From: Thomas Schubart <thomas@gitpod.io>
Date: Tue, 19 Apr 2022 13:36:49 +0000
Subject: [PATCH] Fix io imits not written after workspace stop

---
 components/ws-daemon/pkg/cgroup/cgroup.go     |  2 +-
 .../ws-daemon/pkg/cgroup/plugin_iolimit_v2.go | 47 +++++++++++++------
 components/ws-daemon/pkg/daemon/daemon.go     |  7 +--
 3 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/components/ws-daemon/pkg/cgroup/cgroup.go b/components/ws-daemon/pkg/cgroup/cgroup.go
index d6a4dc8d4938b6..27c198262e5627 100644
--- a/components/ws-daemon/pkg/cgroup/cgroup.go
+++ b/components/ws-daemon/pkg/cgroup/cgroup.go
@@ -81,7 +81,7 @@ func (host *PluginHost) WorkspaceAdded(ctx context.Context, ws *dispatch.Workspa
 
 	cgroupPath, err := disp.Runtime.ContainerCGroupPath(context.Background(), ws.ContainerID)
 	if err != nil {
-		return xerrors.Errorf("cannot start governer: %w", err)
+		return xerrors.Errorf("cannot get cgroup path for container %s: %w", ws.ContainerID, err)
 	}
 
 	for _, plg := range host.Plugins {
diff --git a/components/ws-daemon/pkg/cgroup/plugin_iolimit_v2.go b/components/ws-daemon/pkg/cgroup/plugin_iolimit_v2.go
index 4a11baa35b5336..ccfea0bec4e4cf 100644
--- a/components/ws-daemon/pkg/cgroup/plugin_iolimit_v2.go
+++ b/components/ws-daemon/pkg/cgroup/plugin_iolimit_v2.go
@@ -15,11 +15,35 @@ import (
 	"github.com/gitpod-io/gitpod/common-go/log"
 )
 
-type IOLimiterV2 struct {
+var clearLimits = ioLimitOptions{
+	WriteBytesPerSecond: 0,
+	ReadBytesPerSecond:  0,
+	WriteIOPs:           0,
+	ReadIOPs:            0,
+}
+
+type ioLimitOptions struct {
 	WriteBytesPerSecond int64
 	ReadBytesPerSecond  int64
-	ReadIOPs            int64
 	WriteIOPs           int64
+	ReadIOPs            int64
+}
+
+type IOLimiterV2 struct {
+	limits ioLimitOptions
+}
+
+func NewIOLimiterV2(writeBytesPerSecond, readBytesPerSecond, writeIOPs, readIOPs int64) *IOLimiterV2 {
+	limits := ioLimitOptions{
+		WriteBytesPerSecond: writeBytesPerSecond,
+		ReadBytesPerSecond:  readBytesPerSecond,
+		WriteIOPs:           writeIOPs,
+		ReadIOPs:            readIOPs,
+	}
+
+	return &IOLimiterV2{
+		limits: limits,
+	}
 }
 
 func (c *IOLimiterV2) Name() string  { return "iolimiter-v2" }
@@ -42,19 +66,15 @@ func (c *IOLimiterV2) Apply(ctx context.Context, basePath, cgroupPath string) er
 				// Prior to shutting down though, we need to reset the IO limits to ensure we don't have
 				// processes stuck in the uninterruptable "D" (disk sleep) state. This would prevent the
 				// workspace pod from shutting down.
-				c.WriteBytesPerSecond = 0
-				c.ReadBytesPerSecond = 0
-				c.WriteIOPs = 0
-				c.ReadIOPs = 0
 
-				err := c.writeIOMax(ioMaxFile)
+				err := c.writeIOMax(ioMaxFile, clearLimits)
 				if err != nil {
 					log.WithError(err).WithField("cgroupPath", cgroupPath).Error("cannot write IO limits")
 				}
 				log.WithField("cgroupPath", cgroupPath).Debug("stopping io limiting")
 				return
 			case <-ticker.C:
-				err := c.writeIOMax(ioMaxFile)
+				err := c.writeIOMax(ioMaxFile, c.limits)
 				if err != nil {
 					log.WithError(err).WithField("cgroupPath", cgroupPath).Error("cannot write IO limits")
 				}
@@ -64,7 +84,7 @@ func (c *IOLimiterV2) Apply(ctx context.Context, basePath, cgroupPath string) er
 	return nil
 }
 
-func (c *IOLimiterV2) writeIOMax(cgroupPath string) error {
+func (c *IOLimiterV2) writeIOMax(cgroupPath string, options ioLimitOptions) error {
 	iostat, err := os.ReadFile(filepath.Join(string(cgroupPath), "io.stat"))
 	if os.IsNotExist(err) {
 		// cgroup gone is ok due to the dispatch/container race
@@ -80,7 +100,6 @@ func (c *IOLimiterV2) writeIOMax(cgroupPath string) error {
 	// 9 block	Metadisk (RAID) devices
 	// source https://www.kernel.org/doc/Documentation/admin-guide/devices.txt
 	var classesToLimit = []string{"8", "9"}
-
 	var devs []string
 	for _, line := range strings.Split(string(iostat), "\n") {
 		fields := strings.Fields(line)
@@ -100,10 +119,10 @@ func (c *IOLimiterV2) writeIOMax(cgroupPath string) error {
 		limit := fmt.Sprintf(
 			"%s wbps=%s rbps=%s wiops=%s riops=%s",
 			dev,
-			getLimit(c.WriteBytesPerSecond),
-			getLimit(c.ReadBytesPerSecond),
-			getLimit(c.WriteIOPs),
-			getLimit(c.ReadIOPs),
+			getLimit(options.WriteBytesPerSecond),
+			getLimit(options.ReadBytesPerSecond),
+			getLimit(options.WriteIOPs),
+			getLimit(options.ReadIOPs),
 		)
 
 		log.WithField("limit", limit).WithField("ioMaxPath", ioMaxPath).Debug("creating io.max limit")
diff --git a/components/ws-daemon/pkg/daemon/daemon.go b/components/ws-daemon/pkg/daemon/daemon.go
index 158604c7d163f5..349ea47753a784 100644
--- a/components/ws-daemon/pkg/daemon/daemon.go
+++ b/components/ws-daemon/pkg/daemon/daemon.go
@@ -57,12 +57,7 @@ func NewDaemon(config Config, reg prometheus.Registerer) (*Daemon, error) {
 		&cgroup.CacheReclaim{},
 		&cgroup.FuseDeviceEnablerV1{},
 		&cgroup.FuseDeviceEnablerV2{},
-		&cgroup.IOLimiterV2{
-			WriteBytesPerSecond: config.IOLimit.WriteBWPerSecond.Value(),
-			ReadBytesPerSecond:  config.IOLimit.ReadBWPerSecond.Value(),
-			WriteIOPs:           config.IOLimit.WriteIOPS,
-			ReadIOPs:            config.IOLimit.ReadIOPS,
-		},
+		cgroup.NewIOLimiterV2(config.IOLimit.WriteBWPerSecond.Value(), config.IOLimit.ReadBWPerSecond.Value(), config.IOLimit.WriteIOPS, config.IOLimit.ReadIOPS),
 	)
 	if err != nil {
 		return nil, err