-
Notifications
You must be signed in to change notification settings - Fork 8
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #279 from slr71/main
CORE-2024: memory and cpu request limit fixes
- Loading branch information
Showing
3 changed files
with
153 additions
and
39 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| (ns apps.service.apps.de.jobs.resources | ||
| (:require [apps.util.config :as config])) | ||
|
|
||
| (defn- get-min-resource-setting | ||
| "Determines the minimum amount of a specific resource required for the container." | ||
| [min-kw max-kw default container requirements] | ||
| (let [c-min (get container min-kw) | ||
| c-max (get container max-kw) | ||
| r-min (get requirements min-kw) | ||
| r-max (get requirements max-kw)] | ||
| (cond | ||
| ;; A nonexisent or nonsensical request defaults to the minimum defined for the container, which may be nil. | ||
| (or (nil? r-min) (zero? r-min) (neg? r-min)) | ||
| c-min | ||
|
|
||
| ;; If the requested minimum is less than the container minimum, default to the container's minimum setting. | ||
| (and c-min (< r-min c-min)) | ||
| c-min | ||
|
|
||
| ;; In all other cases, the requested minimum must not be greater than the requested or container maximum. | ||
| :else | ||
| (apply min (filter pos? (remove nil? [r-min r-max (or c-max default)])))))) | ||
|
|
||
| (defn- get-max-resource-setting | ||
| "Determines the maximum amount of a specific resource required for the container." | ||
| [max-kw default min-resource-setting-fn container requirements] | ||
| (let [c-max (get container max-kw) | ||
| r-max (get requirements max-kw)] | ||
| (cond | ||
| ;; If the requested maximum wasn't specified, use the requested minimum. | ||
| (or (nil? r-max) (zero? r-max) (neg? r-max)) | ||
| (min-resource-setting-fn container requirements) | ||
|
|
||
| ;; If the requested maximum was specified, it must not be greater than the container maximum. | ||
| :else | ||
| (min (or c-max default) r-max)))) | ||
|
|
||
| (defn get-required-memory | ||
| "Determines the minimum amount of memory required for the container." | ||
| [container requirements] | ||
| (get-min-resource-setting :min_memory_limit :memory_limit (config/default-memory-limit) container requirements)) | ||
|
|
||
| (defn get-max-memory | ||
| "Determines the maximum amount of memory that may be consumed by the app." | ||
| [container requirements] | ||
| (get-max-resource-setting :memory_limit (config/default-memory-limit) get-required-memory container requirements)) | ||
|
|
||
| (defn get-required-cpus | ||
| "Determines the minimum number of CPUs required for the container." | ||
| [container requirements] | ||
| (get-min-resource-setting :min_cpu_cores :max_cpu_cores (config/default-cpu-limit) container requirements)) | ||
|
|
||
| (defn get-max-cpus | ||
| "Determines the maximum amount of CPUs that may be used by the app." | ||
| [container requirements] | ||
| (get-max-resource-setting :max_cpu_cores (config/default-cpu-limit) get-required-cpus container requirements)) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| (ns apps.service.apps.resources-test | ||
| (:require [apps.service.apps.de.jobs.resources :as r] | ||
| [clojure.test :refer [deftest] :as t])) | ||
|
|
||
| (def gib (* 1024 1024 1024)) | ||
|
|
||
| (defn reqs | ||
| [min-mem max-mem min-cpu max-cpu] | ||
| {:min_memory_limit (when min-mem (* min-mem gib)) | ||
| :memory_limit (when max-mem (* max-mem gib)) | ||
| :min_cpu_cores min-cpu | ||
| :max_cpu_cores max-cpu}) | ||
|
|
||
| (def test-cases | ||
| [{:container (reqs 2 256 2 256) | ||
| :requirements (reqs 4 32 4 16) | ||
| :expected (reqs 4 32 4 16) | ||
| :desc "all values specified and within range"} | ||
| {:container (reqs 2 256 2 256) | ||
| :requirements (reqs nil 32 4 16) | ||
| :expected (reqs 2 32 4 16) | ||
| :desc "unspecified minimum memory setting"} | ||
| {:container (reqs 2 256 2 256) | ||
| :requirements (reqs 4 nil 4 16) | ||
| :expected (reqs 4 4 4 16) | ||
| :desc "unspecified maximum memory setting"} | ||
| {:container (reqs 2 256 2 256) | ||
| :requirements (reqs 4 32 nil 16) | ||
| :expected (reqs 4 32 2 16) | ||
| :desc "unspecified min cpu setting"} | ||
| {:container (reqs 2 256 2 256) | ||
| :requirements (reqs 4 32 4 nil) | ||
| :expected (reqs 4 32 4 4) | ||
| :desc "unspecified max cpu setting"} | ||
| {:container (reqs nil 256 2 256) | ||
| :requirements (reqs 4 32 4 16) | ||
| :expected (reqs 4 32 4 16) | ||
| :desc "unspecified min memory setting in container"} | ||
| {:container (reqs 2 nil 2 256) | ||
| :requirements (reqs 4 32 4 16) | ||
| :expected (reqs 4 16 4 16) | ||
| :desc "unspecified max memory setting in container, request higher than overall default"} | ||
| {:container (reqs 2 256 nil 256) | ||
| :requirements (reqs 4 32 4 16) | ||
| :expected (reqs 4 32 4 16) | ||
| :desc "unspecified min cpu setting in container"} | ||
| {:container (reqs 2 256 2 nil) | ||
| :requirements (reqs 4 32 4 16) | ||
| :expected (reqs 4 32 4 4) | ||
| :desc "unspecified max cpu setting in container, request higher than overall default"} | ||
| {:container (reqs 2 256 2 256) | ||
| :requirements (reqs 1 32 1 16) | ||
| :expected (reqs 2 32 2 16) | ||
| :desc "minimum requests less than container minimums"} | ||
| {:container (reqs 2 256 2 256) | ||
| :requirements (reqs 4 512 4 512) | ||
| :expected (reqs 4 256 4 256) | ||
| :desc "maximum requests greater than container maximums"} | ||
| {:container (reqs 2 256 2 256) | ||
| :requirements (reqs 4 nil 4 nil) | ||
| :expected (reqs 4 4 4 4) | ||
| :desc "maximum requests not specified"} | ||
| {:container (reqs 2 256 2 256) | ||
| :requirements (reqs nil nil nil nil) | ||
| :expected (reqs 2 2 2 2) | ||
| :desc "no requests specified"} | ||
| {:container (reqs nil nil nil nil) | ||
| :requirements (reqs 32 32 32 32) | ||
| :expected (reqs 16 16 4 4) | ||
| :desc "no container settings specified"} | ||
| {:container (reqs nil 256 nil 256) | ||
| :requirements (reqs nil nil nil nil) | ||
| :expected (reqs nil nil nil nil) | ||
| :desc "no requests specified, and no container minimums specified"}]) | ||
|
|
||
| (deftest test-resource-requests | ||
| (doseq [{:keys [container requirements expected desc]} test-cases] | ||
| (t/testing desc | ||
| (t/is (= (:min_memory_limit expected) (r/get-required-memory container requirements))) | ||
| (t/is (= (:memory_limit expected) (r/get-max-memory container requirements))) | ||
| (t/is (= (:min_cpu_cores expected) (r/get-required-cpus container requirements))) | ||
| (t/is (= (:max_cpu_cores expected) (r/get-max-cpus container requirements)))))) |