Skip to content

Commit 27cd21e

Browse files
committed
oomwatch: small tweaks
- Change memory usage percent threshold to `uint8` to no longer allow fractions. - Validate interval to prevent configurations `<50ms`. Signed-off-by: Hidde Beydals <hidde@hhh.computer>
1 parent 3dbb013 commit 27cd21e

File tree

3 files changed

+26
-12
lines changed

3 files changed

+26
-12
lines changed

internal/oomwatch/watch.go

+10-6
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ type Watcher struct {
4747
memoryCurrentPath string
4848
// memoryUsagePercentThreshold is the threshold at which the system is
4949
// considered to be near OOM.
50-
memoryUsagePercentThreshold float64
50+
memoryUsagePercentThreshold uint8
5151
// interval is the interval at which to check for OOM.
5252
interval time.Duration
5353
// logger is the logger to use.
@@ -62,9 +62,13 @@ type Watcher struct {
6262
}
6363

6464
// New returns a new Watcher.
65-
func New(memoryMaxPath, memoryCurrentPath string, memoryUsagePercentThreshold float64, interval time.Duration, logger logr.Logger) (*Watcher, error) {
65+
func New(memoryMaxPath, memoryCurrentPath string, memoryUsagePercentThreshold uint8, interval time.Duration, logger logr.Logger) (*Watcher, error) {
6666
if memoryUsagePercentThreshold < 1 || memoryUsagePercentThreshold > 100 {
67-
return nil, fmt.Errorf("memory usage percent threshold must be between 1 and 100, got %.2f", memoryUsagePercentThreshold)
67+
return nil, fmt.Errorf("memory usage percent threshold must be between 1 and 100, got %d", memoryUsagePercentThreshold)
68+
}
69+
70+
if minInterval := 50 * time.Millisecond; interval < minInterval {
71+
return nil, fmt.Errorf("interval must be at least %s, got %s", minInterval, interval)
6872
}
6973

7074
if _, err := os.Lstat(memoryCurrentPath); err != nil {
@@ -86,7 +90,7 @@ func New(memoryMaxPath, memoryCurrentPath string, memoryUsagePercentThreshold fl
8690
}
8791

8892
// NewDefault returns a new Watcher with default path values.
89-
func NewDefault(memoryUsagePercentThreshold float64, interval time.Duration, logger logr.Logger) (*Watcher, error) {
93+
func NewDefault(memoryUsagePercentThreshold uint8, interval time.Duration, logger logr.Logger) (*Watcher, error) {
9094
return New(
9195
filepath.Join(DefaultCgroupPath, MemoryMaxFile),
9296
filepath.Join(DefaultCgroupPath, MemoryCurrentFile),
@@ -128,13 +132,13 @@ func (w *Watcher) watchForNearOOM(ctx context.Context) {
128132
}
129133

130134
currentPercentage := float64(current) / float64(w.memoryMax) * 100
131-
if currentPercentage >= w.memoryUsagePercentThreshold {
135+
if currentPercentage >= float64(w.memoryUsagePercentThreshold) {
132136
w.logger.Info(fmt.Sprintf("Memory usage is near OOM (%s/%s), shutting down",
133137
formatSize(current), formatSize(w.memoryMax)))
134138
w.cancel()
135139
return
136140
}
137-
w.logger.V(2).Info(fmt.Sprintf("Current memory usage %s/%s (%.2f%% out of %.2f%%)",
141+
w.logger.V(2).Info(fmt.Sprintf("Current memory usage %s/%s (%.2f%% out of %d%%)",
138142
formatSize(current), formatSize(w.memoryMax), currentPercentage, w.memoryUsagePercentThreshold))
139143
}
140144
}

internal/oomwatch/watch_test.go

+14-4
Original file line numberDiff line numberDiff line change
@@ -57,22 +57,32 @@ func TestNew(t *testing.T) {
5757

5858
_, err := New("", "", 0, 0, logr.Discard())
5959
g.Expect(err).To(HaveOccurred())
60-
g.Expect(err).To(MatchError("memory usage percent threshold must be between 1 and 100, got 0.00"))
60+
g.Expect(err).To(MatchError("memory usage percent threshold must be between 1 and 100, got 0"))
6161
})
6262
t.Run("greater than 100", func(t *testing.T) {
6363
g := NewWithT(t)
6464

6565
_, err := New("", "", 101, 0, logr.Discard())
6666
g.Expect(err).To(HaveOccurred())
67-
g.Expect(err).To(MatchError("memory usage percent threshold must be between 1 and 100, got 101.00"))
67+
g.Expect(err).To(MatchError("memory usage percent threshold must be between 1 and 100, got 101"))
68+
})
69+
})
70+
71+
t.Run("interval", func(t *testing.T) {
72+
t.Run("less than 50ms", func(t *testing.T) {
73+
g := NewWithT(t)
74+
75+
_, err := New("", "", 1, 49*time.Millisecond, logr.Discard())
76+
g.Expect(err).To(HaveOccurred())
77+
g.Expect(err).To(MatchError("interval must be at least 50ms, got 49ms"))
6878
})
6979
})
7080

7181
t.Run("memory current path", func(t *testing.T) {
7282
t.Run("does not exist", func(t *testing.T) {
7383
g := NewWithT(t)
7484

75-
_, err := New("", "", 1, 0, logr.Discard())
85+
_, err := New("", "", 1, 50*time.Second, logr.Discard())
7686
g.Expect(err).To(HaveOccurred())
7787
g.Expect(err.Error()).To(ContainSubstring("failed to stat memory.current \"\": lstat : no such file or directory"))
7888
})
@@ -86,7 +96,7 @@ func TestNew(t *testing.T) {
8696
_, err := os.Create(mockMemoryCurrent)
8797
g.Expect(err).NotTo(HaveOccurred())
8898

89-
_, err = New("", mockMemoryCurrent, 1, 0, logr.Discard())
99+
_, err = New("", mockMemoryCurrent, 1, 50*time.Second, logr.Discard())
90100
g.Expect(err).To(HaveOccurred())
91101
g.Expect(err.Error()).To(ContainSubstring("failed to read memory.max \"\": open : no such file or directory"))
92102
})

main.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func main() {
8686
leaderElectionOptions leaderelection.Options
8787
rateLimiterOptions helper.RateLimiterOptions
8888
oomWatchInterval time.Duration
89-
oomWatchMemoryThreshold float64
89+
oomWatchMemoryThreshold uint8
9090
)
9191

9292
flag.StringVar(&metricsAddr, "metrics-addr", ":8080",
@@ -107,7 +107,7 @@ func main() {
107107
"The maximum number of retries when failing to fetch artifacts over HTTP.")
108108
flag.StringVar(&intkube.DefaultServiceAccountName, "default-service-account", "",
109109
"Default service account used for impersonation.")
110-
flag.Float64Var(&oomWatchMemoryThreshold, "oom-watch-memory-threshold", 95,
110+
flag.Uint8Var(&oomWatchMemoryThreshold, "oom-watch-memory-threshold", 95,
111111
"The memory threshold in percentage at which the OOM watcher will trigger a graceful shutdown. Requires feature gate 'OOMWatch' to be enabled.")
112112
flag.DurationVar(&oomWatchInterval, "oom-watch-interval", 500*time.Millisecond,
113113
"The interval at which the OOM watcher will check for memory usage. Requires feature gate 'OOMWatch' to be enabled.")

0 commit comments

Comments
 (0)