You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/03/02 17:53:40 UTC
[GitHub] [ozone] adoroszlai opened a new pull request #1984: HDDS-4888. Merge basic CI checks
adoroszlai opened a new pull request #1984:
URL: https://github.com/apache/ozone/pull/1984
## What changes were proposed in this pull request?
1. Merge basic CI checks (`author`, `bats`, etc.) into a single check with matrix build. This reduces code duplication.
2. Add scripts to install check-specific dependencies (eg. `bats` or `spotbugs`). This reduces complexity of Github-specific code in the workflow definition, and allows simpler prototyping/testing.
3. Use Github Actions cache instead of building inside `ozone-build` container. This reduces the effect of outdated dependencies in the image.
4. Prefer check-specific cache because:
* various checks require slightly different dependencies (mostly related to Maven plugins)
* concurrent checks may have problem sharing cache (noticed with _compile (8)_ and _compile (11)_ checks)
If check-specific cache is not available, fallback to shared cache at restore time.
https://issues.apache.org/jira/browse/HDDS-4888
## How was this patch tested?
https://github.com/adoroszlai/hadoop-ozone/actions/runs/614723003
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #1984: HDDS-4888. Merge basic CI checks
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1984:
URL: https://github.com/apache/ozone/pull/1984#issuecomment-819382251
Thanks @elek for the review.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] elek commented on a change in pull request #1984: HDDS-4888. Merge basic CI checks
Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1984:
URL: https://github.com/apache/ozone/pull/1984#discussion_r591714406
##########
File path: hadoop-ozone/dev-support/checks/install/_install.sh
##########
@@ -0,0 +1,34 @@
+#!/usr/bin/env bash
Review comment:
Tricky name, as this script is responsible both for install and executing the scripts.
I am wondering if it would be easier to do a laze download at the beginning of each of the scripts. For example at the beginning of the findbugs.sh we can check if spotbugs exists and download if not.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] elek commented on pull request #1984: HDDS-4888. Merge basic CI checks
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1984:
URL: https://github.com/apache/ozone/pull/1984#issuecomment-809300552
> Unfortunately this does not work for the kubernetes check. K3s installs stuff to global directories (/etc).
>
> It also does not apply to Robot Framework installed via pip.
robotframework can be installed with virtualenv + pip to local directory (As far as I remember it worked for me, but let me know if it doesn't)
Kubernetes is a more tricky question:
flekszible can be installed to local dir
I think the installation of k3s can remain in the github actions (if possible). K3s is just one way to provide kubernetes cluster. I think `hadoop-ozone/dev-support/checks/kubernetes.sh` should assume that we have working kubernetes install (similar that we assume that java and maven are available, we don't install them).
I can imagine that someone would like to execute tests on minikube (or any other one node local k8s cluster).
In github action environment we know that k3s should be installed, but locally it's not so straightforward, therefore I would skip the installation from the script. (We can print out a warning and recommend k3s installation if kubectl is not on the path)
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #1984: HDDS-4888. Merge basic CI checks
Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #1984:
URL: https://github.com/apache/ozone/pull/1984
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #1984: HDDS-4888. Merge basic CI checks
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1984:
URL: https://github.com/apache/ozone/pull/1984#issuecomment-796780535
> With this approach:
>
> 1. we can cache the `./.dev-tools` directory itself which can make next builds faster
> 2. `./.dev-tools` can be populated during the local script executions. Easy to run any test as required tool will be downloaded (and persisted) first time.
Unfortunately this does not work for the _kubernetes_ check. K3s installs stuff to global directories (`/etc`).
It also does not apply to Robot Framework installed via `pip`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #1984: HDDS-4888. Merge basic CI checks
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1984:
URL: https://github.com/apache/ozone/pull/1984#issuecomment-795794410
Thanks @elek for the review. I like the idea of integrated lazy download. I would make it optional (and enable in CI), as developers may have their own preferences for installing tools (eg. via package manager).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a change in pull request #1984: HDDS-4888. Merge basic CI checks
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1984:
URL: https://github.com/apache/ozone/pull/1984#discussion_r591726837
##########
File path: hadoop-ozone/dev-support/checks/install/_install.sh
##########
@@ -0,0 +1,34 @@
+#!/usr/bin/env bash
Review comment:
> Tricky name, as this script is responsible both for install and executing the scripts.
This is just a wrapper to invoke a specific install script. It does not run the check. The only reason for its existence: the workflow file can blindly invoke it without checking for the existence of the specific install script (I'm not even sure it would be possible).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org