Skip to content

Commit

Permalink
Adding loggroomer sidecar to dag processor (#30726)
Browse files Browse the repository at this point in the history 
* Adding loggroomer sidecar to dag processor

* Adding log-groomer test code for dagProcessor

* applying pre-commit

---------

Co-authored-by: Hussein Awala <[email protected]>
Co-authored-by: Jed Cunningham <[email protected]>
  • Loading branch information
@wookiist @hussein-awala @jedcunningham
3 people authored May 8, 2023
1 parent ffe3a68 commit 9577113
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 12 deletions.
29 changes: 29 additions & 0 deletions chart/templates/dag-processor/dag-processor-deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,35 @@ spec:
{{- if and (.Values.dags.gitSync.enabled) (not .Values.dags.persistence.enabled) }}
{{- include "git_sync_container" . | indent 8 }}
{{- end }}
{{- if .Values.dagProcessor.logGroomerSidecar.enabled }}
- name: dag-processor-log-groomer
resources: {{- toYaml .Values.dagProcessor.logGroomerSidecar.resources | nindent 12 }}
image: {{ template "airflow_image" . }}
imagePullPolicy: {{ .Values.images.airflow.pullPolicy }}
{{- if .Values.dagProcessor.logGroomerSidecar.command }}
command: {{ tpl (toYaml .Values.dagProcessor.logGroomerSidecar.command) . | nindent 12 }}
{{- end }}
{{- if .Values.dagProcessor.logGroomerSidecar.args }}
args: {{- tpl (toYaml .Values.dagProcessor.logGroomerSidecar.args) . | nindent 12 }}
{{- end }}
{{- if .Values.dagProcessor.logGroomerSidecar.retentionDays }}
env:
- name: AIRFLOW__LOG_RETENTION_DAYS
value: "{{ .Values.dagProcessor.logGroomerSidecar.retentionDays }}"
{{- end }}
volumeMounts:
- name: logs
mountPath: {{ template "airflow_logs" . }}
{{- if .Values.volumeMounts }}
{{- toYaml .Values.volumeMounts | nindent 12 }}
{{- end }}
{{- if .Values.dagProcessor.extraVolumeMounts }}
{{- tpl (toYaml .Values.dagProcessor.extraVolumeMounts) . | nindent 12 }}
{{- end }}
{{- if or .Values.webserver.webserverConfig .Values.webserver.webserverConfigConfigMapName }}
{{- include "airflow_webserver_config_mount" . | nindent 12 }}
{{- end }}
{{- end }}
{{- if .Values.dagProcessor.extraContainers }}
{{- toYaml .Values.dagProcessor.extraContainers | nindent 8 }}
{{- end }}
Expand Down
4 changes: 4 additions & 0 deletions chart/values.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2705,6 +2705,10 @@
}
]
},
"logGroomerSidecar": {
"$ref": "#/definitions/logGroomerConfigType",
"description": "Configuration for log groomer sidecar"
},
"waitForMigrations": {
"description": "wait-for-airflow-migrations init container.",
"type": "object",
Expand Down
17 changes: 17 additions & 0 deletions chart/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1361,6 +1361,23 @@ dagProcessor:

podAnnotations: {}

logGroomerSidecar:
# Whether to deploy the Airflow dag processor log groomer sidecar.
enabled: true
# Command to use when running the Airflow dag processor log groomer sidecar (templated).
command: ~
# Args to use when running the Airflow dag processor log groomer sidecar (templated).
args: ["bash", "/clean-logs"]
# Number of days to retain logs
retentionDays: 15
resources: {}
# limits:
# cpu: 100m
# memory: 128Mi
# requests:
# cpu: 100m
# memory: 128Mi

waitForMigrations:
# Whether to create init container to wait for db migrations
enabled: true
Expand Down
6 changes: 6 additions & 0 deletions tests/charts/airflow_core/test_dag_processor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import pytest

from tests.charts.helm_template_generator import render_chart
from tests.charts.log_groomer import LogGroomerTestBase


class TestDagProcessor:
Expand Down Expand Up @@ -555,3 +556,8 @@ def test_should_add_component_specific_annotations(self):
)
assert "annotations" in jmespath.search("metadata", docs[0])
assert jmespath.search("metadata.annotations", docs[0])["test_annotation"] == "test_annotation_value"


class TestDagProcessorLogGroomer(LogGroomerTestBase):
obj_name = "dag-processor"
folder = "dag-processor"
102 changes: 90 additions & 12 deletions tests/charts/log_groomer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,62 @@ class LogGroomerTestBase:
folder: str = ""

def test_log_groomer_collector_default_enabled(self):
docs = render_chart(show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"])
if self.obj_name == "dag-processor":
values = {"dagProcessor": {"enabled": True}}
else:
values = None

docs = render_chart(
values=values, show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"]
)

assert 2 == len(jmespath.search("spec.template.spec.containers", docs[0]))
assert f"{self.obj_name}-log-groomer" in [
c["name"] for c in jmespath.search("spec.template.spec.containers", docs[0])
]

def test_log_groomer_collector_can_be_disabled(self):
if self.obj_name == "dag-processor":
values = {
"dagProcessor": {
"enabled": True,
"logGroomerSidecar": {"enabled": False},
}
}
else:
values = {f"{self.folder}": {"logGroomerSidecar": {"enabled": False}}}

docs = render_chart(
values={f"{self.folder}": {"logGroomerSidecar": {"enabled": False}}},
values=values,
show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"],
)

actual = jmespath.search("spec.template.spec.containers", docs[0])

assert len(actual) == 1

def test_log_groomer_collector_default_command_and_args(self):
docs = render_chart(show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"])
if self.obj_name == "dag-processor":
values = {"dagProcessor": {"enabled": True}}
else:
values = None

docs = render_chart(
values=values, show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"]
)

assert jmespath.search("spec.template.spec.containers[1].command", docs[0]) is None
assert ["bash", "/clean-logs"] == jmespath.search("spec.template.spec.containers[1].args", docs[0])

def test_log_groomer_collector_default_retention_days(self):
docs = render_chart(show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"])
if self.obj_name == "dag-processor":
values = {"dagProcessor": {"enabled": True}}
else:
values = None

docs = render_chart(
values=values, show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"]
)

assert "AIRFLOW__LOG_RETENTION_DAYS" == jmespath.search(
"spec.template.spec.containers[1].env[0].name", docs[0]
Expand All @@ -59,24 +92,47 @@ def test_log_groomer_collector_default_retention_days(self):
@pytest.mark.parametrize("command", [None, ["custom", "command"]])
@pytest.mark.parametrize("args", [None, ["custom", "args"]])
def test_log_groomer_command_and_args_overrides(self, command, args):
if self.obj_name == "dag-processor":
values = {
"dagProcessor": {
"enabled": True,
"logGroomerSidecar": {"command": command, "args": args},
}
}
else:
values = {f"{self.folder}": {"logGroomerSidecar": {"command": command, "args": args}}}

docs = render_chart(
values={f"{self.folder}": {"logGroomerSidecar": {"command": command, "args": args}}},
values=values,
show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"],
)

assert command == jmespath.search("spec.template.spec.containers[1].command", docs[0])
assert args == jmespath.search("spec.template.spec.containers[1].args", docs[0])

def test_log_groomer_command_and_args_overrides_are_templated(self):
docs = render_chart(
values={
if self.obj_name == "dag-processor":
values = {
"dagProcessor": {
"enabled": True,
"logGroomerSidecar": {
"command": ["{{ .Release.Name }}"],
"args": ["{{ .Release.Service }}"],
},
}
}
else:
values = {
f"{self.folder}": {
"logGroomerSidecar": {
"command": ["{{ .Release.Name }}"],
"args": ["{{ .Release.Service }}"],
}
}
},
}

docs = render_chart(
values=values,
show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"],
)

Expand All @@ -85,8 +141,15 @@ def test_log_groomer_command_and_args_overrides_are_templated(self):

@pytest.mark.parametrize("retention_days, retention_result", [(None, None), (30, "30")])
def test_log_groomer_retention_days_overrides(self, retention_days, retention_result):
if self.obj_name == "dag-processor":
values = {
"dagProcessor": {"enabled": True, "logGroomerSidecar": {"retentionDays": retention_days}}
}
else:
values = {f"{self.folder}": {"logGroomerSidecar": {"retentionDays": retention_days}}}

docs = render_chart(
values={f"{self.folder}": {"logGroomerSidecar": {"retentionDays": retention_days}}},
values=values,
show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"],
)

Expand All @@ -101,8 +164,20 @@ def test_log_groomer_retention_days_overrides(self, retention_days, retention_re
assert jmespath.search("spec.template.spec.containers[1].env", docs[0]) is None

def test_log_groomer_resources(self):
docs = render_chart(
values={
if self.obj_name == "dag-processor":
values = {
"dagProcessor": {
"enabled": True,
"logGroomerSidecar": {
"resources": {
"requests": {"memory": "2Gi", "cpu": "1"},
"limits": {"memory": "3Gi", "cpu": "2"},
}
},
}
}
else:
values = {
f"{self.folder}": {
"logGroomerSidecar": {
"resources": {
Expand All @@ -111,7 +186,10 @@ def test_log_groomer_resources(self):
}
}
}
},
}

docs = render_chart(
values=values,
show_only=[f"templates/{self.folder}/{self.obj_name}-deployment.yaml"],
)

Expand Down

0 comments on commit 9577113

Please sign in to comment.