Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helm: add deployment strategy to chart #27048

Merged
merged 3 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ metadata:
annotations: {{ $deployment.annotations | toYaml | nindent 4 }}
spec:
replicas: 1
{{- if $deployment.deploymentStrategy }}
strategy:
{{- toYaml $deployment.deploymentStrategy | nindent 4 }}
{{- end }}
selector:
matchLabels:
{{- include "dagster.selectorLabels" $ | nindent 6 }}
Expand Down
18 changes: 18 additions & 0 deletions helm/dagster/charts/dagster-user-deployments/values.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions helm/dagster/charts/dagster-user-deployments/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ deployments:
startupProbe:
enabled: false

# Strategy to follow when replacing old pods with new pods. See:
# https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy
deploymentStrategy: {}

service:
annotations: {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class UserDeployment(BaseModel):
schedulerName: Optional[str] = None
initContainers: Optional[list[kubernetes.Container]] = None
sidecarContainers: Optional[list[kubernetes.Container]] = None
deploymentStrategy: Optional[kubernetes.DeploymentStrategy] = None


class UserDeployments(BaseModel):
Expand Down
9 changes: 9 additions & 0 deletions helm/dagster/schema/schema/charts/utils/kubernetes.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,12 @@ class Container(BaseModel):
"extra": "allow",
"json_schema_extra": {"$ref": create_definition_ref("io.k8s.api.core.v1.Container")},
}


class DeploymentStrategy(BaseModel):
model_config = {
"extra": "allow",
"json_schema_extra": {
"$ref": create_definition_ref("io.k8s.api.apps.v1.DeploymentStrategy")
},
}
40 changes: 39 additions & 1 deletion helm/dagster/schema/schema_tests/test_user_deployments.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import json
import subprocess
from typing import Union
from typing import Any, Optional, Union

import pytest
from dagster_k8s.models import k8s_model_from_dict, k8s_snake_case_dict
Expand Down Expand Up @@ -1392,3 +1392,41 @@ def test_old_env(template: HelmTemplate, user_deployment_configmap_template):

[cm] = user_deployment_configmap_template.render(helm_values)
assert cm.data["test_env"] == "test_value"


@pytest.mark.parametrize(
"strategy,expected",
[
(None, None),
({}, None),
(
{
"type": "RollingUpdate",
"rollingUpdate": {"maxSurge": 10, "maxUnavailable": 1},
},
{
"type": "RollingUpdate",
"rolling_update": {"max_surge": 10, "max_unavailable": 1},
},
),
(
{"type": "Recreate"},
{"type": "Recreate", "rolling_update": None},
),
],
)
def test_deployment_strategy(
template: HelmTemplate,
strategy: Optional[dict[str, Any]],
expected: Optional[dict[str, Any]],
):
deployment = create_simple_user_deployment("foo")
if strategy:
deployment.deploymentStrategy = kubernetes.DeploymentStrategy.model_construct(**strategy)
helm_values = DagsterHelmValues.construct(
dagsterUserDeployments=UserDeployments.model_construct(deployments=[deployment])
)

dagster_user_deployment = template.render(helm_values)
assert len(dagster_user_deployment) == 1
assert dagster_user_deployment[0].to_dict()["spec"]["strategy"] == expected
18 changes: 18 additions & 0 deletions helm/dagster/values.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions helm/dagster/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,10 @@ dagster-user-deployments:
startupProbe:
enabled: false

# Strategy to follow when replacing old pods with new pods. See:
# https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy
deploymentStrategy: {}

service:
annotations: {}

Expand Down