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/08/02 18:30:16 UTC
[GitHub] [airflow] rrcrrcrrc opened a new pull request, #25484: HostAliases support for scheduler and webserver
rrcrrcrrc opened a new pull request, #25484:
URL: https://github.com/apache/airflow/pull/25484
This PR adds `scheduler.hostAliases` and `webserver.hostAliases` parameter in helm chart in order to use [kubernetes HostAliases](https://kubernetes.io/docs/concepts/services-networking/add-entries-to-pod-etc-hosts-with-host-aliases/) in scheduler and webserver pods.
--
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] rrcrrcrrc commented on pull request #25484: HostAliases support for Scheduler, Worker, Trigger and Webserver
Posted by GitBox <gi...@apache.org>.
rrcrrcrrc commented on PR #25484:
URL: https://github.com/apache/airflow/pull/25484#issuecomment-1213295144
> Looks like you accidentally deleted some non-helm tests?
Yes @jedcunningham , my fault, just wanna change the "indent" to "nindent". Can someone do it for all? This week I got not PC
--
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] rrcrrcrrc commented on pull request #25484: HostAliases support for Scheduler, Worker, Trigger and Webserver
Posted by GitBox <gi...@apache.org>.
rrcrrcrrc commented on PR #25484:
URL: https://github.com/apache/airflow/pull/25484#issuecomment-1236196305
Hi, I don´t know what to do this this PR rigth now.. Literally is my first PR
--
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 #25484: HostAliases support for scheduler and webserver
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25484:
URL: https://github.com/apache/airflow/pull/25484#discussion_r940776096
##########
tests/charts/test_webserver.py:
##########
@@ -216,6 +216,19 @@ def test_should_add_extra_init_containers(self):
"image": "test-registry/test-repo:test-tag",
} == jmespath.search("spec.template.spec.initContainers[-1]", docs[0])
+ def test_webserver_host_aliases(self):
+ docs = render_chart(
+ values={
+ "webserver": {
+ "hostAliases": [{"ip": "127.0.0.2", "hostnames": ["test.hostname"]}],
+ },
+ },
+ show_only=["templates/webserver/webserver-deployment.yaml"],
+ )
+
+ assert "127.0.0.2" == jmespath.search("spec.template.spec.hostAliases[0].ip", docs[0])
+ assert "test.hostname" == jmespath.search("spec.template.spec.hostAliases[0].hostnames[0]", docs[0])
+
Review Comment:
```suggestion
```
--
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] rrcrrcrrc commented on pull request #25484: HostAliases support for scheduler and webserver
Posted by GitBox <gi...@apache.org>.
rrcrrcrrc commented on PR #25484:
URL: https://github.com/apache/airflow/pull/25484#issuecomment-1203124677
Can we add this feature to Helm Chart 1.7.0?
--
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] potiuk commented on pull request #25484: HostAliases support for scheduler and webserver
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25484:
URL: https://github.com/apache/airflow/pull/25484#issuecomment-1203173665
> Can we add this feature to Helm Chart 1.7.0?
First it would have to be reviewed and apprved as all other PRs. You need to exercise a bit of patience to get there.
--
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 #25484: HostAliases support for scheduler and webserver
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25484:
URL: https://github.com/apache/airflow/pull/25484#discussion_r940931232
##########
tests/charts/test_webserver.py:
##########
@@ -216,6 +216,18 @@ def test_should_add_extra_init_containers(self):
"image": "test-registry/test-repo:test-tag",
} == jmespath.search("spec.template.spec.initContainers[-1]", docs[0])
+ def test_webserver_host_aliases(self):
+ docs = render_chart(
+ values={
+ "webserver": {
+ "hostAliases": [{"ip": "127.0.0.2", "hostnames": ["test.hostname"]}],
+ },
+ },
+ show_only=["templates/webserver/webserver-deployment.yaml"],
+ )
+
+ assert "127.0.0.2" == jmespath.search("spec.template.spec.hostAliases[0].ip", docs[0])
+ assert "test.hostname" == jmespath.search("spec.template.spec.hostAliases[0].hostnames[0]", docs[0])
Review Comment:
```suggestion
assert "test.hostname" == jmespath.search("spec.template.spec.hostAliases[0].hostnames[0]", docs[0])
```
--
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] rrcrrcrrc commented on pull request #25484: HostAliases support for scheduler and webserver
Posted by GitBox <gi...@apache.org>.
rrcrrcrrc commented on PR #25484:
URL: https://github.com/apache/airflow/pull/25484#issuecomment-1207193948
Already edited what you comment and added hostAlises to the triggerer component too. This would make hostAliases available for Scheduler, Worker, Triggerer and Webserver.
--
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 #25484: HostAliases support for scheduler and webserver
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25484:
URL: https://github.com/apache/airflow/pull/25484#discussion_r937094291
##########
tests/charts/test_scheduler.py:
##########
@@ -115,6 +115,20 @@ def test_should_add_extra_volume_and_extra_volume_mount(self):
"spec.template.spec.containers[0].volumeMounts[*].name", docs[0]
)
+ def test_scheduler_host_aliases(self):
+ docs = render_chart(
+ values={
+ "executor": "CeleryExecutor",
Review Comment:
```suggestion
```
Don't need that, it's the default.
##########
chart/values.schema.json:
##########
@@ -1802,6 +1802,28 @@
"$ref": "#/definitions/io.k8s.api.core.v1.Toleration"
}
},
+ "hostAliases": {
+ "description": "Specify HostAliases for scheduler.",
+ "items": {
+ "$ref": "#/definitions/io.k8s.api.core.v1.HostAlias"
+ },
+ "type": "array",
+ "default": [],
+ "examples": [
+ {
+ "ip": "127.0.0.2",
+ "hostnames": [
+ "test.hostname.one"
+ ]
+ },
+ {
+ "ip": "127.0.0.3",
+ "hostnames": [
+ "test.hostname.two"
+ ]
Review Comment:
```suggestion
```
Here as well.
##########
chart/values.yaml:
##########
@@ -685,6 +685,16 @@ scheduler:
# weight: 100
tolerations: []
topologySpreadConstraints: []
+ # hostAliases to use in scheduler pods.
+ # See:
+ # https://kubernetes.io/docs/concepts/services-networking/add-entries-to-pod-etc-hosts-with-host-aliases/
Review Comment:
I think we can get away without these.
```suggestion
```
##########
chart/values.yaml:
##########
@@ -685,6 +685,16 @@ scheduler:
# weight: 100
tolerations: []
topologySpreadConstraints: []
+ # hostAliases to use in scheduler pods.
+ # See:
+ # https://kubernetes.io/docs/concepts/services-networking/add-entries-to-pod-etc-hosts-with-host-aliases/
+ hostAliases: []
+ # - ip: "127.0.0.2"
+ # hostnames:
+ # - "test.hostname.one"
+ # - ip: "127.0.0.3"
+ # hostnames:
+ # - "test.hostname.two"
Review Comment:
```suggestion
```
I think one example should be sufficient?
--
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 #25484: HostAliases support for scheduler and webserver
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25484:
URL: https://github.com/apache/airflow/pull/25484#discussion_r940397615
##########
chart/templates/triggerer/triggerer-deployment.yaml:
##########
@@ -99,7 +99,11 @@ spec:
tolerations:
{{- toYaml $tolerations | nindent 8 }}
topologySpreadConstraints:
- {{- toYaml $topologySpreadConstraints | nindent 8 }}
+ {{- toYaml $topologySpreadConstraints | nindent 8 }}
Review Comment:
```suggestion
{{- toYaml $topologySpreadConstraints | nindent 8 }}
```
--
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] potiuk commented on pull request #25484: HostAliases support for Scheduler, Worker, Trigger and Webserver
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #25484:
URL: https://github.com/apache/airflow/pull/25484#issuecomment-1241451683
> Hi, I don´t know what to do this this PR rigth now.. Literally is my first PR
If you deleted stuff accidentally, If you have no idea where things get wrong - close it, open a new one and start from scratch copying new code. This is nothing Airflow specific (though we use rebase workflow) - but you can read CONTRIBUTING.rst and possibly search the internet on rebasing, solving conflicts etc. If you have no experience, then start small and take your time to learn. No hurry - you can even take online courses. When you want to contribute to open-source you need ot be self-driven , self-learning and self sufficient and while some help and guidance is given, usually it is in the form of comments in PR for concrete changes or documentation (like CONTRIBUTING) written down for you to read and learn rathe than individual hand-holding and instructing step-by-step
--
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] boring-cyborg[bot] commented on pull request #25484: HostAliases support for scheduler and webserver
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #25484:
URL: https://github.com/apache/airflow/pull/25484#issuecomment-1203075336
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 Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/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/docs/apache-airflow/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/BREEZE.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.rst#coding-style-and-best-practices).
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
[GitHub] [airflow] rrcrrcrrc closed pull request #25484: HostAliases support for Scheduler, Worker, Trigger and Webserver
Posted by GitBox <gi...@apache.org>.
rrcrrcrrc closed pull request #25484: HostAliases support for Scheduler, Worker, Trigger and Webserver
URL: https://github.com/apache/airflow/pull/25484
--
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 #25484: HostAliases support for scheduler and webserver
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25484:
URL: https://github.com/apache/airflow/pull/25484#discussion_r940631728
##########
chart/values.schema.json:
##########
@@ -2148,6 +2158,22 @@
"$ref": "#/definitions/io.k8s.api.core.v1.TopologySpreadConstraint"
}
},
+ "hostAliases": {
+ "description": "Specify HostAliases for trigerer.",
Review Comment:
```suggestion
"description": "Specify HostAliases for triggerer.",
```
--
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