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 2020/10/18 15:19:11 UTC
[GitHub] [airflow] potiuk opened a new pull request #11634: Improves stability of K8S tests by caching binaries and repeats
potiuk opened a new pull request #11634:
URL: https://github.com/apache/airflow/pull/11634
The K8S tests on CI are controlled from the host, not from
inside of the CI container image. Therefore it needs virtualenv
to run the tests as well as some tools such as helm, kubectl
and kind. While those tools can bee downloaded and installed
on demand, from time to time the download fails intermittently.
This change introduces the following improvements:
* the commands to download and setup kind, helm, kubectl are
repeated up to 4 times in case they fail
* the "bin" directory where those binaries are downloaded is
cached between runs. Only the same combination of
versions of the tools are sharing the same cache.
This way both cases - regular re-runs of the same jobs and
upgrade of tools will be much more stable.
<!--
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 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/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, 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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #11634: Improves stability of K8S tests by caching binaries and repeats
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #11634:
URL: https://github.com/apache/airflow/pull/11634#discussion_r507189869
##########
File path: scripts/ci/libraries/_repeats.sh
##########
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# 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.
+
+# Repeat the command up to n times in case of failure
+# Parameters:
+# $1 - how many times to repeat
+# $2 - command to repeat (run through bash -c)
+function repeats::repeat_up_to_n_times() {
Review comment:
+1 for `run_with_retry`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #11634: Improves stability of K8S tests by caching binaries and repeats
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11634:
URL: https://github.com/apache/airflow/pull/11634#discussion_r507189023
##########
File path: scripts/ci/libraries/_repeats.sh
##########
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# 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.
+
+# Repeat the command up to n times in case of failure
+# Parameters:
+# $1 - how many times to repeat
+# $2 - command to repeat (run through bash -c)
+function repeats::repeat_up_to_n_times() {
Review comment:
I think about the name of this method, and I don't think the word "repeat" describes its use precisely. What do you think of "run_with_retry" or any other with with "retry" instead of "repeat"??
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #11634: Improves stability of K8S tests by caching binaries and repeats
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11634:
URL: https://github.com/apache/airflow/pull/11634#discussion_r507198632
##########
File path: scripts/ci/libraries/_kind.sh
##########
@@ -66,7 +67,8 @@ function kind::make_sure_kubernetes_tools_are_installed() {
if [[ ! -f "${KUBECTL_BINARY_PATH}" || ${DOWNLOADED_KUBECTL_VERSION} != "${KUBECTL_VERSION}" ]]; then
echo
echo "Downloading Kubectl version ${KUBECTL_VERSION}"
- curl --fail --location "${KUBECTL_URL}" --output "${KUBECTL_BINARY_PATH}"
+ repeats::repeat_up_to_n_times 4 \
Review comment:
```suggestion
repeats::run_with_retry 4 \
```
##########
File path: scripts/ci/libraries/_repeats.sh
##########
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# 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.
+
+# Repeat the command up to n times in case of failure
+# Parameters:
+# $1 - how many times to repeat
+# $2 - command to repeat (run through bash -c)
+function repeats::repeat_up_to_n_times() {
Review comment:
```suggestion
function repeats::run_with_retry() {
```
##########
File path: scripts/ci/libraries/_kind.sh
##########
@@ -51,7 +51,8 @@ function kind::make_sure_kubernetes_tools_are_installed() {
if [[ ! -f "${KIND_BINARY_PATH}" || ${DOWNLOADED_KIND_VERSION} != "${KIND_VERSION}" ]]; then
echo
echo "Downloading Kind version ${KIND_VERSION}"
- curl --fail --location "${KIND_URL}" --output "${KIND_BINARY_PATH}"
+ repeats::repeat_up_to_n_times 4 \
Review comment:
```suggestion
repeats::run_with_retry 4 \
```
##########
File path: scripts/ci/libraries/_kind.sh
##########
@@ -81,8 +83,8 @@ function kind::make_sure_kubernetes_tools_are_installed() {
if [[ ! -f "${HELM_BINARY_PATH}" || ${DOWNLOADED_HELM_VERSION} != "${HELM_VERSION}" ]]; then
echo
echo "Downloading Helm version ${HELM_VERSION}"
- curl --location "${HELM_URL}" |
- tar -xvz -O "${SYSTEM}-amd64/helm" >"${HELM_BINARY_PATH}"
+ repeats::repeat_up_to_n_times 4 \
Review comment:
```suggestion
repeats::run_with_retry 4 \
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #11634: Improves stability of K8S tests by caching binaries and repeats
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11634:
URL: https://github.com/apache/airflow/pull/11634#discussion_r507189023
##########
File path: scripts/ci/libraries/_repeats.sh
##########
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# 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.
+
+# Repeat the command up to n times in case of failure
+# Parameters:
+# $1 - how many times to repeat
+# $2 - command to repeat (run through bash -c)
+function repeats::repeat_up_to_n_times() {
Review comment:
I think about the name of this method, and I don't think the word "repeat" describes its use precisely. What do you think of "run_with_retry"?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11634: Improves stability of K8S tests by caching binaries and repeats
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11634:
URL: https://github.com/apache/airflow/pull/11634#issuecomment-711371180
Hey @kaxil - that one is for stabiility of K8S tests. They fail rather frequently recently due to intermittent download problems - but this fixes it entirely.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #11634: Improves stability of K8S tests by caching binaries and repeats
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #11634:
URL: https://github.com/apache/airflow/pull/11634
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11634: Improves stability of K8S tests by caching binaries and repeats
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11634:
URL: https://github.com/apache/airflow/pull/11634#issuecomment-711412464
All good @kaxil @mik-laj -> another transient test failure that I am going to take a look tomorrow. But the K8S stability is 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #11634: Improves stability of K8S tests by caching binaries and repeats
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11634:
URL: https://github.com/apache/airflow/pull/11634#discussion_r507189023
##########
File path: scripts/ci/libraries/_repeats.sh
##########
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# 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.
+
+# Repeat the command up to n times in case of failure
+# Parameters:
+# $1 - how many times to repeat
+# $2 - command to repeat (run through bash -c)
+function repeats::repeat_up_to_n_times() {
Review comment:
I think about the name of this method, and I don't think the word "repeat" describes its use precisely. What do you think of "run_with_retry" or any other with "retry" word instead of "repeat"?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #11634: Improves stability of K8S tests by caching binaries and repeats
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #11634:
URL: https://github.com/apache/airflow/pull/11634#discussion_r507198882
##########
File path: scripts/ci/libraries/_repeats.sh
##########
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+# 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.
+
+# Repeat the command up to n times in case of failure
+# Parameters:
+# $1 - how many times to repeat
+# $2 - command to repeat (run through bash -c)
+function repeats::repeat_up_to_n_times() {
Review comment:
No problem :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org