You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "atul-ram (via GitHub)" <gi...@apache.org> on 2024/03/04 07:14:21 UTC
[I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
atul-ram opened a new issue, #37872:
URL: https://github.com/apache/airflow/issues/37872
### Official Helm Chart version
1.12.0 (latest released)
### Apache Airflow version
2.8
### Kubernetes Version
1.27.7
### Helm Chart configuration
no changes to values.yaml
### Docker Image customizations
NA
### What happened
the output scheduler definition has repeated annotation.
```
cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
```
### What you think should happen instead
the output file should not have repeated annotation value for respective objects
```
cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
```
### How to reproduce
helm template demo apache-airflow/airflow --version 1.12.0 --namespace demo > output.yaml
and check output file for annotation
cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
### Anything else
similarly check for same annotation for other types.
### Are you willing to submit PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "brokenjacobs (via GitHub)" <gi...@apache.org>.
brokenjacobs commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979704151
1: Is there a way to subscribe to RC's for helm on this repo? I kept checking to test and didn't catch the RC when it happened.
2: Can we implement a yaml lint pass in CI from a helm template command with at least the default values? It would catch a bunch of this stuff.
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979802312
I am afraid @jedcunningham (I know you love it) that it means 1.13.1 release for the chart :(
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979730359
(BTW. does it break anything for you if the annotation is duplicated ? Is there some deployment that complains about it ?
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1975872865
Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979663447
From here:
```
{{- $safeToEvict := dict "cluster-autoscaler.kubernetes.io/safe-to-evict" (.Values.workers.safeToEvict | toString) }}
{{- $podAnnotations := mergeOverwrite .Values.airflowPodAnnotations $safeToEvict .Values.workers.podAnnotations }}
```
Does it break anything ?
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979681529
Generally mergeIverwrite updates `.Values.airflowPodAnnotations` and theose `.Values.airflowPodAnnoitations` are then later used in triggerer and scheduler
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "brokenjacobs (via GitHub)" <gi...@apache.org>.
brokenjacobs commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979753375
Yes this breaks deployments with fluxcd. It won't install the chart.
I went and checked for what is going here, and it looks like yamllint isn't the correct tool, it's kubeconform in strict mode. See: https://github.com/apache/airflow/issues/37919
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979738033
Interesting thing is that yamllint catches quite a lot more errors - some of them can be fixed by allowing longer line length , but some - trailing spaces, and indentation likely need some fixes in the templates, so if you would like to take it on, it's up for grabs.
```
145:81 error line too long (92 > 80 characters) (line-length)
162:81 error line too long (124 > 80 characters) (line-length)
181:81 error line too long (86 > 80 characters) (line-length)
201:24 error trailing spaces (trailing-spaces)
203:1 error trailing spaces (trailing-spaces)
206:1 error trailing spaces (trailing-spaces)
213:1 error trailing spaces (trailing-spaces)
217:1 error trailing spaces (trailing-spaces)
222:1 error trailing spaces (trailing-spaces)
228:1 error trailing spaces (trailing-spaces)
237:1 error trailing spaces (trailing-spaces)
244:1 error trailing spaces (trailing-spaces)
248:1 error trailing spaces (trailing-spaces)
254:1 error trailing spaces (trailing-spaces)
262:1 error trailing spaces (trailing-spaces)
265:1 error trailing spaces (trailing-spaces)
269:1 error trailing spaces (trailing-spaces)
271:1 error trailing spaces (trailing-spaces)
273:1 error trailing spaces (trailing-spaces)
276:81 error line too long (111 > 80 characters) (line-length)
319:6 error trailing spaces (trailing-spaces)
326:1 error trailing spaces (trailing-spaces)
332:1 error trailing spaces (trailing-spaces)
338:1 error trailing spaces (trailing-spaces)
344:1 error trailing spaces (trailing-spaces)
350:1 error trailing spaces (trailing-spaces)
355:1 error trailing spaces (trailing-spaces)
360:1 error trailing spaces (trailing-spaces)
365:1 error trailing spaces (trailing-spaces)
370:1 error trailing spaces (trailing-spaces)
375:1 error trailing spaces (trailing-spaces)
380:1 error trailing spaces (trailing-spaces)
385:1 error trailing spaces (trailing-spaces)
684:1 error trailing spaces (trailing-spaces)
721:81 error line too long (88 > 80 characters) (line-length)
723:81 error line too long (90 > 80 characters) (line-length)
750:81 error line too long (98 > 80 characters) (line-length)
751:81 error line too long (104 > 80 characters) (line-length)
752:81 error line too long (106 > 80 characters) (line-length)
753:81 error line too long (97 > 80 characters) (line-length)
754:81 error line too long (99 > 80 characters) (line-length)
755:81 error line too long (96 > 80 characters) (line-length)
757:9 error duplication of key "cluster-autoscaler.kubernetes.io/safe-to-evict" in mapping (key-duplicates)
764:11 error wrong indentation: expected 12 but found 10 (indentation)
777:23 error trailing spaces (trailing-spaces)
786:27 error trailing spaces (trailing-spaces)
800:16 error trailing spaces (trailing-spaces)
805:19 error trailing spaces (trailing-spaces)
807:15 error trailing spaces (trailing-spaces)
810:24 error trailing spaces (trailing-spaces)
819:81 error line too long (82 > 80 characters) (line-length)
850:27 error trailing spaces (trailing-spaces)
855:16 error trailing spaces (trailing-spaces)
859:19 error trailing spaces (trailing-spaces)
861:15 error trailing spaces (trailing-spaces)
864:24 error trailing spaces (trailing-spaces)
873:81 error line too long (82 > 80 characters) (line-length)
898:34 error trailing spaces (trailing-spaces)
905:23 error trailing spaces (trailing-spaces)
909:81 error line too long (103 > 80 characters) (line-length)
916:23 error trailing spaces (trailing-spaces)
920:81 error line too long (103 > 80 characters) (line-length)
944:27 error trailing spaces (trailing-spaces)
1005:23 error trailing spaces (trailing-spaces)
1012:27 error trailing spaces (trailing-spaces)
1017:16 error trailing spaces (trailing-spaces)
1090:81 error line too long (98 > 80 characters) (line-length)
1091:81 error line too long (106 > 80 characters) (line-length)
1092:81 error line too long (103 > 80 characters) (line-length)
1093:81 error line too long (97 > 80 characters) (line-length)
1094:81 error line too long (99 > 80 characters) (line-length)
1095:81 error line too long (99 > 80 characters) (line-length)
1096:81 error line too long (96 > 80 characters) (line-length)
1105:11 error wrong indentation: expected 12 but found 10 (indentation)
1116:23 error trailing spaces (trailing-spaces)
1125:27 error trailing spaces (trailing-spaces)
1139:16 error trailing spaces (trailing-spaces)
1144:19 error trailing spaces (trailing-spaces)
1146:15 error trailing spaces (trailing-spaces)
1149:24 error trailing spaces (trailing-spaces)
1158:81 error line too long (82 > 80 characters) (line-length)
1188:27 error trailing spaces (trailing-spaces)
1241:19 error trailing spaces (trailing-spaces)
1243:15 error trailing spaces (trailing-spaces)
1246:24 error trailing spaces (trailing-spaces)
1255:81 error line too long (82 > 80 characters) (line-length)
1280:34 error trailing spaces (trailing-spaces)
1322:1 error trailing spaces (trailing-spaces)
1325:1 error trailing spaces (trailing-spaces)
1337:1 error trailing spaces (trailing-spaces)
1350:15 error wrong indentation: expected 16 but found 14 (indentation)
1422:81 error line too long (108 > 80 characters) (line-length)
1488:23 error trailing spaces (trailing-spaces)
1494:27 error trailing spaces (trailing-spaces)
1553:81 error line too long (98 > 80 characters) (line-length)
1554:81 error line too long (106 > 80 characters) (line-length)
1555:81 error line too long (97 > 80 characters) (line-length)
1556:81 error line too long (99 > 80 characters) (line-length)
1557:81 error line too long (96 > 80 characters) (line-length)
1559:9 error duplication of key "cluster-autoscaler.kubernetes.io/safe-to-evict" in mapping (key-duplicates)
1566:11 error wrong indentation: expected 12 but found 10 (indentation)
1579:23 error trailing spaces (trailing-spaces)
1588:27 error trailing spaces (trailing-spaces)
1602:16 error trailing spaces (trailing-spaces)
1607:19 error trailing spaces (trailing-spaces)
1609:15 error trailing spaces (trailing-spaces)
1612:24 error trailing spaces (trailing-spaces)
1621:81 error line too long (82 > 80 characters) (line-length)
1651:27 error trailing spaces (trailing-spaces)
1656:16 error trailing spaces (trailing-spaces)
1673:19 error trailing spaces (trailing-spaces)
1675:15 error trailing spaces (trailing-spaces)
1678:24 error trailing spaces (trailing-spaces)
1687:81 error line too long (82 > 80 characters) (line-length)
1713:1 error trailing spaces (trailing-spaces)
1720:23 error trailing spaces (trailing-spaces)
1724:81 error line too long (103 > 80 characters) (line-length)
1734:27 error trailing spaces (trailing-spaces)
1792:81 error line too long (98 > 80 characters) (line-length)
1793:81 error line too long (104 > 80 characters) (line-length)
1794:81 error line too long (106 > 80 characters) (line-length)
1795:81 error line too long (103 > 80 characters) (line-length)
1796:81 error line too long (98 > 80 characters) (line-length)
1797:81 error line too long (97 > 80 characters) (line-length)
1798:81 error line too long (99 > 80 characters) (line-length)
1799:81 error line too long (96 > 80 characters) (line-length)
1807:11 error wrong indentation: expected 12 but found 10 (indentation)
1820:23 error trailing spaces (trailing-spaces)
1829:27 error trailing spaces (trailing-spaces)
1843:16 error trailing spaces (trailing-spaces)
1848:19 error trailing spaces (trailing-spaces)
1850:15 error trailing spaces (trailing-spaces)
1853:24 error trailing spaces (trailing-spaces)
1862:81 error line too long (82 > 80 characters) (line-length)
1892:27 error trailing spaces (trailing-spaces)
1897:16 error trailing spaces (trailing-spaces)
1914:81 error line too long (176 > 80 characters) (line-length)
1929:19 error trailing spaces (trailing-spaces)
1932:81 error line too long (102 > 80 characters) (line-length)
1934:25 error trailing spaces (trailing-spaces)
1937:24 error trailing spaces (trailing-spaces)
1946:81 error line too long (82 > 80 characters) (line-length)
1971:34 error trailing spaces (trailing-spaces)
1975:27 error trailing spaces (trailing-spaces)
1980:16 error trailing spaces (trailing-spaces)
2260:81 error line too long (85 > 80 characters) (line-length)
2268:81 error line too long (97 > 80 characters) (line-length)
2338:23 error trailing spaces (trailing-spaces)
2355:27 error trailing spaces (trailing-spaces)
2360:16 error trailing spaces (trailing-spaces)
2379:19 error trailing spaces (trailing-spaces)
2381:15 error trailing spaces (trailing-spaces)
2384:24 error trailing spaces (trailing-spaces)
2393:81 error line too long (82 > 80 characters) (line-length)
2418:34 error trailing spaces (trailing-spaces)
2464:23 error trailing spaces (trailing-spaces)
2481:27 error trailing spaces (trailing-spaces)
2492:19 error trailing spaces (trailing-spaces)
2494:15 error trailing spaces (trailing-spaces)
2499:25 error trailing spaces (trailing-spaces)
2508:81 error line too long (82 > 80 characters) (line-length)
```
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979734128
And of course the duplicate key is not caught by `helm lint`:
```
helm lint . -f values.yaml
==> Linting .
1 chart(s) linted, 0 chart(s) failed
```
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "brokenjacobs (via GitHub)" <gi...@apache.org>.
brokenjacobs commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979698013
I think the issue here is that mergeOverwrite doesn't return a value, it overwrites into .Values.airflowPodAnnotations.
```
Nested objects that are merged are the same instance on both dicts. If you want a deep copy along with the merge than use the deepCopy function along with merging.
```
So this should be:
```
{{- $podAnnotations := mergeOverwrite (deepCopy .Values.airflowPodAnnotations) $safeToEvict .Values.workers.podAnnotations }}
```
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "brokenjacobs (via GitHub)" <gi...@apache.org>.
brokenjacobs commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979754344
> Interesting thing is that yamllint catches quite a lot more errors - some of them can be fixed by allowing longer line length , but some - trailing spaces, and indentation likely need some fixes in the templates, so if you would like to take it on, it's up for grabs.
These rules are configurable in yamllint, but ultimately I think kubeconform is the better tool.
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed issue #37872: the value safeToEvict: true for scheduler is reflected two times.
URL: https://github.com/apache/airflow/issues/37872
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "brokenjacobs (via GitHub)" <gi...@apache.org>.
brokenjacobs commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979690335
It appears what is happening is this line in the worker deployment is overriding the global pod annotations:
https://github.com/apache/airflow/blob/17cc4c911c17e1d18061cdf805d854a3fda66b15/chart/templates/workers/worker-deployment.yaml#L42
Just tested removing podAnnotations from the workers and disabling the safe to evict and the copying of resources stopped.
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979728335
> 1: Is there a way to subscribe to RC's for helm on this repo? I kept checking to test and didn't ca the RC when it happened.
Yes. Subscribe to devlist - https://airflow.apache.org/community/ - last time it was https://lists.apache.org/thread/0xgxsjwsbd8qk3h1tn4gj01hoplz1p5k (and it had this issue https://github.com/apache/airflow/issues/37844 where we ask people to comment on the RC
2. We already implement `helmlint`. https://github.com/apache/airflow/blob/main/.pre-commit-config.yaml#L768 which uses `helm lint` under the hood https://github.com/apache/airflow/blob/main/scripts/ci/pre_commit/pre_commit_helm_lint.py
I doubt yamllint will do a better job - those annotations are just duplicate entries whicha are perfectly valid yaml. But yes - if you think addling yamlint is also good idea (it does detect duplicate keys) - then absolutely - feel free. Would you like to do it ?
--
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: [I] the value safeToEvict: true for scheduler is reflected two times. [airflow]
Posted by "brokenjacobs (via GitHub)" <gi...@apache.org>.
brokenjacobs commented on issue #37872:
URL: https://github.com/apache/airflow/issues/37872#issuecomment-1979637030
this also happens in the triggerer, in 1.13.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