<h3 dir="auto">Apache Airflow Provider(s)</h3>
<p dir="auto">amazon</p>
<h3 dir="auto">Versions of Apache Airflow Providers</h3>
<p dir="auto">6.2.0</p>
<h3 dir="auto">Apache Airflow version</h3>
<p dir="auto">2.5.0</p>
<h3 dir="auto">Operating System</h3>
<p dir="auto">CentOS 7</p>
<h3 dir="auto">Deployment</h3>
<p dir="auto">Other Docker-based deployment</p>
<h3 dir="auto">Deployment details</h3>
<p dir="auto"><em>No response</em></p>
<h3 dir="auto">What happened</h3>
<p dir="auto">Upon upgrading to airflow 2.5.0, EMR Add Steps Operator now:</p>
<ol dir="auto">
<li>Succeeds at adding a step to our emr job.</li>
<li>tries to wait for the job to complete</li>
<li>times out, retries, and duplicates the steps on our emr cluster.</li>
<li>times out, and fails.</li>
</ol>
<p dir="auto">The cause seems to be an error in the operator:<br>
<a href="https://github.com/apache/airflow/pull/27286/files#diff-94612f7f88a5f95ca9e3f23051f39f1e0b6a4cadac1f533615db5480b379ed34R115">https://github.com/apache/airflow/pull/27286/files#diff-94612f7f88a5f95ca9e3f23051f39f1e0b6a4cadac1f533615db5480b379ed34R115</a></p>
<p dir="auto">Instead of passing <code class="notranslate">wait_for_completion=True</code> to the emr hook method <code class="notranslate">add_job_flow_steps</code>, it should probably pass <code class="notranslate">self.wait_for_completion</code>. Also, currently <code class="notranslate">self.wait_for_completion</code> seem to not be used anywhere in the project.</p>
<h3 dir="auto">What you think should happen instead</h3>
<p dir="auto">The task should succeed once the EMR steps are added and move on to the next task in the dag.</p>
<h3 dir="auto">How to reproduce</h3>
<p dir="auto"><em>No response</em></p>
<h3 dir="auto">Anything else</h3>
<p dir="auto"><em>No response</em></p>
<h3 dir="auto">Are you willing to submit PR?</h3>
<ul class="contains-task-list">
<li class="task-list-item"><input type="checkbox" id="" disabled="" class="task-list-item-checkbox" checked=""> Yes I am willing to submit a PR!</li>
</ul>
<h3 dir="auto">Code of Conduct</h3>
<ul class="contains-task-list">
<li class="task-list-item"><input type="checkbox" id="" disabled="" class="task-list-item-checkbox" checked=""> I agree to follow this project's <a href="https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md">Code of Conduct</a></li>
</ul>
<p dir="auto">Hello,</p>
<h1 dir="auto">Overview</h1>
<p dir="auto">Currently, Helm Chart for Airflow has two types of tests: (<a href="https://blog.gruntwork.io/automated-testing-for-kubernetes-and-helm-charts-using-terratest-a4ddc4e67344" rel="nofollow">Learn the best practices of Helm Chart testing</a>)</p>
<ul dir="auto">
<li><strong>Template testing (unit testing)</strong>: these tests render the templates against various input values. These types of tests let us verify that the template rendered the expected resources in the manner you intended. These tests are fast to execute and can catch syntactic errors of your template</li>
<li><strong>Integration testing</strong>: these tests take the rendered templates and deploy them to a real Kubernetes cluster. We can then verify the deployed infrastructure works as intended by hitting the endpoints or querying Kubernetes for the resources. These tests closely resemble an actual deployment and give us a close approximation of how it might behave when ready to push the chart to production.</li>
</ul>
<p dir="auto">In each case, we use a different framework:</p>
<ul dir="auto">
<li>For template testing, we use <a href="https://github.com/lrills/helm-unittest">Helm-unittest</a></li>
<li>For integration testing, we use bash scripts and Pytest + kind cluster</li>
</ul>
<h1 dir="auto">Start reading here</h1>
<p dir="auto">Today I would like to talk about template testing. In my opinion, using helm-unittest is not a very good solution.</p>
<ul dir="auto">
<li>This framework is not very popular, so any contributor who wants to change must read the documentation first to add new tests.  (People are lazy by nature, so they don't add these tests).</li>
<li>Another problem is the form - the YAML file. The YAML file is not very flexible and makes it very difficult to write more unusual tests. It would be more flexible to express the tests in a more normal programming language.</li>
</ul>
<p dir="auto">During the discussion with <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/dimberman/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/dimberman">@dimberman</a>, I prepared a small POC that shows how we can test the Helm Chart with Pytest + Python.<br>
Here is link: <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="721739710" data-permission-text="Title is private" data-url="https://github.com/apache/airflow/issues/11533" data-hovercard-type="pull_request" data-hovercard-url="/apache/airflow/pull/11533/hovercard?comment_id=504998933&amp;comment_type=review_comment" href="https://github.com/apache/airflow/pull/11533#discussion_r504998933">#11533 (comment)</a></p>
<div class="highlight highlight-source-python notranslate position-relative overflow-auto" dir="auto" data-snippet-clipboard-copy-content="import subprocess
import unittest
from tempfile import NamedTemporaryFile

import yaml
from typing import Dict, Optional

OBJECT_COUNT_IN_BASIC_DEPLOYMENT = 22


class TestBaseChartTest(unittest.TestCase):
    def render_chart(self, name, values: Optional[Dict] = None):
        values = values or {}
        with NamedTemporaryFile() as tmp_file:
            content = yaml.dump(values)
            tmp_file.write(content.encode())
            tmp_file.flush()
            templates = subprocess.check_output([&quot;helm&quot;, &quot;template&quot;, name, &quot;..&quot;, '--values', tmp_file.name])
            k8s_objects = yaml.load_all(templates)
            k8s_objects = [k8s_object for k8s_object in k8s_objects if k8s_object]
            return k8s_objects

    def test_basic_deployments(self):
        k8s_objects = self.render_chart(&quot;TEST-BASIC&quot;, {&quot;chart&quot;: {'metadata': 'AA'}})
        list_of_kind_names_tuples = [
            (k8s_object['kind'], k8s_object['metadata']['name'])
            for k8s_object in k8s_objects
        ]
        self.assertEqual(
            list_of_kind_names_tuples,
            [
                ('ServiceAccount', 'TEST-BASIC-scheduler'),
                ('ServiceAccount', 'TEST-BASIC-webserver'),
                ('ServiceAccount', 'TEST-BASIC-worker'),
                ('Secret', 'TEST-BASIC-postgresql'),
                ('Secret', 'TEST-BASIC-airflow-metadata'),
                ('Secret', 'TEST-BASIC-airflow-result-backend'),
                ('ConfigMap', 'TEST-BASIC-airflow-config'),
                ('ClusterRole', 'TEST-BASIC-pod-launcher-role'),
                ('ClusterRoleBinding', 'TEST-BASIC-pod-launcher-rolebinding'),
                ('Service', 'TEST-BASIC-postgresql-headless'),
                ('Service', 'TEST-BASIC-postgresql'),
                ('Service', 'TEST-BASIC-statsd'),
                ('Service', 'TEST-BASIC-webserver'),
                ('Deployment', 'TEST-BASIC-scheduler'),
                ('Deployment', 'TEST-BASIC-statsd'),
                ('Deployment', 'TEST-BASIC-webserver'),
                ('StatefulSet', 'TEST-BASIC-postgresql'),
                ('Secret', 'TEST-BASIC-fernet-key'),
                ('Secret', 'TEST-BASIC-redis-password'),
                ('Secret', 'TEST-BASIC-broker-url'),
                ('Job', 'TEST-BASIC-create-user'),
                ('Job', 'TEST-BASIC-run-airflow-migrations')
            ]
        )
        self.assertEqual(OBJECT_COUNT_IN_BASIC_DEPLOYMENT, len(k8s_objects))

    def test_basic_deployment_without_default_users(self):
        k8s_objects = self.render_chart(&quot;TEST-BASIC&quot;, {&quot;webserver&quot;: {'defaultUser': {'enabled': False}}})
        list_of_kind_names_tuples = [
            (k8s_object['kind'], k8s_object['metadata']['name'])
            for k8s_object in k8s_objects
        ]
        self.assertNotIn(('Job', 'TEST-BASIC-create-user'), list_of_kind_names_tuples)
        self.assertEqual(OBJECT_COUNT_IN_BASIC_DEPLOYMENT - 1, len(k8s_objects))
"><pre class="notranslate"><span class="pl-k">import</span> <span class="pl-s1">subprocess</span>
<span class="pl-k">import</span> <span class="pl-s1">unittest</span>
<span class="pl-k">from</span> <span class="pl-s1">tempfile</span> <span class="pl-k">import</span> <span class="pl-v">NamedTemporaryFile</span>

<span class="pl-k">import</span> <span class="pl-s1">yaml</span>
<span class="pl-k">from</span> <span class="pl-s1">typing</span> <span class="pl-k">import</span> <span class="pl-v">Dict</span>, <span class="pl-v">Optional</span>

<span class="pl-v">OBJECT_COUNT_IN_BASIC_DEPLOYMENT</span> <span class="pl-c1">=</span> <span class="pl-c1">22</span>


<span class="pl-k">class</span> <span class="pl-v">TestBaseChartTest</span>(<span class="pl-s1">unittest</span>.<span class="pl-v">TestCase</span>):
    <span class="pl-k">def</span> <span class="pl-en">render_chart</span>(<span class="pl-s1">self</span>, <span class="pl-s1">name</span>, <span class="pl-s1">values</span>: <span class="pl-v">Optional</span>[<span class="pl-v">Dict</span>] <span class="pl-c1">=</span> <span class="pl-c1">None</span>):
        <span class="pl-s1">values</span> <span class="pl-c1">=</span> <span class="pl-s1">values</span> <span class="pl-c1">or</span> {}
        <span class="pl-k">with</span> <span class="pl-v">NamedTemporaryFile</span>() <span class="pl-k">as</span> <span class="pl-s1">tmp_file</span>:
            <span class="pl-s1">content</span> <span class="pl-c1">=</span> <span class="pl-s1">yaml</span>.<span class="pl-en">dump</span>(<span class="pl-s1">values</span>)
            <span class="pl-s1">tmp_file</span>.<span class="pl-en">write</span>(<span class="pl-s1">content</span>.<span class="pl-en">encode</span>())
            <span class="pl-s1">tmp_file</span>.<span class="pl-en">flush</span>()
            <span class="pl-s1">templates</span> <span class="pl-c1">=</span> <span class="pl-s1">subprocess</span>.<span class="pl-en">check_output</span>([<span class="pl-s">"helm"</span>, <span class="pl-s">"template"</span>, <span class="pl-s1">name</span>, <span class="pl-s">".."</span>, <span class="pl-s">'--values'</span>, <span class="pl-s1">tmp_file</span>.<span class="pl-s1">name</span>])
            <span class="pl-s1">k8s_objects</span> <span class="pl-c1">=</span> <span class="pl-s1">yaml</span>.<span class="pl-en">load_all</span>(<span class="pl-s1">templates</span>)
            <span class="pl-s1">k8s_objects</span> <span class="pl-c1">=</span> [<span class="pl-s1">k8s_object</span> <span class="pl-k">for</span> <span class="pl-s1">k8s_object</span> <span class="pl-c1">in</span> <span class="pl-s1">k8s_objects</span> <span class="pl-k">if</span> <span class="pl-s1">k8s_object</span>]
            <span class="pl-k">return</span> <span class="pl-s1">k8s_objects</span>

    <span class="pl-k">def</span> <span class="pl-en">test_basic_deployments</span>(<span class="pl-s1">self</span>):
        <span class="pl-s1">k8s_objects</span> <span class="pl-c1">=</span> <span class="pl-s1">self</span>.<span class="pl-en">render_chart</span>(<span class="pl-s">"TEST-BASIC"</span>, {<span class="pl-s">"chart"</span>: {<span class="pl-s">'metadata'</span>: <span class="pl-s">'AA'</span>}})
        <span class="pl-s1">list_of_kind_names_tuples</span> <span class="pl-c1">=</span> [
            (<span class="pl-s1">k8s_object</span>[<span class="pl-s">'kind'</span>], <span class="pl-s1">k8s_object</span>[<span class="pl-s">'metadata'</span>][<span class="pl-s">'name'</span>])
            <span class="pl-k">for</span> <span class="pl-s1">k8s_object</span> <span class="pl-c1">in</span> <span class="pl-s1">k8s_objects</span>
        ]
        <span class="pl-s1">self</span>.<span class="pl-en">assertEqual</span>(
            <span class="pl-s1">list_of_kind_names_tuples</span>,
            [
                (<span class="pl-s">'ServiceAccount'</span>, <span class="pl-s">'TEST-BASIC-scheduler'</span>),
                (<span class="pl-s">'ServiceAccount'</span>, <span class="pl-s">'TEST-BASIC-webserver'</span>),
                (<span class="pl-s">'ServiceAccount'</span>, <span class="pl-s">'TEST-BASIC-worker'</span>),
                (<span class="pl-s">'Secret'</span>, <span class="pl-s">'TEST-BASIC-postgresql'</span>),
                (<span class="pl-s">'Secret'</span>, <span class="pl-s">'TEST-BASIC-airflow-metadata'</span>),
                (<span class="pl-s">'Secret'</span>, <span class="pl-s">'TEST-BASIC-airflow-result-backend'</span>),
                (<span class="pl-s">'ConfigMap'</span>, <span class="pl-s">'TEST-BASIC-airflow-config'</span>),
                (<span class="pl-s">'ClusterRole'</span>, <span class="pl-s">'TEST-BASIC-pod-launcher-role'</span>),
                (<span class="pl-s">'ClusterRoleBinding'</span>, <span class="pl-s">'TEST-BASIC-pod-launcher-rolebinding'</span>),
                (<span class="pl-s">'Service'</span>, <span class="pl-s">'TEST-BASIC-postgresql-headless'</span>),
                (<span class="pl-s">'Service'</span>, <span class="pl-s">'TEST-BASIC-postgresql'</span>),
                (<span class="pl-s">'Service'</span>, <span class="pl-s">'TEST-BASIC-statsd'</span>),
                (<span class="pl-s">'Service'</span>, <span class="pl-s">'TEST-BASIC-webserver'</span>),
                (<span class="pl-s">'Deployment'</span>, <span class="pl-s">'TEST-BASIC-scheduler'</span>),
                (<span class="pl-s">'Deployment'</span>, <span class="pl-s">'TEST-BASIC-statsd'</span>),
                (<span class="pl-s">'Deployment'</span>, <span class="pl-s">'TEST-BASIC-webserver'</span>),
                (<span class="pl-s">'StatefulSet'</span>, <span class="pl-s">'TEST-BASIC-postgresql'</span>),
                (<span class="pl-s">'Secret'</span>, <span class="pl-s">'TEST-BASIC-fernet-key'</span>),
                (<span class="pl-s">'Secret'</span>, <span class="pl-s">'TEST-BASIC-redis-password'</span>),
                (<span class="pl-s">'Secret'</span>, <span class="pl-s">'TEST-BASIC-broker-url'</span>),
                (<span class="pl-s">'Job'</span>, <span class="pl-s">'TEST-BASIC-create-user'</span>),
                (<span class="pl-s">'Job'</span>, <span class="pl-s">'TEST-BASIC-run-airflow-migrations'</span>)
            ]
        )
        <span class="pl-s1">self</span>.<span class="pl-en">assertEqual</span>(<span class="pl-v">OBJECT_COUNT_IN_BASIC_DEPLOYMENT</span>, <span class="pl-en">len</span>(<span class="pl-s1">k8s_objects</span>))

    <span class="pl-k">def</span> <span class="pl-en">test_basic_deployment_without_default_users</span>(<span class="pl-s1">self</span>):
        <span class="pl-s1">k8s_objects</span> <span class="pl-c1">=</span> <span class="pl-s1">self</span>.<span class="pl-en">render_chart</span>(<span class="pl-s">"TEST-BASIC"</span>, {<span class="pl-s">"webserver"</span>: {<span class="pl-s">'defaultUser'</span>: {<span class="pl-s">'enabled'</span>: <span class="pl-c1">False</span>}}})
        <span class="pl-s1">list_of_kind_names_tuples</span> <span class="pl-c1">=</span> [
            (<span class="pl-s1">k8s_object</span>[<span class="pl-s">'kind'</span>], <span class="pl-s1">k8s_object</span>[<span class="pl-s">'metadata'</span>][<span class="pl-s">'name'</span>])
            <span class="pl-k">for</span> <span class="pl-s1">k8s_object</span> <span class="pl-c1">in</span> <span class="pl-s1">k8s_objects</span>
        ]
        <span class="pl-s1">self</span>.<span class="pl-en">assertNotIn</span>((<span class="pl-s">'Job'</span>, <span class="pl-s">'TEST-BASIC-create-user'</span>), <span class="pl-s1">list_of_kind_names_tuples</span>)
        <span class="pl-s1">self</span>.<span class="pl-en">assertEqual</span>(<span class="pl-v">OBJECT_COUNT_IN_BASIC_DEPLOYMENT</span> <span class="pl-c1">-</span> <span class="pl-c1">1</span>, <span class="pl-en">len</span>(<span class="pl-s1">k8s_objects</span>))</pre></div>
<p dir="auto">Alternatively, we can also <a href="https://blog.gruntwork.io/automated-testing-for-kubernetes-and-helm-charts-using-terratest-a4ddc4e67344#d009" rel="nofollow">migrate to terratest</a>, which has native Helm Chart integration but uses Go-lang.</p>
<p dir="auto">Now there is a question for the community. What do you think about it? Should we migrate all tests to Pytestt? Is this the only reason contributors don't add unit tests? What else can we do to encourage contributors to write tests?</p>
<p dir="auto">Best regards,<br>
Kamil Breguła</p>