You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/07/13 18:35:05 UTC
[GitHub] [airflow] rishkarajgi opened a new pull request, #25031: added labels to airflow specific pods
rishkarajgi opened a new pull request, #25031:
URL: https://github.com/apache/airflow/pull/25031
<!--
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/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.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+Improvements+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 [newsfragments](https://github.com/apache/airflow/tree/main/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
[GitHub] [airflow] jedcunningham commented on a diff in pull request #25031: added labels to airflow specific pods
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25031:
URL: https://github.com/apache/airflow/pull/25031#discussion_r952732539
##########
tests/charts/test_webserver.py:
##########
@@ -808,3 +816,61 @@ def test_deprecated_from_param(self):
assert [{"namespaceSelector": {"matchLabels": {"release": "myrelease"}}}] == jmespath.search(
"spec.ingress[0].from", docs[0]
)
+
+ def test_should_add_component_specific_labels(self):
+ docs = render_chart(
+ values={
+ "networkPolicies": {"enabled": True},
+ "webserver": {
+ "labels": {"test_label": "test_label_value"},
+ },
+ },
+ show_only=["templates/webserver/webserver-networkpolicy.yaml"],
+ )
+ assert "test_label" in jmespath.search("metadata.labels", docs[0])
+ assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+
+class WebserverIngressTest(unittest.TestCase):
Review Comment:
There are separate files for ingess tests, e.g. `test_ingress_web.py`.
##########
tests/charts/test_webserver.py:
##########
@@ -808,3 +816,61 @@ def test_deprecated_from_param(self):
assert [{"namespaceSelector": {"matchLabels": {"release": "myrelease"}}}] == jmespath.search(
"spec.ingress[0].from", docs[0]
)
+
+ def test_should_add_component_specific_labels(self):
+ docs = render_chart(
+ values={
+ "networkPolicies": {"enabled": True},
+ "webserver": {
+ "labels": {"test_label": "test_label_value"},
+ },
+ },
+ show_only=["templates/webserver/webserver-networkpolicy.yaml"],
+ )
+ assert "test_label" in jmespath.search("metadata.labels", docs[0])
+ assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+
+class WebserverIngressTest(unittest.TestCase):
+ def test_should_add_component_specific_labels(self):
+ docs = render_chart(
+ values={
+ "ingress": {"enabled": True},
+ "webserver": {
+ "labels": {"test_label": "test_label_value"},
+ },
+ },
+ show_only=["templates/webserver/webserver-ingress.yaml"],
+ )
+ assert "test_label" in jmespath.search("metadata.labels", docs[0])
+ assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+
+class WebserverPodDisruptionBudgetTest(unittest.TestCase):
Review Comment:
Same with these, e.g. `test_pdb_webserver.py`.
--
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
[GitHub] [airflow] rishkarajgi commented on pull request #25031: added labels to airflow specific pods
Posted by GitBox <gi...@apache.org>.
rishkarajgi commented on PR #25031:
URL: https://github.com/apache/airflow/pull/25031#issuecomment-1223014511
> Component specific labels should go on all objects, not just pods, like the global labels do. Also there are merge conflicts currently.
@jedcunningham Have addressed the above. Please take a look
--
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
[GitHub] [airflow] jedcunningham merged pull request #25031: added labels to airflow specific pods
Posted by GitBox <gi...@apache.org>.
jedcunningham merged PR #25031:
URL: https://github.com/apache/airflow/pull/25031
--
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
[GitHub] [airflow] pgvishnuram commented on pull request #25031: added labels to airflow specific pods
Posted by GitBox <gi...@apache.org>.
pgvishnuram commented on PR #25031:
URL: https://github.com/apache/airflow/pull/25031#issuecomment-1220874790
@jedcunningham - pls see if this can be merged
--
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
[GitHub] [airflow] rishkarajgi commented on a diff in pull request #25031: added labels to airflow specific pods
Posted by GitBox <gi...@apache.org>.
rishkarajgi commented on code in PR #25031:
URL: https://github.com/apache/airflow/pull/25031#discussion_r952172338
##########
chart/values.yaml:
##########
@@ -586,6 +586,9 @@ workers:
podAnnotations: {}
+ # Labels specific to workers
Review Comment:
done
--
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
[GitHub] [airflow] rishkarajgi commented on a diff in pull request #25031: added labels to airflow specific pods
Posted by GitBox <gi...@apache.org>.
rishkarajgi commented on code in PR #25031:
URL: https://github.com/apache/airflow/pull/25031#discussion_r952756072
##########
tests/charts/test_webserver.py:
##########
@@ -808,3 +816,61 @@ def test_deprecated_from_param(self):
assert [{"namespaceSelector": {"matchLabels": {"release": "myrelease"}}}] == jmespath.search(
"spec.ingress[0].from", docs[0]
)
+
+ def test_should_add_component_specific_labels(self):
+ docs = render_chart(
+ values={
+ "networkPolicies": {"enabled": True},
+ "webserver": {
+ "labels": {"test_label": "test_label_value"},
+ },
+ },
+ show_only=["templates/webserver/webserver-networkpolicy.yaml"],
+ )
+ assert "test_label" in jmespath.search("metadata.labels", docs[0])
+ assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+
+class WebserverIngressTest(unittest.TestCase):
+ def test_should_add_component_specific_labels(self):
+ docs = render_chart(
+ values={
+ "ingress": {"enabled": True},
+ "webserver": {
+ "labels": {"test_label": "test_label_value"},
+ },
+ },
+ show_only=["templates/webserver/webserver-ingress.yaml"],
+ )
+ assert "test_label" in jmespath.search("metadata.labels", docs[0])
+ assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+
+class WebserverPodDisruptionBudgetTest(unittest.TestCase):
Review Comment:
done
##########
tests/charts/test_webserver.py:
##########
@@ -808,3 +816,61 @@ def test_deprecated_from_param(self):
assert [{"namespaceSelector": {"matchLabels": {"release": "myrelease"}}}] == jmespath.search(
"spec.ingress[0].from", docs[0]
)
+
+ def test_should_add_component_specific_labels(self):
+ docs = render_chart(
+ values={
+ "networkPolicies": {"enabled": True},
+ "webserver": {
+ "labels": {"test_label": "test_label_value"},
+ },
+ },
+ show_only=["templates/webserver/webserver-networkpolicy.yaml"],
+ )
+ assert "test_label" in jmespath.search("metadata.labels", docs[0])
+ assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value"
+
+
+class WebserverIngressTest(unittest.TestCase):
Review Comment:
done
--
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
[GitHub] [airflow] jedcunningham commented on a diff in pull request #25031: added labels to airflow specific pods
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25031:
URL: https://github.com/apache/airflow/pull/25031#discussion_r951924404
##########
tests/charts/test_flower.py:
##########
@@ -280,6 +280,25 @@ def test_should_add_extraEnvs(self):
"spec.template.spec.containers[0].env", docs[0]
)
+ def test_should_add_component_specific_labels(self):
+ docs = render_chart(
+ values={
+ "flower": {
+ "enabled": True,
+ "labels": {"test_label": "test_label_value"},
+ },
+ },
+ show_only=["templates/flower/flower-deployment.yaml"],
Review Comment:
You need to check all of the flower templates.
##########
chart/values.yaml:
##########
@@ -586,6 +586,9 @@ workers:
podAnnotations: {}
+ # Labels specific to workers
Review Comment:
```suggestion
# Labels specific to workers objects and pods
```
Should be like this across the board, also in the schema description.
--
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
[GitHub] [airflow] rishkarajgi commented on a diff in pull request #25031: added labels to airflow specific pods
Posted by GitBox <gi...@apache.org>.
rishkarajgi commented on code in PR #25031:
URL: https://github.com/apache/airflow/pull/25031#discussion_r952281580
##########
tests/charts/test_flower.py:
##########
@@ -280,6 +280,25 @@ def test_should_add_extraEnvs(self):
"spec.template.spec.containers[0].env", docs[0]
)
+ def test_should_add_component_specific_labels(self):
+ docs = render_chart(
+ values={
+ "flower": {
+ "enabled": True,
+ "labels": {"test_label": "test_label_value"},
+ },
+ },
+ show_only=["templates/flower/flower-deployment.yaml"],
Review Comment:
done. Added for the rest of the pods as well
--
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