diff --git a/.credo.exs b/.credo.exs index da31610..63bc4dd 100644 --- a/.credo.exs +++ b/.credo.exs @@ -130,7 +130,6 @@ {Credo.Check.Refactor.MatchInCondition, []}, {Credo.Check.Refactor.NegatedConditionsInUnless, []}, {Credo.Check.Refactor.NegatedConditionsWithElse, []}, - {Credo.Check.Refactor.Nesting, []}, {Credo.Check.Refactor.RedundantWithClauseResult, []}, {Credo.Check.Refactor.RejectReject, []}, {Credo.Check.Refactor.UnlessWithElse, []}, @@ -194,6 +193,7 @@ {Credo.Check.Refactor.MapMap, []}, {Credo.Check.Refactor.ModuleDependencies, []}, {Credo.Check.Refactor.NegatedIsNil, []}, + {Credo.Check.Refactor.Nesting, []}, {Credo.Check.Refactor.PassAsyncInTestCases, []}, {Credo.Check.Refactor.PipeChainStart, []}, {Credo.Check.Refactor.RejectFilter, []}, diff --git a/CHANGELOG.md b/CHANGELOG.md index 36cfb33..a4d8493 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,22 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [0.5.0] - 2024-08-27 + +### Fixed + +- `FLAMEK8sBackend.RunnerPodTemplate`: Only set `PHX_SERVER` if it is not passed. +- `FLAMEK8sBackend.RunnerPodTemplate`: Reject `FLAME_PARENT`, not `FLAME_BACKEND` in passed env vars. + ### Added - `FLAMEK8sBackend.RunnerPodTemplate`: Set `.metadata.namespace` and `.metadata.generateName` on runner pod if not set ([#43](https://github.com/mruoss/flame_k8s_backend/pull/43)) +### Changed + +- `FLAMEK8sBackend.RunnerPodTemplate`: Also copy `env_from` if `add_parent_env` is `true` +- Improve documentation + ## [0.4.3] - 2024-08-22 ### Fixed diff --git a/lib/flame_k8s_backend/runner_pod_template.ex b/lib/flame_k8s_backend/runner_pod_template.ex index 085a7ac..e6f0094 100644 --- a/lib/flame_k8s_backend/runner_pod_template.ex +++ b/lib/flame_k8s_backend/runner_pod_template.ex @@ -101,39 +101,6 @@ defmodule FLAMEK8sBackend.RunnerPodTemplate do end ``` - ### Advanced Use Case - Passing a Callback Function - - In some cases you might need advanced control over the runner pod manifest. - Maybe you want to set node affinity because you need your runners to run on - nodes with GPUs or you need additional volumes etc. In this case, you can pass - a callback via `runner_pod_tpl` option to the `FLAMEK8sBackend`. - - The callback has to be of type - `t:FLAMEK8sBackend.RunnerPodTemplate.callback/0`. The callback will be called - with the manifest of the parent pod which can be used to extract information. - It should return a pod template as a map - - Define a callback, e.g. in a separate module: - - ``` - defmodule MyApp.FLAMERunnerPodTemplate do - def runner_pod_manifest(parent_pod_manifest) do - %{ - "metadata" => %{ - # namespace, labels, ownerReferences,... - }, - "spec" => %{ - "containers" => [ - %{ - # container definition - } - ] - } - } - end - end - ``` - Register the backend: ``` @@ -162,6 +129,17 @@ defmodule FLAMEK8sBackend.RunnerPodTemplate do > * The container `image` (set to the image of the parent pod's app > container) + > #### Automatically Defined Environment Variables {: .info} + > + > Some environment variables are defined automatically on the + > runner pod: + > + > * `POD_IP` is set to the runner Pod's IP address (`.status.podIP`) - (not overridable) + > * `POD_NAME` is set to the runner Pod's name (`.metadata.name`) - (not overridable) + > * `POD_NAMESPACE` is set to the runner Pod's namespace (`.metadata.namespace`) - (not overridable) + > * `PHX_SERVER` is set to `false` (overridable) + > * `FLAME_PARENT` used internally by FLAME - (not overridable) + ### Options * `:omit_owner_reference` - Omit generating and appending the parent pod as @@ -175,6 +153,20 @@ defmodule FLAMEK8sBackend.RunnerPodTemplate do defstruct [:env, :resources, add_parent_env: true] + @typedoc """ + Describing the Runner Pod Template struct + + ### Fields + + * `env` - a map describing a Pod environment variable declaration + `%{"name" => "MY_ENV_VAR", "value" => "my_env_var_value"}` + + * `resources` - Pod resource requests and limits. + + * `add_parent_env` - If true, all env vars of the main container + including `envFrom` are copied to the runner pod. + default: `true` + """ @type t :: %__MODULE__{ env: map() | nil, resources: map() | nil, @@ -208,10 +200,8 @@ defmodule FLAMEK8sBackend.RunnerPodTemplate do app_container = app_container(parent_pod_manifest, opts) env = template_opts.env || [] - parent_env = - if template_opts.add_parent_env, - do: app_container["env"], - else: [] + parent_env = if template_opts.add_parent_env, do: app_container["env"] + parent_env_from = if template_opts.add_parent_env, do: app_container["envFrom"] runner_pod_template = %{ "metadata" => %{ @@ -221,7 +211,8 @@ defmodule FLAMEK8sBackend.RunnerPodTemplate do "containers" => [ %{ "resources" => template_opts.resources || app_container["resources"], - "env" => env ++ parent_env + "env" => env ++ List.wrap(parent_env), + "envFrom" => List.wrap(parent_env_from) } ] } @@ -272,37 +263,48 @@ defmodule FLAMEK8sBackend.RunnerPodTemplate do |> Map.put("ownerReferences", object_references) end) |> put_in(~w(spec restartPolicy), "Never") - |> put_in(~w(spec serviceAccount), parent_pod_manifest["spec"]["serviceAccount"]) - |> update_in(["spec", "containers", Access.at(0)], fn container -> - container - |> Map.put("image", app_container["image"]) - |> Map.put("name", "runner") - |> Map.put_new("env", []) - |> Map.update!("env", fn env -> - [ - %{ - "name" => "POD_NAME", - "valueFrom" => %{"fieldRef" => %{"fieldPath" => "metadata.name"}} - }, - %{ - "name" => "POD_IP", - "valueFrom" => %{"fieldRef" => %{"fieldPath" => "status.podIP"}} - }, - %{ - "name" => "POD_NAMESPACE", - "valueFrom" => %{"fieldRef" => %{"fieldPath" => "metadata.namespace"}} - }, - %{"name" => "PHX_SERVER", "value" => "false"}, - %{"name" => "FLAME_PARENT", "value" => encoded_parent} - | Enum.reject( - env, - &(&1["name"] in ["FLAME_BACKEND", "POD_NAME", "POD_NAMESPACE"]) - ) - ] + |> update_in(~w(spec), fn spec -> + spec + |> Map.put("restartPolicy", "Never") + |> Map.put_new("serviceAccount", parent_pod_manifest["spec"]["serviceAccount"]) + |> update_in(["containers", Access.at(0)], fn container -> + container + |> Map.put("image", app_container["image"]) + |> Map.put("name", "runner") + |> Map.put_new("env", []) + |> Map.update!("env", fn env -> + [ + %{ + "name" => "POD_NAME", + "valueFrom" => %{"fieldRef" => %{"fieldPath" => "metadata.name"}} + }, + %{ + "name" => "POD_IP", + "valueFrom" => %{"fieldRef" => %{"fieldPath" => "status.podIP"}} + }, + %{ + "name" => "POD_NAMESPACE", + "valueFrom" => %{"fieldRef" => %{"fieldPath" => "metadata.namespace"}} + }, + %{"name" => "FLAME_PARENT", "value" => encoded_parent} + | Enum.reject( + env, + &(&1["name"] in ["FLAME_PARENT", "POD_NAME", "POD_NAMESPACE", "POD_IP"]) + ) + ] + |> put_new_env("PHX_SERVER", "false") + end) end) end) end + defp put_new_env(env, name, value) do + case get_in(env, [Access.filter(&(&1["name"] == name))]) do + [] -> [%{"name" => name, "value" => value} | env] + _ -> env + end + end + defp app_container(parent_pod_manifest, opts) do container_access = case opts[:app_container_name] do diff --git a/test/flame_k8s_backend/runner_pod_template_test.exs b/test/flame_k8s_backend/runner_pod_template_test.exs index f033e49..a29f934 100644 --- a/test/flame_k8s_backend/runner_pod_template_test.exs +++ b/test/flame_k8s_backend/runner_pod_template_test.exs @@ -48,7 +48,8 @@ defmodule FLAMEK8sBackend.RunnerPodTemplateTest do ) end - MUT.manifest(parent_pod_manifest, callback, make_ref()) + pod_manifest = MUT.manifest(parent_pod_manifest, callback, make_ref()) + assert get_in(pod_manifest, env_var_access("PHX_SERVER")) == ["false"] end test "should return pod manifest with data form callback", %{ @@ -64,6 +65,9 @@ defmodule FLAMEK8sBackend.RunnerPodTemplateTest do limits: memory: 500Mi cpu: 500 + env: + - name: PHX_SERVER + value: "true" """ |> put_in( app_container_access(~w(resources requests)), @@ -73,6 +77,7 @@ defmodule FLAMEK8sBackend.RunnerPodTemplateTest do pod_manifest = MUT.manifest(parent_pod_manifest, callback, make_ref()) + assert get_in(pod_manifest, env_var_access("PHX_SERVER")) == ["true"] assert get_in(pod_manifest, app_container_access(~w(resources requests memory))) == "100Mi" assert get_in(pod_manifest, app_container_access(~w(resources limits memory))) == "500Mi" end @@ -182,6 +187,10 @@ defmodule FLAMEK8sBackend.RunnerPodTemplateTest do pod_manifest = MUT.manifest(parent_pod_manifest, template_opts, make_ref()) assert get_in(pod_manifest, env_var_access("RELEASE_NODE")) == ["flame_test@$(POD_IP)"] + + assert get_in(pod_manifest, app_container_access("envFrom")) == [ + %{"configMapRef" => %{"name" => "some-config-map"}} + ] end test "Only default envs if add_parent_env is set to false", %{ @@ -193,6 +202,8 @@ defmodule FLAMEK8sBackend.RunnerPodTemplateTest do assert get_in(pod_manifest, app_container_access(~w(resources requests memory))) == "100Mi" assert get_in(pod_manifest, env_var_access("PHX_SERVER")) == ["false"] + assert get_in(pod_manifest, app_container_access("envFrom")) == [] + parent = flame_parent(pod_manifest) assert parent.ref == ref end @@ -207,6 +218,10 @@ defmodule FLAMEK8sBackend.RunnerPodTemplateTest do assert get_in(pod_manifest, env_var_access("RELEASE_NODE")) == ["flame_test@$(POD_IP)"] assert get_in(pod_manifest, env_var_access("FOO")) == ["bar"] + + assert get_in(pod_manifest, app_container_access("envFrom")) == [ + %{"configMapRef" => %{"name" => "some-config-map"}} + ] end test "No parent envs if add_parent_env is set to false", %{ @@ -217,6 +232,7 @@ defmodule FLAMEK8sBackend.RunnerPodTemplateTest do assert get_in(pod_manifest, env_var_access("RELEASE_NODE")) == [] assert get_in(pod_manifest, env_var_access("FOO")) == ["bar"] + assert get_in(pod_manifest, app_container_access("envFrom")) == [] end end diff --git a/test_support/pods.ex b/test_support/pods.ex index 002724b..03d3fc4 100644 --- a/test_support/pods.ex +++ b/test_support/pods.ex @@ -44,6 +44,9 @@ defmodule FLAMEK8sBackend.TestSupport.Pods do value: name - name: RELEASE_NODE value: flame_test@$(POD_IP) + envFrom: + - configMapRef: + name: some-config-map image: flame-test-image:0.1.0 imagePullPolicy: IfNotPresent name: flame