⚠️ Potential issue | 🟠 Major

Improve error handling to distinguish file-not-found from other errors.

The current implementation treats any error from os.Stat as an indication of cgroup v1, which could lead to incorrect behavior if the error is due to permission issues, I/O errors, or other filesystem problems rather than the file not existing.

Apply this diff to properly handle errors:

 // If the user wants to disable the OOM killer, we need to handle it differently
-// depending on the cgroup version.
+// depending on the cgroup version. On cgroup v2, disabling the OOM killer is not
+// supported (see containers/crun#1416, pterodactyl/panel#4928), so we ignore the
+// user setting and keep it enabled.
 if !l.OOMKiller {
-    if _, err := os.Stat("/sys/fs/cgroup/cgroup.controllers"); err == nil {
-    } else {
+    if _, err := os.Stat("/sys/fs/cgroup/cgroup.controllers"); os.IsNotExist(err) {
+        // cgroup v1: honor user preference to disable OOM killer
+        log.Debug("detected cgroup v1, disabling OOM killer as requested")
         resources.OomKillDisable = boolPtr(true)
+    } else if err != nil {
+        // Unexpected error (permissions, I/O, etc.) - log and default to safe behavior
+        log.WithError(err).Warn("failed to detect cgroup version, keeping OOM killer enabled")
+    } else {
+        // cgroup v2: ignore user preference and keep OOM killer enabled
+        log.Debug("detected cgroup v2, ignoring OOM killer disable request")
     }
 }

This approach:

• Uses os.IsNotExist to specifically check for file-not-found
• Adds logging for observability and debugging
• Handles unexpected errors safely by keeping OOM killer enabled
• Improves the comment to explain the rationale and reference related issues

In environment/settings.go around lines 121 to 128, the os.Stat error is
currently treated as "file missing" for cgroup detection which conflates
missing-file with other errors; update the logic to call os.Stat and then use
os.IsNotExist(err) to detect cgroup v1 explicitly, set resources.OomKillDisable
= boolPtr(true) only when the file does not exist, and in the else branch log
unexpected errors (permission/IO) and leave OOM killer enabled (do not set
OomKillDisable) so we fail safe; also expand the comment to explain the
rationale and reference related issues for future debugging.

I think this would be the best way check if the file doesn't exist instead of any error.

if err == os.ErrNotExist {
    // cgroup v1
}

Go os.IsNotExist vs os.ErrNotExist direct comparison best practice

import (
  "errors"
  "io/fs"
  "os"
)

_, err := os.Stat(path)
if err != nil {
  if errors.Is(err, fs.ErrNotExist) {
    // missing
  } else {
    // other error
  }
}

@parkervcp, I appreciate the feedback! However, direct comparison with err == os.ErrNotExist unfortunately won't work reliably in this case. Here's why:

os.Stat() returns wrapped errors (typically *os.PathError), not the raw os.ErrNotExist, so direct equality comparison will always be false even when the file doesn't exist.

Modern best practice (Go 1.13+):

if errors.Is(err, fs.ErrNotExist) {
    // cgroup v1: file doesn't exist
    resources.OomKillDisable = boolPtr(true)
}

Also acceptable (legacy helper):

if os.IsNotExist(err) {
    // cgroup v1: file doesn't exist
    resources.OomKillDisable = boolPtr(true)
}

Both errors.Is() and os.IsNotExist() properly unwrap the error to check the underlying condition, while direct comparison does not. The Go documentation for os.IsNotExist explicitly recommends using errors.Is(err, fs.ErrNotExist) for new code.

Would you prefer errors.Is(err, fs.ErrNotExist) (modern approach) or os.IsNotExist(err) (simpler, still works)?