Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure Github workflow runs on docker image used by Production Distribution Build #3868

Closed
wants to merge 1 commit into from
Closed

Ensure Github workflow runs on docker image used by Production Distribution Build #3868

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 45 additions & 6 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ env:
CI_ENVIRONMENT: normal

jobs:
Get-CI-Image-Tag:
uses: opensearch-project/opensearch-build/.github/workflows/get-ci-image-tag.yml@main
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets use a published task not a direct repo reference

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... may you share the example? (PS: this step is replicated to all repos)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like tightly coupling these repositories together; security depends on opensearch-build, opensearch build depends on security - this is a formula to get into a broken state and have to start bypassing CI.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, here is a GitHub action that allows for multi-platform file download [1] it shouldn't be part of the security project even though it was originally written to support this project.

I have trouble adopting this level of coupling when we still have problems doing normal builds after version bumps [2]

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still unsure what you are suggesting, my apologies. If this is not the way we should share the workflows, no objections to change it, but to what model?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should publish the workflows as github actions e.g. opensearch-project/opensearch-ci-image-by-tag@v1?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, thanks @peternied , @peterzhuamazon fyi ^^

with:
product: opensearch

generate-test-list:
runs-on: ubuntu-latest
outputs:
Expand All @@ -32,16 +37,49 @@ jobs:
run: |
echo "separateTestsNames=$(./gradlew listTasksAsJSON -q --console=plain | tail -n 1)" >> $GITHUB_OUTPUT

test:
name: test
test-linux:
name: test (linux)
needs:
- generate-test-list
- Get-CI-Image-Tag
if: github.repository == 'opensearch-project/security'
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
gradle_task: ${{ fromJson(needs.generate-test-list.outputs.separateTestsNames) }}
jdk: [11, 17, 21]
container:
# using the same image which is used by opensearch-build team to build the OpenSearch Distribution
# this image tag is subject to change as more dependencies and updates will arrive over time
image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }}
# need to switch to root so that github actions can install runner binary on container without permission issues.
options: --user root

steps:
- uses: actions/checkout@v3
name: Checkout security
- name: Build and Test
run: |
chown -R ci-runner:ci-runner `pwd`
su ci-runner -c "source /etc/profile.d/java_home.sh && ./gradlew ${{ matrix.gradle_task }} --no-build-cache -Dbuild.snapshot=false -Dorg.gradle.java.home=/opt/java/openjdk-${{ matrix.jdk }}"

- uses: alehechka/upload-tartifact@v2
if: always()
with:
name: ubuntu-latest-JDK${{ matrix.jdk }}-${{ matrix.gradle_task }}-reports
path: |
./build/reports/

test-windows:
name: test (windows)
needs: generate-test-list
strategy:
fail-fast: false
matrix:
gradle_task: ${{ fromJson(needs.generate-test-list.outputs.separateTestsNames) }}
platform: [windows-latest, ubuntu-latest]
jdk: [11, 17, 21]
runs-on: ${{ matrix.platform }}
runs-on: windows-latest

steps:
- name: Set up JDK for build and test
Expand All @@ -63,13 +101,14 @@ jobs:
- uses: alehechka/upload-tartifact@v2
if: always()
with:
name: ${{ matrix.platform }}-JDK${{ matrix.jdk }}-${{ matrix.gradle_task }}-reports
name: windows-latest-JDK${{ matrix.jdk }}-${{ matrix.gradle_task }}-reports
path: |
./build/reports/

report-coverage:
needs:
reta marked this conversation as resolved.
Show resolved Hide resolved
- "test"
- "test-linux"
- "test-windows"
- "integration-tests"
runs-on: ubuntu-latest
steps:
Expand Down
Loading