Miretpl commented on code in PR #69945:
URL: https://github.com/apache/airflow/pull/69945#discussion_r3625977430
##########
chart/docs/production-guide.rst:
##########
@@ -411,16 +411,35 @@ the chart only creates the ``HTTPRoute`` and attaches it
to the Gateway via ``pa
hostnames:
- airflow.example.com
-For fine-grained routing, supply ``apiServer.httpRoute.rules`` directly — the
entry mirrors the
-upstream ``HTTPRouteRule`` schema and overrides the default rule generated
from ``path`` + ``pathType``.
+ executor: CeleryExecutor
Review Comment:
I would move `executor` to the top to resemble the `values.yaml` file config
order.
##########
chart/docs/production-guide.rst:
##########
@@ -411,16 +411,35 @@ the chart only creates the ``HTTPRoute`` and attaches it
to the Gateway via ``pa
hostnames:
- airflow.example.com
-For fine-grained routing, supply ``apiServer.httpRoute.rules`` directly — the
entry mirrors the
-upstream ``HTTPRouteRule`` schema and overrides the default rule generated
from ``path`` + ``pathType``.
+ executor: CeleryExecutor
+
+ flower:
+ enabled: true
+ httpRoute:
+ enabled: true
+ parentRefs:
+ - name: main-gateway
+ namespace: gateway-system
+ sectionName: https
+ hostnames:
+ - flower.example.com
+
+Flower HTTPRoute resources are only created when Flower itself is created, so
``flower.enabled`` must be
+``true`` and the executor must include ``CeleryExecutor``.
+
+For fine-grained routing, supply ``apiServer.httpRoute.rules`` or
``flower.httpRoute.rules`` directly —
+the entry mirrors the upstream ``HTTPRouteRule`` schema and overrides the
default rule generated from
+``path`` + ``pathType``. For Flower deployments that customize
``flower.service.ports`` away from the
Review Comment:
```suggestion
``apiServer/flower.httpRoute.path`` +
``apiServer/flower.httpRoute.pathType``. For Flower deployments that customize
``flower.service.ports`` away from the
```
it should be more clear for the reader in that way I think.
##########
chart/docs/production-guide.rst:
##########
@@ -411,16 +411,35 @@ the chart only creates the ``HTTPRoute`` and attaches it
to the Gateway via ``pa
hostnames:
- airflow.example.com
-For fine-grained routing, supply ``apiServer.httpRoute.rules`` directly — the
entry mirrors the
-upstream ``HTTPRouteRule`` schema and overrides the default rule generated
from ``path`` + ``pathType``.
+ executor: CeleryExecutor
+
+ flower:
+ enabled: true
+ httpRoute:
+ enabled: true
+ parentRefs:
+ - name: main-gateway
+ namespace: gateway-system
+ sectionName: https
+ hostnames:
+ - flower.example.com
+
+Flower HTTPRoute resources are only created when Flower itself is created, so
``flower.enabled`` must be
+``true`` and the executor must include ``CeleryExecutor``.
+
+For fine-grained routing, supply ``apiServer.httpRoute.rules`` or
``flower.httpRoute.rules`` directly —
+the entry mirrors the upstream ``HTTPRouteRule`` schema and overrides the
default rule generated from
+``path`` + ``pathType``. For Flower deployments that customize
``flower.service.ports`` away from the
+standard ``ports.flowerUI`` value, use ``flower.httpRoute.rules`` to point at
the desired backend port.
.. note::
- ``HTTPRoute`` is an alternative to the API server ``Ingress``, so enable
only one of
- ``ingress.apiServer`` or ``apiServer.httpRoute`` — enabling both at the
same time fails template
- rendering. When ``apiServer.httpRoute.enabled`` is ``true``, the chart also
verifies (via Helm
- ``Capabilities``) that the Gateway API CRDs are installed and fails with a
clear message if they
- are not.
+ ``HTTPRoute`` is an alternative to ``Ingress`` for the same component, so
enable only one routing
+ mechanism for each component. For the API server, enable only one of
``ingress.apiServer`` or
+ ``apiServer.httpRoute``. For Flower, enable only one of ``ingress.flower``
or
+ ``flower.httpRoute``. Enabling both for the same component fails template
rendering. When an
+ ``HTTPRoute`` is enabled, the chart also verifies (via Helm
``Capabilities``) that the Gateway API
+ CRDs are installed and fails with a clear message if they are not.
Review Comment:
```suggestion
``HTTPRoute`` is an alternative to ``Ingress`` for the same component, so
enable only one routing
mechanism for each component. Enabling both for the same component fails
template rendering.
When an ``HTTPRoute`` is enabled, the chart also verifies (via Helm
``Capabilities``) that the Gateway
API CRDs are installed and fails with a clear message if they are not.
```
##########
chart/docs/production-guide.rst:
##########
@@ -411,16 +411,35 @@ the chart only creates the ``HTTPRoute`` and attaches it
to the Gateway via ``pa
hostnames:
- airflow.example.com
-For fine-grained routing, supply ``apiServer.httpRoute.rules`` directly — the
entry mirrors the
-upstream ``HTTPRouteRule`` schema and overrides the default rule generated
from ``path`` + ``pathType``.
+ executor: CeleryExecutor
+
+ flower:
+ enabled: true
+ httpRoute:
+ enabled: true
+ parentRefs:
+ - name: main-gateway
+ namespace: gateway-system
+ sectionName: https
+ hostnames:
+ - flower.example.com
+
+Flower HTTPRoute resources are only created when Flower itself is created, so
``flower.enabled`` must be
+``true`` and the executor must include ``CeleryExecutor``.
+
+For fine-grained routing, supply ``apiServer.httpRoute.rules`` or
``flower.httpRoute.rules`` directly —
+the entry mirrors the upstream ``HTTPRouteRule`` schema and overrides the
default rule generated from
+``path`` + ``pathType``. For Flower deployments that customize
``flower.service.ports`` away from the
+standard ``ports.flowerUI`` value, use ``flower.httpRoute.rules`` to point at
the desired backend port.
Review Comment:
Port handling works in the same way for apiServer and flower looking at the
current implementation, so I think that the last sentence should be deleted.
##########
chart/values.schema.json:
##########
@@ -6504,6 +6504,102 @@
"type": "boolean",
"default": false
},
+ "httpRoute": {
Review Comment:
As we have almost the same definition of `httpRoute` for `flower` and
`apiServer`, we could extract this to `defs` and use it as `$ref` in the schema
file. It can be also follow-up PR.
##########
chart/values.yaml:
##########
@@ -2327,6 +2327,54 @@ flower:
# If True, and using CeleryExecutor, will deploy flower app.
enabled: false
+ # Kubernetes Gateway API (HTTPRoute) configuration for Flower.
+ # Requires the Gateway API CRDs to be installed in the cluster
+ # (https://gateway-api.sigs.k8s.io/guides/#installing-gateway-api).
+ # The HTTPRoute references an externally managed Gateway via `parentRefs`.
+ # This is an alternative to the Flower Ingress (`ingress.flower`);
+ # the two are mutually exclusive and enabling both will fail rendering.
Review Comment:
```suggestion
```
Information duplication: looking at the comment over the enabled flag. Could
you also adjust the comment next to apiServer? I imagine that there is the same
one 🤔
##########
chart/values.yaml:
##########
@@ -2327,6 +2327,54 @@ flower:
# If True, and using CeleryExecutor, will deploy flower app.
enabled: false
+ # Kubernetes Gateway API (HTTPRoute) configuration for Flower.
+ # Requires the Gateway API CRDs to be installed in the cluster
+ # (https://gateway-api.sigs.k8s.io/guides/#installing-gateway-api).
+ # The HTTPRoute references an externally managed Gateway via `parentRefs`.
+ # This is an alternative to the Flower Ingress (`ingress.flower`);
+ # the two are mutually exclusive and enabling both will fail rendering.
+ httpRoute:
+ # Enable Flower HTTPRoute resource.
+ # Mutually exclusive with `ingress.flower.enabled`; enable only one of
them.
Review Comment:
Could you add similar info next to ingress one?
##########
chart/values.yaml:
##########
@@ -2327,6 +2327,54 @@ flower:
# If True, and using CeleryExecutor, will deploy flower app.
enabled: false
+ # Kubernetes Gateway API (HTTPRoute) configuration for Flower.
+ # Requires the Gateway API CRDs to be installed in the cluster
+ # (https://gateway-api.sigs.k8s.io/guides/#installing-gateway-api).
+ # The HTTPRoute references an externally managed Gateway via `parentRefs`.
+ # This is an alternative to the Flower Ingress (`ingress.flower`);
+ # the two are mutually exclusive and enabling both will fail rendering.
+ httpRoute:
+ # Enable Flower HTTPRoute resource.
+ # Mutually exclusive with `ingress.flower.enabled`; enable only one of
them.
+ enabled: false
+
+ # Extra labels for the Flower HTTPRoute
+ labels: {}
+
+ # Annotations for the Flower HTTPRoute
+ annotations: {}
+
+ # List of parent Gateway references this HTTPRoute attaches to. Required
when enabled.
+ # Defaults to `~` (null) so the schema's `minItems: 1` fails fast on an
explicitly
+ # empty list (`[]`); a still-null value while enabled is caught by the
template guard.
Review Comment:
```suggestion
```
Implementation details that are rather not needed on configuration level. If
the same is next to apiServer, could you adjust it?
##########
chart/tests/helm_tests/other/test_httproute_flower.py:
##########
@@ -0,0 +1,413 @@
+# 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 HelmFailedError, render_chart
+
+SHOW_ONLY = ["templates/flower/flower-httproute.yaml"]
+FLOWER_INGRESS_SHOW_ONLY = ["templates/flower/flower-ingress.yaml"]
+
+GATEWAY_API_VERSIONS = ["gateway.networking.k8s.io/v1"]
+MINIMAL_PARENT_REFS = [{"name": "main-gateway"}]
+
+
+class TestHTTPRouteFlower:
+ """Tests HTTPRoute Flower (Kubernetes Gateway API)."""
+
+ def test_should_pass_validation_with_minimal_config(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert len(docs) == 1
+ assert jmespath.search("kind", docs[0]) == "HTTPRoute"
+ assert jmespath.search("metadata.name", docs[0]) ==
"release-name-flower-httproute"
+
+ def test_should_set_api_version_and_kind(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("apiVersion", docs[0]) ==
"gateway.networking.k8s.io/v1"
+ assert jmespath.search("kind", docs[0]) == "HTTPRoute"
+
+ def test_should_allow_more_than_one_annotation(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "annotations": {"aa": "bb", "cc": "dd"},
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("metadata.annotations", docs[0]) == {"aa":
"bb", "cc": "dd"}
+
+ def test_should_add_extra_labels(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "labels": {"custom": "value"},
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search('metadata.labels."custom"', docs[0]) == "value"
+
+ def test_should_pass_parent_refs_through(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": [
+ {"name": "main-gateway", "namespace":
"gateway-system"},
+ {"name": "main-gateway", "namespace":
"gateway-system", "sectionName": "https"},
+ ],
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.parentRefs", docs[0]) == [
+ {"name": "main-gateway", "namespace": "gateway-system"},
+ {"name": "main-gateway", "namespace": "gateway-system",
"sectionName": "https"},
+ ]
+
+ def test_should_set_hostnames(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "hostnames": ["flower.example.com",
"flower2.example.com"],
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.hostnames", docs[0]) == [
+ "flower.example.com",
+ "flower2.example.com",
+ ]
+
+ def test_hostnames_should_be_templated(self):
+ docs = render_chart(
+ name="airflow",
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "hostnames": ["{{ .Release.Name }}.example.com"],
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.hostnames", docs[0]) ==
["airflow.example.com"]
+
+ def test_should_default_to_path_prefix_and_flower_backend(self):
+ docs = render_chart(
+ name="my-release",
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.rules", docs[0]) == [
+ {
+ "matches": [{"path": {"type": "PathPrefix", "value": "/"}}],
+ "backendRefs": [{"name": "my-release-flower", "port": 5555}],
+ },
+ ]
+
+ def test_custom_path_and_path_type_should_apply(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "path": "/flower",
+ "pathType": "Exact",
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.rules[0].matches[0].path.type", docs[0])
== "Exact"
+ assert jmespath.search("spec.rules[0].matches[0].path.value", docs[0])
== "/flower"
+
+ def test_custom_rules_override_default_rule(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "rules": [
+ {
+ "matches": [{"path": {"type": "PathPrefix",
"value": "/custom-flower"}}],
+ "backendRefs": [{"name": "external-flower",
"port": 8443}],
+ },
+ ],
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.rules", docs[0]) == [
+ {
+ "matches": [{"path": {"type": "PathPrefix", "value":
"/custom-flower"}}],
+ "backendRefs": [{"name": "external-flower", "port": 8443}],
+ },
+ ]
+
+ def test_should_use_flower_ui_port_for_default_backend(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "ports": {"flowerUI": 9000},
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.rules[0].backendRefs[0].port", docs[0])
== 9000
+
+ def test_httproute_not_created_when_unset(self):
+ docs = render_chart(
+ values={"executor": "CeleryExecutor", "flower": {"enabled": True,
"httpRoute": {}}},
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert docs == []
+
+ def test_httproute_not_created_when_disabled(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {"enabled": True, "httpRoute": {"enabled": False}},
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert docs == []
+
+ def test_httproute_created_when_enabled(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert len(docs) == 1
+
+ def test_should_not_render_when_flower_disabled(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": False,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert docs == []
+
+ @pytest.mark.parametrize("executor", ["LocalExecutor",
"KubernetesExecutor"])
+ def test_should_not_render_without_celery_executor(self, executor):
+ docs = render_chart(
+ values={
+ "executor": executor,
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert docs == []
+
+ def test_should_fail_when_gateway_api_crd_missing(self):
+ with pytest.raises(HelmFailedError) as exc_info:
+ render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ )
+ assert "Gateway API HTTPRoute CRD" in exc_info.value.stderr.decode()
+
+ def test_should_fail_when_parent_refs_empty(self):
+ with pytest.raises(HelmFailedError) as exc_info:
+ render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {"enabled": True, "httpRoute": {"enabled":
True}},
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert "parentRefs" in exc_info.value.stderr.decode()
+
+ def test_should_fail_schema_when_parent_refs_explicitly_empty(self):
+ with pytest.raises(HelmFailedError) as exc_info:
+ render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {"enabled": True, "httpRoute": {"enabled": True,
"parentRefs": []}},
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert "parentRefs" in exc_info.value.stderr.decode()
+
+ def test_should_fail_when_ingress_and_httproute_both_enabled(self):
+ with pytest.raises(HelmFailedError) as exc_info:
+ render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "ingress": {"flower": {"enabled": True}},
+ "flower": {"enabled": True, "httpRoute": {"enabled":
True}},
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert "enable only one of them" in exc_info.value.stderr.decode()
+
+ def
test_should_fail_when_ingress_template_sees_ingress_and_httproute_both_enabled(self):
+ with pytest.raises(HelmFailedError) as exc_info:
+ render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "ingress": {"flower": {"enabled": True}},
+ "flower": {"enabled": True, "httpRoute": {"enabled":
True}},
+ },
+ show_only=FLOWER_INGRESS_SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert "enable only one of them" in exc_info.value.stderr.decode()
+
+ def test_backend_service_name_with_fullname_override(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "fullnameOverride": "airflow-fullname-override",
+ "useStandardNaming": True,
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert (
+ jmespath.search("spec.rules[0].backendRefs[0].name", docs[0])
+ == "airflow-fullname-override-flower"
+ )
+
+ def test_should_add_component_specific_labels(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "labels": {"label1": "value1", "label2": "value2"},
+ "flower": {
+ "enabled": True,
+ "labels": {"test_label": "test_label_value"},
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "labels": {"route_label": "route_value"},
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert "tier" in jmespath.search("metadata.labels", docs[0])
+ assert "release" in jmespath.search("metadata.labels", docs[0])
Review Comment:
Do we have a test for this (labels added for every resource) somewhere under
common tests? 🤔
##########
chart/tests/helm_tests/other/test_httproute_flower.py:
##########
@@ -0,0 +1,413 @@
+# 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 HelmFailedError, render_chart
+
+SHOW_ONLY = ["templates/flower/flower-httproute.yaml"]
+FLOWER_INGRESS_SHOW_ONLY = ["templates/flower/flower-ingress.yaml"]
+
+GATEWAY_API_VERSIONS = ["gateway.networking.k8s.io/v1"]
+MINIMAL_PARENT_REFS = [{"name": "main-gateway"}]
+
+
+class TestHTTPRouteFlower:
+ """Tests HTTPRoute Flower (Kubernetes Gateway API)."""
+
+ def test_should_pass_validation_with_minimal_config(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert len(docs) == 1
+ assert jmespath.search("kind", docs[0]) == "HTTPRoute"
+ assert jmespath.search("metadata.name", docs[0]) ==
"release-name-flower-httproute"
+
+ def test_should_set_api_version_and_kind(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("apiVersion", docs[0]) ==
"gateway.networking.k8s.io/v1"
+ assert jmespath.search("kind", docs[0]) == "HTTPRoute"
+
+ def test_should_allow_more_than_one_annotation(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "annotations": {"aa": "bb", "cc": "dd"},
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("metadata.annotations", docs[0]) == {"aa":
"bb", "cc": "dd"}
+
+ def test_should_add_extra_labels(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "labels": {"custom": "value"},
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search('metadata.labels."custom"', docs[0]) == "value"
+
+ def test_should_pass_parent_refs_through(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": [
+ {"name": "main-gateway", "namespace":
"gateway-system"},
+ {"name": "main-gateway", "namespace":
"gateway-system", "sectionName": "https"},
+ ],
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.parentRefs", docs[0]) == [
+ {"name": "main-gateway", "namespace": "gateway-system"},
+ {"name": "main-gateway", "namespace": "gateway-system",
"sectionName": "https"},
+ ]
+
+ def test_should_set_hostnames(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "hostnames": ["flower.example.com",
"flower2.example.com"],
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.hostnames", docs[0]) == [
+ "flower.example.com",
+ "flower2.example.com",
+ ]
+
+ def test_hostnames_should_be_templated(self):
+ docs = render_chart(
+ name="airflow",
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "hostnames": ["{{ .Release.Name }}.example.com"],
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.hostnames", docs[0]) ==
["airflow.example.com"]
+
+ def test_should_default_to_path_prefix_and_flower_backend(self):
+ docs = render_chart(
+ name="my-release",
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.rules", docs[0]) == [
+ {
+ "matches": [{"path": {"type": "PathPrefix", "value": "/"}}],
+ "backendRefs": [{"name": "my-release-flower", "port": 5555}],
+ },
+ ]
+
+ def test_custom_path_and_path_type_should_apply(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "path": "/flower",
+ "pathType": "Exact",
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.rules[0].matches[0].path.type", docs[0])
== "Exact"
+ assert jmespath.search("spec.rules[0].matches[0].path.value", docs[0])
== "/flower"
+
+ def test_custom_rules_override_default_rule(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {
+ "enabled": True,
+ "parentRefs": MINIMAL_PARENT_REFS,
+ "rules": [
+ {
+ "matches": [{"path": {"type": "PathPrefix",
"value": "/custom-flower"}}],
+ "backendRefs": [{"name": "external-flower",
"port": 8443}],
+ },
+ ],
+ },
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.rules", docs[0]) == [
+ {
+ "matches": [{"path": {"type": "PathPrefix", "value":
"/custom-flower"}}],
+ "backendRefs": [{"name": "external-flower", "port": 8443}],
+ },
+ ]
+
+ def test_should_use_flower_ui_port_for_default_backend(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "ports": {"flowerUI": 9000},
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert jmespath.search("spec.rules[0].backendRefs[0].port", docs[0])
== 9000
+
+ def test_httproute_not_created_when_unset(self):
+ docs = render_chart(
+ values={"executor": "CeleryExecutor", "flower": {"enabled": True,
"httpRoute": {}}},
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert docs == []
+
+ def test_httproute_not_created_when_disabled(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {"enabled": True, "httpRoute": {"enabled": False}},
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert docs == []
+
+ def test_httproute_created_when_enabled(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": True,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert len(docs) == 1
+
+ def test_should_not_render_when_flower_disabled(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
+ "flower": {
+ "enabled": False,
+ "httpRoute": {"enabled": True, "parentRefs":
MINIMAL_PARENT_REFS},
+ },
+ },
+ show_only=SHOW_ONLY,
+ api_versions=GATEWAY_API_VERSIONS,
+ )
+ assert docs == []
+
+ @pytest.mark.parametrize("executor", ["LocalExecutor",
"KubernetesExecutor"])
Review Comment:
```suggestion
@pytest.mark.parametrize("executor", ["LocalExecutor",
"KubernetesExecutor", "LocalExecutor,KubernetesExecutor"])
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
