Repository navigation
HDDS-17509. Replace reusable check.yml workflow with an action - #11946
adoroszlai wants to merge 19 commits into
Conversation
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for working on this! @adoroszlai I found a few spots where steps from check.yml may not have carried over, see inline comments. 🙏
| if: ${{ !cancelled() }} | ||
| uses: actions/upload-artifact@v7 | ||
| with: | ||
| name: ${{ (inputs.split && format('{0}-{1}', inputs.script, inputs.split)) || inputs.script }} |
There was a problem hiding this comment.
I noticed in the CI run that acceptance-ozone/test-s3a.sh was actually failed because artifact name is not valid. It's green because of continue-on-error.
Should we keep the name sanitizing from the old Calculate artifact name step?
| uses: actions/checkout@v7 | ||
| with: | ||
| persist-credentials: false | ||
| ref: ${{ env.SHA }} |
There was a problem hiding this comment.
Should we set SHA from needs.build-info.outputs.sha somewhere? I couldn't find it defined.
| timeout-minutes: 60 | ||
| steps: | ||
| - name: Run check | ||
| uses: $/.github/actions/check |
There was a problem hiding this comment.
Should the Hadoop/Ranger cache restore move here? The old check.yml steps seem to be dropped.
| java-version: ${{ matrix.java }} | ||
| needs-ozone-source-tarball: true | ||
| ratis-args: ${{ inputs.ratis_args }} | ||
| runner: ${{ matrix.os }} |
There was a problem hiding this comment.
I wonder if runs-on should use matrix.os here? The action has no runner input. Currently, the compile (21, macos-15) ran on ubuntu-24.04.
| start_end::group_start "Check if everything should be run" | ||
| local pattern_array=( | ||
| "^.github/workflows/check.yml" | ||
| "^.github/actions/check/action.yml" |
There was a problem hiding this comment.
Should this be action.yaml?
| "^.github/actions/check/action.yml" | |
| "^.github/actions/check/action.yaml" |
| # `env` context cannot be used when calling reusable workflow, so we need to convert these to `outputs` | ||
| build-args: ${{ env.BUILD_ARGS }} | ||
| java-version: ${{ env.TEST_JAVA_VERSION }} | ||
| with-coverage: ${{ env.OZONE_WITH_COVERAGE }} |
There was a problem hiding this comment.
nit: I think we can probably drop the build-args, java-version and with-coverage here since jobs now read env directly, so they and the comment above look unused.
What changes were proposed in this pull request?
Replace
workflows/check.yml(a reusable workflow) withactions/check/action.yaml(a reusable action).Benefits:
builduploads binary artifacts to be consumed by other jobs, this part previously had to be hard-coded incheck.yml.Drawbacks:
runs-oncannot use environment variables defined in the workflowhttps://issues.apache.org/jira/browse/HDDS-17509
How was this patch tested?
CI:
https://github.com/adoroszlai/ozone/actions/runs/42025404878