You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "HsiuChuanHsu (via GitHub)" <gi...@apache.org> on 2025/06/28 07:22:09 UTC

[PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

HsiuChuanHsu opened a new pull request, #52392:
URL: https://github.com/apache/airflow/pull/52392

   <!--
    Licensed to the Apache Software Foundation (ASF) under one
    or more contributor license agreements.  See the NOTICE file
    distributed with this work for additional information
    regarding copyright ownership.  The ASF licenses this file
    to you under the Apache License, Version 2.0 (the
    "License"); you may not use this file except in compliance
    with the License.  You may obtain a copy of the License at
   
      http://www.apache.org/licenses/LICENSE-2.0
   
    Unless required by applicable law or agreed to in writing,
    software distributed under the License is distributed on an
    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    KIND, either express or implied.  See the License for the
    specific language governing permissions and limitations
    under the License.
    -->
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of an existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   
   
   <!-- Please keep an empty line above the dashes. -->
   ## Description:
   This pull request introduces Horizontal Pod Autoscaler (HPA) support for the Airflow API server in the Helm chart, enabling automatic scaling of API server pods based on resource utilization or custom metrics. The changes enhance the chart's flexibility and scalability, allowing users to configure autoscaling for the API server to handle varying workloads efficiently.
   
   ## Changes:
   1. **Added HPA Configuration Template**
   Created `chart/templates/api-server/api-server-hpa.yaml `to define the HPA resource for the API server.
   2. **Updated Values Schema**
   Modified `chart/values.schema.json` to include a new hpa section under apiServer.
   3. **Updated Default Values**
   Added HPA configuration in `chart/values.yaml` under apiServer.hpa with sensible defaults
   4. **Added Unit Tests**:
   Introduced `helm-tests/tests/helm_tests/apiserver/test_hpa_apiserver.py` to validate HPA behavior
   
   ## How to Test:
   1. Enable HPA with `apiServer.hpa.enabled=true` and verify the HPA resource is created with default settings.
   2. Override `minReplicaCount`, `maxReplicaCount`, `metrics`, or `behavior` in values.yaml and confirm the HPA resource reflects the changes.
   3. Run unit tests with pytest `helm-tests/tests/helm_tests/apiserver/test_hpa_apiserver.py` to validate logic.
   
   ## Additional Notes:
   ![Screenshot 2025-06-28 at 3 15 04 PM](https://github.com/user-attachments/assets/d255c629-f9d9-4767-bc4f-70c664f31be2)
   Included deployment and HPA status screenshot from kubectl displaying an Airflow API server deployment with 4 pods running.
   
   ## Related Issues:
   #51935 
   
   ---
   **^ Add meaningful description above**
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #52392:
URL: https://github.com/apache/airflow/pull/52392#discussion_r2484904488


##########
chart/values.schema.json:
##########
@@ -5094,6 +5094,53 @@
                     ],
                     "default": null
                 },
+                "hpa": {
+                    "description": "HPA configuration.",
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "enabled": {
+                            "description": "Allow HPA autoscaling",
+                            "type": "boolean",
+                            "default": false
+                        },
+                        "minReplicaCount": {
+                            "description": "Minimum number of webservers created by HPA.",
+                            "type": "integer",
+                            "default": 1
+                        },
+                        "maxReplicaCount": {
+                            "description": "Maximum number of webservers created by HPA.",
+                            "type": "integer",
+                            "default": 5
+                        },
+                        "metrics": {
+                            "description": "Specifications for which to use to calculate the desired replica count.",
+                            "type": "array",
+                            "default": [
+                                {
+                                    "type": "Resource",
+                                    "resource": {
+                                        "name": "cpu",
+                                        "target": {
+                                            "type": "Utilization",
+                                            "averageUtilization": 80

Review Comment:
   In regards of responsiveness and time it needs to run another instance, is 80% somethings to mention as a good default? I would have expected to start scaling already at 50% to ensure good response times.
   
   Can you share a value from your production that is a sensible default?



##########
chart/values.schema.json:
##########
@@ -5094,6 +5094,53 @@
                     ],
                     "default": null
                 },
+                "hpa": {
+                    "description": "HPA configuration.",
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "enabled": {
+                            "description": "Allow HPA autoscaling",

Review Comment:
   Wording nit.
   ```suggestion
                               "description": "Enable HPA autoscaling for API server",
   ```



##########
chart/values.schema.json:
##########
@@ -5094,6 +5094,53 @@
                     ],
                     "default": null
                 },
+                "hpa": {
+                    "description": "HPA configuration.",
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "enabled": {
+                            "description": "Allow HPA autoscaling",
+                            "type": "boolean",
+                            "default": false
+                        },
+                        "minReplicaCount": {
+                            "description": "Minimum number of webservers created by HPA.",

Review Comment:
   Can you adjust the text in line 5050 as well that replicas are not considered if HPA is enabled?



##########
chart/values.schema.json:
##########
@@ -5094,6 +5094,53 @@
                     ],
                     "default": null
                 },
+                "hpa": {
+                    "description": "HPA configuration.",
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "enabled": {
+                            "description": "Allow HPA autoscaling",
+                            "type": "boolean",
+                            "default": false
+                        },
+                        "minReplicaCount": {
+                            "description": "Minimum number of webservers created by HPA.",

Review Comment:
   ```suggestion
                               "description": "Minimum number of webservers created by HPA if HPA enabled.",
   ```



##########
chart/values.schema.json:
##########
@@ -5094,6 +5094,53 @@
                     ],
                     "default": null
                 },
+                "hpa": {
+                    "description": "HPA configuration.",

Review Comment:
   Can you be a bit mor verbose for all knowing what HPA is?
   ```suggestion
                       "description": "Horizontal Pod Autoscaler (HPA) configuration. (optional)",
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "HsiuChuanHsu (via GitHub)" <gi...@apache.org>.
HsiuChuanHsu commented on code in PR #52392:
URL: https://github.com/apache/airflow/pull/52392#discussion_r2565245507


##########
helm-tests/tests/helm_tests/apiserver/test_hpa_apiserver.py:
##########
@@ -0,0 +1,139 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import jmespath
+import pytest
+from chart_utils.helm_template_generator import render_chart
+
+
+class TestAPIServerHPA:
+    """Tests HPA."""
+
+    def test_hpa_disabled_by_default(self):
+        """Disabled by default."""
+        docs = render_chart(
+            values={},
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert docs == []
+
+    def test_should_add_component_specific_labels(self):
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {"enabled": True},
+                    "labels": {"test_label": "test_label_value"},
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+
+        assert "test_label" in jmespath.search("metadata.labels", docs[0])
+        assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+    @pytest.mark.parametrize(
+        "min_replicas, max_replicas",
+        [
+            (None, None),
+            (2, 8),
+        ],
+    )
+    def test_min_max_replicas(self, min_replicas, max_replicas):
+        """Verify minimum and maximum replicas."""
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {
+                        "enabled": True,
+                        **({"minReplicaCount": min_replicas} if min_replicas else {}),
+                        **({"maxReplicaCount": max_replicas} if max_replicas else {}),
+                    }
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert jmespath.search("spec.minReplicas", docs[0]) == 1 if min_replicas is None else min_replicas
+        assert jmespath.search("spec.maxReplicas", docs[0]) == 5 if max_replicas is None else max_replicas
+
+    def test_hpa_behavior(self):
+        """Verify HPA behavior."""
+        expected_behavior = {
+            "scaleDown": {
+                "stabilizationWindowSeconds": 300,
+                "policies": [{"type": "Percent", "value": 100, "periodSeconds": 15}],
+            }
+        }
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {
+                        "enabled": True,
+                        "behavior": expected_behavior,
+                    },
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert jmespath.search("spec.behavior", docs[0]) == expected_behavior
+
+    @pytest.mark.parametrize(
+        "metrics, expected_metrics",
+        [
+            # default metrics
+            (
+                None,
+                {
+                    "type": "Resource",
+                    "resource": {"name": "cpu", "target": {"type": "Utilization", "averageUtilization": 50}},
+                },
+            ),
+            # custom metric
+            (
+                [
+                    {
+                        "type": "Pods",
+                        "pods": {
+                            "metric": {"name": "custom"},
+                            "target": {"type": "Utilization", "averageUtilization": 50},
+                        },
+                    }
+                ],
+                {
+                    "type": "Pods",
+                    "pods": {
+                        "metric": {"name": "custom"},
+                        "target": {"type": "Utilization", "averageUtilization": 50},
+                    },
+                },
+            ),
+        ],
+    )
+    def test_should_use_hpa_metrics(self, metrics, expected_metrics):
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {"enabled": True, **({"metrics": metrics} if metrics else {})},
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert expected_metrics == jmespath.search("spec.metrics[0]", docs[0])

Review Comment:
   Fixd! Thanks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl merged PR #52392:
URL: https://github.com/apache/airflow/pull/52392


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #52392:
URL: https://github.com/apache/airflow/pull/52392#discussion_r2596659141


##########
helm-tests/tests/helm_tests/apiserver/test_hpa_apiserver.py:
##########
@@ -0,0 +1,139 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import jmespath
+import pytest
+from chart_utils.helm_template_generator import render_chart
+
+
+class TestAPIServerHPA:
+    """Tests HPA."""
+
+    def test_hpa_disabled_by_default(self):
+        """Disabled by default."""
+        docs = render_chart(
+            values={},
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert docs == []
+
+    def test_should_add_component_specific_labels(self):
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {"enabled": True},
+                    "labels": {"test_label": "test_label_value"},
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+
+        assert "test_label" in jmespath.search("metadata.labels", docs[0])
+        assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+    @pytest.mark.parametrize(
+        "min_replicas, max_replicas",

Review Comment:
   Convert to tuple
   ```suggestion
           ("min_replicas", "max_replicas"),
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #52392:
URL: https://github.com/apache/airflow/pull/52392#issuecomment-3015053143

   Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-pre-commit-hooks) will help you with that.
   - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
   - Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
   - Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
   - Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
   - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
   - Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
   Apache Airflow is a community-driven project and together we are making it better 🚀.
   In case of doubts contact the developers at:
   Mailing List: dev@airflow.apache.org
   Slack: https://s.apache.org/airflow-slack
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "HsiuChuanHsu (via GitHub)" <gi...@apache.org>.
HsiuChuanHsu commented on code in PR #52392:
URL: https://github.com/apache/airflow/pull/52392#discussion_r2565245507


##########
helm-tests/tests/helm_tests/apiserver/test_hpa_apiserver.py:
##########
@@ -0,0 +1,139 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import jmespath
+import pytest
+from chart_utils.helm_template_generator import render_chart
+
+
+class TestAPIServerHPA:
+    """Tests HPA."""
+
+    def test_hpa_disabled_by_default(self):
+        """Disabled by default."""
+        docs = render_chart(
+            values={},
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert docs == []
+
+    def test_should_add_component_specific_labels(self):
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {"enabled": True},
+                    "labels": {"test_label": "test_label_value"},
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+
+        assert "test_label" in jmespath.search("metadata.labels", docs[0])
+        assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+    @pytest.mark.parametrize(
+        "min_replicas, max_replicas",
+        [
+            (None, None),
+            (2, 8),
+        ],
+    )
+    def test_min_max_replicas(self, min_replicas, max_replicas):
+        """Verify minimum and maximum replicas."""
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {
+                        "enabled": True,
+                        **({"minReplicaCount": min_replicas} if min_replicas else {}),
+                        **({"maxReplicaCount": max_replicas} if max_replicas else {}),
+                    }
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert jmespath.search("spec.minReplicas", docs[0]) == 1 if min_replicas is None else min_replicas
+        assert jmespath.search("spec.maxReplicas", docs[0]) == 5 if max_replicas is None else max_replicas
+
+    def test_hpa_behavior(self):
+        """Verify HPA behavior."""
+        expected_behavior = {
+            "scaleDown": {
+                "stabilizationWindowSeconds": 300,
+                "policies": [{"type": "Percent", "value": 100, "periodSeconds": 15}],
+            }
+        }
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {
+                        "enabled": True,
+                        "behavior": expected_behavior,
+                    },
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert jmespath.search("spec.behavior", docs[0]) == expected_behavior
+
+    @pytest.mark.parametrize(
+        "metrics, expected_metrics",
+        [
+            # default metrics
+            (
+                None,
+                {
+                    "type": "Resource",
+                    "resource": {"name": "cpu", "target": {"type": "Utilization", "averageUtilization": 50}},
+                },
+            ),
+            # custom metric
+            (
+                [
+                    {
+                        "type": "Pods",
+                        "pods": {
+                            "metric": {"name": "custom"},
+                            "target": {"type": "Utilization", "averageUtilization": 50},
+                        },
+                    }
+                ],
+                {
+                    "type": "Pods",
+                    "pods": {
+                        "metric": {"name": "custom"},
+                        "target": {"type": "Utilization", "averageUtilization": 50},
+                    },
+                },
+            ),
+        ],
+    )
+    def test_should_use_hpa_metrics(self, metrics, expected_metrics):
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {"enabled": True, **({"metrics": metrics} if metrics else {})},
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert expected_metrics == jmespath.search("spec.metrics[0]", docs[0])

Review Comment:
   Fixd!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "HsiuChuanHsu (via GitHub)" <gi...@apache.org>.
HsiuChuanHsu commented on PR #52392:
URL: https://github.com/apache/airflow/pull/52392#issuecomment-3436284520

   No!! Pls don't!!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #52392:
URL: https://github.com/apache/airflow/pull/52392#discussion_r2551972304


##########
helm-tests/tests/helm_tests/apiserver/test_hpa_apiserver.py:
##########
@@ -0,0 +1,139 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import jmespath
+import pytest
+from chart_utils.helm_template_generator import render_chart
+
+
+class TestAPIServerHPA:
+    """Tests HPA."""
+
+    def test_hpa_disabled_by_default(self):
+        """Disabled by default."""
+        docs = render_chart(
+            values={},
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert docs == []
+
+    def test_should_add_component_specific_labels(self):
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {"enabled": True},
+                    "labels": {"test_label": "test_label_value"},
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+
+        assert "test_label" in jmespath.search("metadata.labels", docs[0])
+        assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+    @pytest.mark.parametrize(
+        "min_replicas, max_replicas",
+        [
+            (None, None),
+            (2, 8),
+        ],
+    )
+    def test_min_max_replicas(self, min_replicas, max_replicas):
+        """Verify minimum and maximum replicas."""
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {
+                        "enabled": True,
+                        **({"minReplicaCount": min_replicas} if min_replicas else {}),
+                        **({"maxReplicaCount": max_replicas} if max_replicas else {}),
+                    }
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert jmespath.search("spec.minReplicas", docs[0]) == 1 if min_replicas is None else min_replicas
+        assert jmespath.search("spec.maxReplicas", docs[0]) == 5 if max_replicas is None else max_replicas
+
+    def test_hpa_behavior(self):
+        """Verify HPA behavior."""
+        expected_behavior = {
+            "scaleDown": {
+                "stabilizationWindowSeconds": 300,
+                "policies": [{"type": "Percent", "value": 100, "periodSeconds": 15}],
+            }
+        }
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {
+                        "enabled": True,
+                        "behavior": expected_behavior,
+                    },
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert jmespath.search("spec.behavior", docs[0]) == expected_behavior
+
+    @pytest.mark.parametrize(
+        "metrics, expected_metrics",
+        [
+            # default metrics
+            (
+                None,
+                {
+                    "type": "Resource",
+                    "resource": {"name": "cpu", "target": {"type": "Utilization", "averageUtilization": 50}},
+                },
+            ),
+            # custom metric
+            (
+                [
+                    {
+                        "type": "Pods",
+                        "pods": {
+                            "metric": {"name": "custom"},
+                            "target": {"type": "Utilization", "averageUtilization": 50},
+                        },
+                    }
+                ],
+                {
+                    "type": "Pods",
+                    "pods": {
+                        "metric": {"name": "custom"},
+                        "target": {"type": "Utilization", "averageUtilization": 50},
+                    },
+                },
+            ),
+        ],
+    )
+    def test_should_use_hpa_metrics(self, metrics, expected_metrics):
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {"enabled": True, **({"metrics": metrics} if metrics else {})},
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert expected_metrics == jmespath.search("spec.metrics[0]", docs[0])

Review Comment:
   ```suggestion
           assert  jmespath.search("spec.metrics[0]", docs[0]) == expected_metrics
   ```
   
   right is right :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "HsiuChuanHsu (via GitHub)" <gi...@apache.org>.
HsiuChuanHsu commented on PR #52392:
URL: https://github.com/apache/airflow/pull/52392#issuecomment-3057917924

   Hi @romsharon98, @eladkal could you help review this PR? Thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "HsiuChuanHsu (via GitHub)" <gi...@apache.org>.
HsiuChuanHsu commented on code in PR #52392:
URL: https://github.com/apache/airflow/pull/52392#discussion_r2486464994


##########
chart/values.schema.json:
##########
@@ -5094,6 +5094,53 @@
                     ],
                     "default": null
                 },
+                "hpa": {
+                    "description": "HPA configuration.",

Review Comment:
   Yeah. It should be more details. Add more description in `apiServer.hpa` section in `values.yaml`.



##########
chart/values.schema.json:
##########
@@ -5094,6 +5094,53 @@
                     ],
                     "default": null
                 },
+                "hpa": {
+                    "description": "HPA configuration.",
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "enabled": {
+                            "description": "Allow HPA autoscaling",

Review Comment:
   Fixed.



##########
chart/values.schema.json:
##########
@@ -5094,6 +5094,53 @@
                     ],
                     "default": null
                 },
+                "hpa": {
+                    "description": "HPA configuration.",
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "enabled": {
+                            "description": "Allow HPA autoscaling",
+                            "type": "boolean",
+                            "default": false
+                        },
+                        "minReplicaCount": {
+                            "description": "Minimum number of webservers created by HPA.",

Review Comment:
   Yes, this one is important for user. Add more description in `apiServer.replica` section in `values.yaml`.
   Also it should be apiServer instead of webservers.
   



##########
chart/values.schema.json:
##########
@@ -5094,6 +5094,53 @@
                     ],
                     "default": null
                 },
+                "hpa": {
+                    "description": "HPA configuration.",
+                    "type": "object",
+                    "additionalProperties": false,
+                    "properties": {
+                        "enabled": {
+                            "description": "Allow HPA autoscaling",
+                            "type": "boolean",
+                            "default": false
+                        },
+                        "minReplicaCount": {
+                            "description": "Minimum number of webservers created by HPA.",
+                            "type": "integer",
+                            "default": 1
+                        },
+                        "maxReplicaCount": {
+                            "description": "Maximum number of webservers created by HPA.",
+                            "type": "integer",
+                            "default": 5
+                        },
+                        "metrics": {
+                            "description": "Specifications for which to use to calculate the desired replica count.",
+                            "type": "array",
+                            "default": [
+                                {
+                                    "type": "Resource",
+                                    "resource": {
+                                        "name": "cpu",
+                                        "target": {
+                                            "type": "Utilization",
+                                            "averageUtilization": 80

Review Comment:
   We do most of our autoscling settings on KEDA instead of HPA, but it makes sense to ensure good response times at 50%.
   Updated!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #52392:
URL: https://github.com/apache/airflow/pull/52392#discussion_r2596659817


##########
helm-tests/tests/helm_tests/apiserver/test_hpa_apiserver.py:
##########
@@ -0,0 +1,139 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import jmespath
+import pytest
+from chart_utils.helm_template_generator import render_chart
+
+
+class TestAPIServerHPA:
+    """Tests HPA."""
+
+    def test_hpa_disabled_by_default(self):
+        """Disabled by default."""
+        docs = render_chart(
+            values={},
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert docs == []
+
+    def test_should_add_component_specific_labels(self):
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {"enabled": True},
+                    "labels": {"test_label": "test_label_value"},
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+
+        assert "test_label" in jmespath.search("metadata.labels", docs[0])
+        assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+    @pytest.mark.parametrize(
+        "min_replicas, max_replicas",
+        [
+            (None, None),
+            (2, 8),
+        ],
+    )
+    def test_min_max_replicas(self, min_replicas, max_replicas):
+        """Verify minimum and maximum replicas."""
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {
+                        "enabled": True,
+                        **({"minReplicaCount": min_replicas} if min_replicas else {}),
+                        **({"maxReplicaCount": max_replicas} if max_replicas else {}),
+                    }
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert jmespath.search("spec.minReplicas", docs[0]) == 1 if min_replicas is None else min_replicas
+        assert jmespath.search("spec.maxReplicas", docs[0]) == 5 if max_replicas is None else max_replicas
+
+    def test_hpa_behavior(self):
+        """Verify HPA behavior."""
+        expected_behavior = {
+            "scaleDown": {
+                "stabilizationWindowSeconds": 300,
+                "policies": [{"type": "Percent", "value": 100, "periodSeconds": 15}],
+            }
+        }
+        docs = render_chart(
+            values={
+                "airflowVersion": "3.0.2",
+                "apiServer": {
+                    "hpa": {
+                        "enabled": True,
+                        "behavior": expected_behavior,
+                    },
+                },
+            },
+            show_only=["templates/api-server/api-server-hpa.yaml"],
+        )
+        assert jmespath.search("spec.behavior", docs[0]) == expected_behavior
+
+    @pytest.mark.parametrize(
+        "metrics, expected_metrics",

Review Comment:
   Convert to tuple
   ```suggestion
           ("metrics", "expected_metrics"),
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #52392:
URL: https://github.com/apache/airflow/pull/52392#issuecomment-3434635695

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add Horizontal Pod Autoscaler (HPA) Support for Airflow API Server [airflow]

Posted by "HsiuChuanHsu (via GitHub)" <gi...@apache.org>.
HsiuChuanHsu commented on PR #52392:
URL: https://github.com/apache/airflow/pull/52392#issuecomment-3480525493

   Happy to see my first PR got reviewed. Thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org