HDDS-2400. Enable github actions based builds for Ozone #122
Conversation
|
For some reason builds are not triggered. It's not clear if it should be merged first or it's disabled for this repo... |
anuengineer
left a comment
anuengineer
left a comment
There was a problem hiding this comment.
Some comments/questions below.
| # 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. | ||
| FROM elek/ozone-build:20191106-1 |
There was a problem hiding this comment.
We need to move this to Apache ozone ? at some point of time. Not for this patch. Let us get this working; and then move this to Apache Ozone Account in Docker Hub.
There was a problem hiding this comment.
Agree. I already opened the required PR: apache/ozone-docker#9 It will be available as apache/ozone-build
| jobs: | ||
| build: | ||
| name: compile | ||
| runs-on: ubuntu-18.04 |
There was a problem hiding this comment.
I thought we were using Alpine for most base. But this is good too. Just commenting since I remember you telling me the reason for this change, but I forgot what it was. Something to do with Java or something ?
There was a problem hiding this comment.
Recently we switched to use centos. Centos is used in the runtime images (apache/ozone and apache/ozone-runner) and it seems to be reasonable to run the unit test in the same environment.
We used alpine earlier but we found very strange seg faults from the rocksdb JNI -> libmusl which has been disappeared with using centos.
This specific line is about the VM. Github actions / azure starts a new VM for each step and we run our build inside a docker container (centos) started by the host (ubuntu).
Except for acceptance testing which requires docker (and nothing more) where it was easier to run the docker-compose based tests from the VM itself.
| name: compile | ||
| runs-on: ubuntu-18.04 | ||
| steps: | ||
| - uses: actions/checkout@master |
There was a problem hiding this comment.
Does this mean that patch will get rebased automatically the master? if so, awesome.
There was a problem hiding this comment.
No, It doesn't. But this is the place where we can implement an automatic rebase. The original branch is available from the event file, so we can easily implement it.
| name: unit | ||
| runs-on: ubuntu-18.04 | ||
| steps: | ||
| - uses: actions/checkout@master |
There was a problem hiding this comment.
Should we run this if and only if the build is successful ?
There was a problem hiding this comment.
We can, but this is fast enough, I think we can run it always. But we can define some ordering between build and acceptance/integration which is not yet implemented here.
| path: target/checkstyle | ||
| findbugs: | ||
| name: findbugs | ||
| runs-on: ubuntu-18.04 |
There was a problem hiding this comment.
Same here, should we serialize this behind the build ? that way if there is a compilation error we will see it and bail.
There was a problem hiding this comment.
I added dependencies between acceptance and others. I am not sure if we need other one here as findbugs is very fast it's not a big difference (in terms of resource usage) if findbugs are executed later.
But I am open to do it.
I would start the experiment and measure the resource usage and adjust the dependencies based on the data.
| name: findbugs | ||
| path: target/findbugs | ||
| acceptance: | ||
| name: acceptance |
There was a problem hiding this comment.
Make this depend on unit.sh ? and enforce Unit.sh with vengeance ? All of us will be uncomfortable for a while, but then we will realize that unittests being green is the natural state. what do you think ?
There was a problem hiding this comment.
I added a dependency between acceptance and the fast jobs (unit/findbugs/checkstyle/builds)
| Execute rm -f NOTICE.txt.1 | ||
| Execute ozone sh key get ${protocol}${server}/${volume}/bb1/key1 NOTICE.txt.1 | ||
| Execute ls -l NOTICE.txt.1 | ||
| Execute rm -f /tmp/NOTICE.txt.1 |
There was a problem hiding this comment.
Sorry, I don't understand what is happening here. Did we remap /opt to /tmp somehow? and the file name is also remapped ?
There was a problem hiding this comment.
To make the acceptance test execute simple I start the acceptance tests directly from the VM (uid=1001) and not from docker container (uid=1000). But the build has been done with uid=1000. To make it possible to run I use /tmp for writing/reading temporary files which is always writable.
|
Seems to be fine, I think it's ready to merge. The acceptance test is failing on the trunk, too. |
anuengineer
left a comment
anuengineer
left a comment
There was a problem hiding this comment.
+1. LGTM. Thank for getting this done. This is huge. I really love this work.
2 participants
What changes were proposed in this pull request?
Current PR checks are executed in a private branch based on the scripts in https://github.com/elek/argo-ozone
but the results are stored in a public repositories:
As we discussed during the community calls, it would be great to use github actions (or any other cloud based build) to make all the build definitions more accessible for the community.
@vivekratnavel checked CircleCI which has better reporting capabilities. But INFRA has concerns about the permission model of circle-ci:
See:
Fortunately we have a clear contract. Or build scripts are stored under
hadoop-ozone/dev-support/checks(return code show the result, details are printed out to the console output). It's very easy to experiment with different build systems.Github action seems to be an obvious choice: it's integrated well with GitHub and it has more generous resource limitations.
With this Jira I propose to enable github actions based PR checks for a few tests (author, rat, unit, acceptance, checkstyle, findbugs) as an experiment.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2400
How was this patch tested?
You should see the result of the github actions based build on the last commit of this PR.