From 5138c5b99e3241351a9663b3171c939744c7d483 Mon Sep 17 00:00:00 2001
From: Seth Hoenig <shoenig@duck.com>
Date: Thu, 22 Jun 2023 09:36:31 -0500
Subject: [PATCH] client: do not disable memory swappiness if kernel does not
 support it (#17625)

* client: do not disable memory swappiness if kernel does not support it

This PR adds a workaround for very old Linux kernels which do not support
the memory swappiness interface file. Normally we write a "0" to the file
to explicitly disable swap. In the case the kernel does not support it,
give libcontainer a nil value so it does not write anything.

Fixes #17448

* client: detect swappiness by writing to the file

* fixup changelog

Co-authored-by: James Rasell <jrasell@users.noreply.github.com>

---------

Co-authored-by: James Rasell <jrasell@users.noreply.github.com>
---
 .changelog/17625.txt                      |  3 +++
 client/lib/cgutil/cgutil_linux.go         | 24 +++++++++++++++++++++++
 client/lib/cgutil/cgutil_linux_test.go    |  9 +++++++++
 drivers/shared/executor/executor_linux.go |  5 ++---
 4 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 .changelog/17625.txt

diff --git a/.changelog/17625.txt b/.changelog/17625.txt
new file mode 100644
index 000000000..df5ea63b9
--- /dev/null
+++ b/.changelog/17625.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+client: Fixed a bug where Nomad incorrectly wrote to memory swappiness cgroup on old kernels
+```
diff --git a/client/lib/cgutil/cgutil_linux.go b/client/lib/cgutil/cgutil_linux.go
index 7727b2a43..ffb3f4c18 100644
--- a/client/lib/cgutil/cgutil_linux.go
+++ b/client/lib/cgutil/cgutil_linux.go
@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 
 	"github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	lcc "github.com/opencontainers/runc/libcontainer/configs"
@@ -148,3 +149,26 @@ func CopyCpuset(source, destination string) error {
 
 	return nil
 }
+
+// MaybeDisableMemorySwappiness will disable memory swappiness, if that controller
+// is available. Always the case for cgroups v2, but is not always the case on
+// very old kernels with cgroups v1.
+func MaybeDisableMemorySwappiness() *uint64 {
+	bypass := (*uint64)(nil)
+	zero := pointer.Of[uint64](0)
+
+	// cgroups v2 always set zero
+	if UseV2 {
+		return zero
+	}
+
+	// cgroups v1 detect if swappiness is supported by attempting to write to
+	// the nomad parent cgroup swappiness interface
+	e := &editor{fromRoot: "memory/nomad"}
+	err := e.write("memory.swappiness", "0")
+	if err != nil {
+		return bypass
+	}
+
+	return zero
+}
diff --git a/client/lib/cgutil/cgutil_linux_test.go b/client/lib/cgutil/cgutil_linux_test.go
index 8047b5ccc..9c51e3ef3 100644
--- a/client/lib/cgutil/cgutil_linux_test.go
+++ b/client/lib/cgutil/cgutil_linux_test.go
@@ -130,3 +130,12 @@ func TestUtil_CopyCpuset(t *testing.T) {
 		require.Equal(t, "0-1", strings.TrimSpace(value))
 	})
 }
+
+func TestUtil_MaybeDisableMemorySwappiness(t *testing.T) {
+	ci.Parallel(t)
+
+	// will return 0 on any reasonable kernel (both cgroups v1 and v2)
+	value := MaybeDisableMemorySwappiness()
+	must.NotNil(t, value)
+	must.Eq(t, 0, *value)
+}
diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go
index 1d9b352fb..d0c4a743b 100644
--- a/drivers/shared/executor/executor_linux.go
+++ b/drivers/shared/executor/executor_linux.go
@@ -689,9 +689,8 @@ func configureCgroups(cfg *lconfigs.Config, command *ExecCommand) error {
 		cfg.Cgroups.Resources.Memory = memHard * 1024 * 1024
 		cfg.Cgroups.Resources.MemoryReservation = memSoft * 1024 * 1024
 
-		// Disable swap to avoid issues on the machine
-		var memSwappiness uint64
-		cfg.Cgroups.Resources.MemorySwappiness = &memSwappiness
+		// Disable swap if possible, to avoid issues on the machine
+		cfg.Cgroups.Resources.MemorySwappiness = cgutil.MaybeDisableMemorySwappiness()
 	}
 
 	cpuShares := res.Cpu.CpuShares