Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion src/apps/containers.clj
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
container-devices
container-volumes
container-volumes-from
container-gpu-models
data-containers
interapps-proxy-settings
ports]]
[apps.persistence.tools :refer [update-tool]]
[apps.util.assertions :refer [assert-not-nil]]
[apps.util.config :as config]
[apps.util.conversions :refer [remove-nil-vals remove-empty-vals]]
[apps.util.db :refer [transaction]]
[apps.validation :refer [validate-image-not-used-in-public-apps
Expand Down Expand Up @@ -160,6 +162,36 @@
(select-keys volume-map [:host_path :container_path])
{:container_settings_id (uuidify settings-uuid)}))))

(defn- gpu-model-mapping?
"Returns true if the combination of container_settings UUID and GPU model
already exists in the database."
[settings-uuid gpu-model]
(pos? (count (sql/select container-gpu-models
(sql/where (and (= :container_settings_id (uuidify settings-uuid))
(= :gpu_model gpu-model)))))))

(defn- validate-gpu-model
"Validates that a GPU model is in the list of valid GPU models."
[gpu-model]
(let [valid-models (set (config/valid-gpu-models))]
(when-not (contains? valid-models gpu-model)
(cxu/bad-request (str "Invalid GPU model: " gpu-model ". Valid models are: " (clojure.string/join ", " valid-models))))))

(defn- add-gpu-model
"Associates a GPU model with the given container_settings UUID."
[settings-uuid gpu-model]
(validate-gpu-model gpu-model)
(when (gpu-model-mapping? settings-uuid gpu-model)
(cxu/exists (str "GPU model mapping already exists: " settings-uuid " " gpu-model)))
(sql/insert container-gpu-models
(sql/values {:container_settings_id (uuidify settings-uuid)
:gpu_model gpu-model})))

(defn list-valid-gpu-models
"Returns the list of valid GPU models that can be configured."
[]
{:gpu_models (vec (config/valid-gpu-models))})

(defn- data-container
"Returns a map describing a data container."
[data-container-id]
Expand Down Expand Up @@ -331,12 +363,15 @@
(sql/fields :name_prefix :read_only)
(sql/with container-images
(sql/fields :name :tag :url :deprecated :osg_image_path))))
(sql/with container-gpu-models
(sql/fields :gpu_model))
(sql/with ports
(sql/fields :host_port :container_port :bind_to_host :id))
(sql/where {:tools_id id}))
first
add-interapps-info
(update :container_volumes_from add-data-container-auth :auth? auth?)
(update :gpu_models #(mapv :gpu_model %))
(merge {:image (tool-image-info tool-uuid :auth? auth?)})
filter-returns))))

Expand Down Expand Up @@ -394,6 +429,7 @@
volumes (:container_volumes info-map)
vfs (:container_volumes_from info-map)
ports (:container_ports info-map)
gpu-models (:gpu_models info-map)
proxy-settings (:interactive_apps info-map)
info-map (assoc info-map :tools_id (uuidify tool-uuid))]
(log/warn "adding container information for tool" tool-uuid ":" info-map)
Expand All @@ -414,13 +450,15 @@
(add-settings-volumes-from settings-uuid vf))
(doseq [p ports]
(add-port settings-uuid p))
(doseq [gm gpu-models]
(add-gpu-model settings-uuid gm))
(tool-container-info tool-uuid)))))

(defn set-tool-container
"Removes all existing container settings for the given tool-id, replacing them with the given settings."
[tool-id
overwrite-public
{:keys [container_devices container_volumes container_volumes_from container_ports interactive_apps] :as settings}]
{:keys [container_devices container_volumes container_volumes_from container_ports gpu_models interactive_apps] :as settings}]
(when-not overwrite-public
(validate-tool-not-used-in-public-apps tool-id))
(transaction
Expand All @@ -441,5 +479,7 @@
(add-settings-volumes-from settings-id volume))
(doseq [port container_ports]
(add-port settings-id port))
(doseq [gpu-model gpu_models]
(add-gpu-model settings-id gpu-model))

(tool-container-info tool-id))))
34 changes: 21 additions & 13 deletions src/apps/persistence/app_metadata.clj
Original file line number Diff line number Diff line change
Expand Up @@ -1076,19 +1076,27 @@

(defn get-resource-requirements-for-task
[task-id]
(-> (select* [entities/container-settings :c])
(join [:tasks :t] {:t.tool_id :c.tools_id})
(fields :memory_limit
:min_memory_limit
:min_cpu_cores
:max_cpu_cores
:min_gpus
:max_gpus
:min_disk_space)
(where {:t.id task-id})
select
first
remove-nil-vals))
(when-let [reqs (-> (select* [entities/container-settings :c])
(join [:tasks :t] {:t.tool_id :c.tools_id})
(fields :c.id
:memory_limit
:min_memory_limit
:min_cpu_cores
:max_cpu_cores
:min_gpus
:max_gpus
:min_disk_space)
(where {:t.id task-id})
select
first
remove-nil-vals)]
(let [gpu-models (mapv :gpu_model
(select entities/container-gpu-models
(fields :gpu_model)
(where {:container_settings_id (:id reqs)})))]
(-> reqs
(dissoc :id)
(cond-> (seq gpu-models) (assoc :gpu_models gpu-models))))))

(defn list-app-publication-requests
[app-id requestor include-completed]
Expand Down
8 changes: 7 additions & 1 deletion src/apps/persistence/entities.clj
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
container-settings
container-devices
container-volumes
container-volumes-from)
container-volumes-from
container-gpu-models)

;; Users who have logged into the DE. Multiple entities are associated with
;; the same table in order to allow us to have multiple relationships between
Expand Down Expand Up @@ -129,6 +130,7 @@
(sql/has-many container-devices)
(sql/has-many container-volumes)
(sql/has-many container-volumes-from)
(sql/has-many container-gpu-models)
(sql/has-many ports)
(sql/has-one interapps-proxy-settings))

Expand All @@ -145,6 +147,10 @@
(sql/belongs-to container-settings)
(sql/belongs-to data-containers))

(sql/defentity container-gpu-models
(sql/table :container_gpu_models :gpu_models)
(sql/belongs-to container-settings))

;; Information about a deployed tool.
(sql/defentity tools
(sql/belongs-to integration_data)
Expand Down
6 changes: 5 additions & 1 deletion src/apps/routes.clj
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
{:name "pipelines", :description "Pipeline endpoints."}
{:name "analyses", :description "Analysis endpoints."}
{:name "bootstrap", :description "Bootstrap endpoints."}
{:name "gpu-models", :description "GPU Model Configuration endpoints."}
{:name "tools", :description "Tool endpoints."}
{:name "workspaces", :description "Workspace endpoints."}
{:name "webhooks", :description "Webhooks endpoints."}
Expand All @@ -74,7 +75,7 @@
{:name "admin-ontologies", :description "Admin App Ontology endpoints."}
{:name "admin-container-images", :description "Admin Tool Docker Images endpoints."}
{:name "admin-data-containers", :description "Admin Docker Data Container endpoints."}
{:name "admin-tools", :description "Admin Tool endpoints."}
{:name "admin-tools", :description "Admin Tool endpoints."}
Comment thread
ianmcorvidae marked this conversation as resolved.
Outdated
{:name "admin-reference-genomes", :description "Admin Reference Genome endpoints."}
{:name "admin-tool-requests", :description "Admin Tool Request endpoints."}
{:name "admin-oauth", :description "Admin OAuth endpoints."}
Expand Down Expand Up @@ -132,6 +133,9 @@
(schema/context "/bootstrap" []
:tags ["bootstrap"]
bootstrap-routes/bootstrap)
(schema/context "/tools/gpu-models" []
:tags ["gpu-models"]
tool-routes/gpu-models)
(schema/context "/tools" []
:tags ["tools"]
tool-routes/tools)
Expand Down
11 changes: 11 additions & 0 deletions src/apps/routes/schemas/containers.clj
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,14 @@
(->optional-param :name)
(dissoc :id))
"A map for updating data container settings."))

(s/defschema GpuModel
(describe
s/Str
"A GPU model name (e.g., 'NVIDIA-A16')."))

(s/defschema GpuModels
(describe
{:gpu_models [GpuModel]}
"A list of valid GPU model names."))

11 changes: 11 additions & 0 deletions src/apps/routes/tools.clj
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@
image-info
image-public-tools
list-images
list-valid-gpu-models
modify-data-container
modify-image-info]]
[apps.metadata.tool-requests :as tool-requests]
[apps.routes.params :refer [SecuredQueryParams SecuredQueryParamsRequired ToolSearchParams]]
[apps.routes.schemas.containers
:refer [DataContainerIdParam
DataContainerUpdateRequest
GpuModel
Comment thread
ianmcorvidae marked this conversation as resolved.
Outdated
GpuModels
ImageId
ImageUpdateParams
ImageUpdateRequest
Expand Down Expand Up @@ -120,6 +123,14 @@
:description "Updates a data container's settings."
(ok (modify-data-container data-container-id body))))

(defroutes gpu-models
(GET "/" []
:query [params SecuredQueryParams]
:return GpuModels
:summary "List Valid GPU Models"
:description "Returns the list of valid GPU model names that can be configured for tools."
(ok (list-valid-gpu-models))))

(defroutes tools
(GET "/" []
:query [params ToolSearchParams]
Expand Down
13 changes: 8 additions & 5 deletions src/apps/service/apps/de/job_view.clj
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@
(defn- format-step-resource-requirements
[requirements step-number add-defaults?]
(if add-defaults?
(merge {:max_cpu_cores (config/default-cpu-limit)
:memory_limit (config/default-memory-limit)
:max_gpus (config/default-gpu-limit)
:step_number step-number}
requirements)
(let [defaults {:max_cpu_cores (config/default-cpu-limit)
:memory_limit (config/default-memory-limit)
:max_gpus (config/default-gpu-limit)
:step_number step-number}
merged (merge defaults requirements)]
(if (and (not (empty? (config/default-gpu-models))) (empty? (:gpu_models merged)))
(assoc merged :gpu_models (vec (config/default-gpu-models)))
merged))
(assoc requirements :step_number step-number)))

(defn- get-step-resource-requirements
Expand Down
16 changes: 15 additions & 1 deletion src/apps/service/apps/de/jobs/common.clj
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
[apps.tools :as t]
[apps.util.assertions :refer [assert-not-nil]]
[apps.util.conversions :refer [remove-nil-vals]]
[clojure.set :as set]
[clojure.string :as string]
[kameleon.uuids :refer [uuid]]
[korma.core :refer [fields join order select select* where]]
Expand Down Expand Up @@ -54,6 +55,18 @@
(mapcat (partial build-environment-entries config default-values))
(into {})))

(defn- reconcile-gpu-models
"Returns the effective GPU model list for a job step.
Filters user-requested models to only those in the tool's allowed set.
Uses the filtered user subset if non-empty, otherwise falls back to
the tool's full list. Returns nil if the tool has no GPU models."
[container requirements]
(let [tool-models (set (:gpu_models container))
user-models (set (:gpu_models requirements))
valid-user (set/intersection user-models tool-models)]
(when (seq tool-models)
(vec (if (seq valid-user) valid-user tool-models)))))

(defn- reconcile-container-requirements
"reconcile submission requirement requests with tool requirements"
[container requirements]
Expand All @@ -64,7 +77,8 @@
:max_cpu_cores (resources/get-max-cpus container requirements)
:min_gpus (resources/get-required-gpus container requirements)
:max_gpus (resources/get-max-gpus container requirements)
:min_disk_space (resources/get-required-disk-space container requirements))
:min_disk_space (resources/get-required-disk-space container requirements)
:gpu_models (reconcile-gpu-models container requirements))
remove-nil-vals))

(defn- add-container-info
Expand Down
14 changes: 8 additions & 6 deletions src/apps/service/apps/jobs/params.clj
Original file line number Diff line number Diff line change
Expand Up @@ -157,17 +157,19 @@
(merge step-reqs
(-> requested-step-reqs
(select-keys [:max_cpu_cores
:min_cpu_cores
:min_gpus
:max_gpus
:min_memory_limit
:min_disk_space])
:min_cpu_cores
:min_gpus
:max_gpus
:min_memory_limit
:min_disk_space
:gpu_models])
Comment thread
ianmcorvidae marked this conversation as resolved.
Outdated
(sets/rename-keys {:max_cpu_cores :default_max_cpu_cores
:min_cpu_cores :default_cpu_cores
:min_gpus :default_gpus
:max_gpus :default_max_gpus
:min_memory_limit :default_memory
:min_disk_space :default_disk_space})))))
:min_disk_space :default_disk_space
:gpu_models :default_gpu_models})))))

(defn- update-resources-reqs
"Converts resource requests from the original submission JSON into default requirement settings,
Expand Down
11 changes: 7 additions & 4 deletions src/apps/tools.clj
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,13 @@
(defn format-container-settings
[container-settings include-defaults]
(if include-defaults
(merge {:max_cpu_cores (config/default-cpu-limit)
:memory_limit (config/default-memory-limit)
:max_gpus (config/default-gpu-limit)}
container-settings)
(let [defaults {:max_cpu_cores (config/default-cpu-limit)
:memory_limit (config/default-memory-limit)
:max_gpus (config/default-gpu-limit)}
merged (merge defaults container-settings)]
(if (and (not (empty? (config/default-gpu-models))) (empty? (:gpu_models merged)))
(assoc merged :gpu_models (vec (config/default-gpu-models)))
merged))
container-settings))

(defn- filter-listing-tool-ids
Expand Down
10 changes: 10 additions & 0 deletions src/apps/util/config.clj
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,16 @@
[props config-valid configs]
"apps.tools.default.gpu-limit" 0)

(cc/defprop-optvec valid-gpu-models
"The list of valid GPU model names that can be configured for tools."
[props config-valid configs]
"apps.tools.valid-gpu-models" ["NVIDIA-A16"])

(cc/defprop-optvec default-gpu-models
"The default GPU models to use when a tool does not specify any and defaults are requested."
[props config-valid configs]
"apps.tools.default.gpu-models" [])

(cc/defprop-optstr workspace-root-app-category
"The name of the root app category in a user's workspace."
[props config-valid configs]
Expand Down
3 changes: 3 additions & 0 deletions test.properties
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ apps.batch.path-list.info-type = ht-analysis-path-list
apps.batch.path-list.max-paths = 16
apps.batch.path-list.max-size = 1048576

# Tool settings.
apps.tools.valid-gpu-models = ["NVIDIA-A16", "NVIDIA-A100-SXM4-40GB", "NVIDIA-A100-SXM4-80GB"]

# Agave connection settings.
apps.agave.base-url = https://agave.iplantc.org
apps.agave.key = not_a_key
Expand Down
21 changes: 21 additions & 0 deletions test/apps/service/apps/de/job_view_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,24 @@
(some? (:max_gpus result))
(not (contains? requirements :max_gpus))))
"Should not add max_gpus default when add-defaults is false"))))

;; ---------------------------------------------------------------------------
;; GPU model preservation and non-injection
;; ---------------------------------------------------------------------------

(deftest test-format-step-resource-requirements-preserves-gpu-models
(testing "Existing gpu_models in requirements are preserved by format-step-resource-requirements"
(let [requirements {:gpu_models ["A16" "A40"]
:max_gpus 2}
step-number 1
result (#'job-view/format-step-resource-requirements requirements step-number true)]
(is (= ["A16" "A40"] (:gpu_models result))
"Should preserve the tool's gpu_models list when present"))))

(deftest test-format-step-resource-requirements-no-gpu-models-when-not-interactive
(testing "gpu_models not injected when add-defaults is false, even if absent"
(let [requirements {:max_gpus 2}
step-number 1
result (#'job-view/format-step-resource-requirements requirements step-number false)]
(is (not (contains? result :gpu_models))
"Should not inject gpu_models when add-defaults is false"))))
Loading