-
Notifications
You must be signed in to change notification settings - Fork 68
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.
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
[nightly] Create Nightly Pipeline, make docker-nightly-publish.yml & integration.yml more modular #2628
[nightly] Create Nightly Pipeline, make docker-nightly-publish.yml & integration.yml more modular #2628
Changes from 26 commits
33f14f6
39a9050
c8233a3
b96628b
85b15e5
ec5bf65
7505dff
1f17c17
3a74892
f63a7fe
8d43fb3
2674540
4133b2e
850a69a
f182852
6f01ae3
5ace20e
e974d65
8de85fa
9808a93
14185c5
9f4a91a
3dde6de
0adabd0
7a75c66
c40f9f5
9fb5574
0bc8a25
b02ca24
a0a1b16
7dfb9c3
86bd0d9
c28e392
b1b0871
277483a
60cd26d
45092e4
66466fd
1dd747b
49322cb
0374887
086065b
03f6e17
186e602
5d4ac70
3949dcc
bcd3555
5a9f70c
f396e74
33ab630
74b72fa
53b0d1c
26636d7
468a260
b5eaf03
ea3b518
0faa058
ab7e10b
5761450
ebc7f89
cf56d95
1d6b0e4
583e8a2
d772470
ce98891
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,11 @@ on: | |
description: 'release/nightly/temp, default is nightly' | ||
required: true | ||
default: 'nightly' | ||
skip_nightly_integ_test: | ||
description: 'buld and push the nightly without running integ test' | ||
required: false | ||
default: false | ||
type: boolean | ||
workflow_call: | ||
inputs: | ||
mode: | ||
|
@@ -21,6 +26,10 @@ permissions: | |
id-token: write | ||
contents: read | ||
|
||
env: | ||
AWS_ECR_REPO: "185921645874.dkr.ecr.us-east-1.amazonaws.com/djl-ci-temp" | ||
DOCKER_HUB_REPO: "deepjavalibrary/djl-serving" | ||
|
||
jobs: | ||
nightly-build: | ||
runs-on: ubuntu-latest | ||
|
@@ -70,29 +79,25 @@ jobs: | |
run: | | ||
./gradlew --refresh-dependencies :serving:dockerDeb -Psnapshot | ||
- name: Build and push nightly docker image | ||
if: ${{ inputs.mode == '' || inputs.mode == 'nightly' }} | ||
siddvenk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
working-directory: serving/docker | ||
run: | | ||
export NIGHTLY="-nightly" | ||
docker compose build --no-cache \ | ||
--build-arg djl_version=${{ env.DJL_VERSION }}-SNAPSHOT \ | ||
--build-arg djl_serving_version=${{ env.SERVING_VERSION }}-SNAPSHOT \ | ||
${{ matrix.arch }} | ||
docker compose push ${{ matrix.arch }} | ||
- name: Build and push temp image | ||
if: ${{ inputs.mode == 'temp' }} | ||
- name: Tag and push temp image to ECR repo | ||
if: ${{ !inputs.skip_nightly_integ_test && inputs.mode == 'nightly' || inputs.mode == 'temp'}} | ||
working-directory: serving/docker | ||
run: | | ||
export NIGHTLY="-nightly" | ||
docker compose build --no-cache \ | ||
--build-arg djl_version=${{ env.DJL_VERSION }}-SNAPSHOT \ | ||
--build-arg djl_serving_version=${{ env.SERVING_VERSION }}-SNAPSHOT \ | ||
${{ matrix.arch }} | ||
repo="185921645874.dkr.ecr.us-east-1.amazonaws.com/djl-ci-temp" | ||
aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin $repo | ||
tempTag="$repo:${{ matrix.arch }}-${GITHUB_SHA}" | ||
docker tag deepjavalibrary/djl-serving:${{ matrix.arch }}-nightly $tempTag | ||
aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin ${{env.AWS_ECR_REPO}} | ||
HappyAmazonian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
tempTag="${{ env.AWS_ECR_REPO }}:${{ matrix.arch }}-${GITHUB_SHA}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will result in new temp images in ECR nightly. Have we configured the retention policy in this ECR repo? We should discuss what that retention period looks like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Someone created the ECR repo already with 7-day retention policy for the temp mode. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The price for ECR storage is $0.09 per GB per month. ~$90 per TB per month. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not really worried about the cost here. I am worried about the time wasted in saving and retrieving images from ECR. ECR is a fairly slow service. |
||
docker tag ${{ env.DOCKER_HUB_REPO }}:${{ matrix.arch }}-nightly $tempTag | ||
docker push $tempTag | ||
- name: Push nightly to dockerhub | ||
if: ${{ inputs.skip_nightly_integ_test && inputs.mode == 'nightly' }} | ||
run: | | ||
docker push ${{ env.DOCKER_HUB_REPO }}:${{ matrix.arch }}-nightly | ||
- name: Build and push release docker image | ||
if: ${{ inputs.mode == 'release' }} | ||
working-directory: serving/docker | ||
|
@@ -111,6 +116,34 @@ jobs: | |
docker tag deepjavalibrary/djl-serving:${{ env.SERVING_VERSION }} deepjavalibrary/djl-serving:latest | ||
docker push deepjavalibrary/djl-serving:latest | ||
|
||
run-integration-tests: | ||
if: ${{ inputs.mode == 'nightly' && !inputs.skip_integ_test }} | ||
needs: [nightly-build, nightly-aarch64] | ||
uses: ./.github/workflows/integration.yml | ||
secrets: inherit | ||
with: | ||
djl-version: temp | ||
|
||
push-to-dockerhub: | ||
runs-on: ubuntu-latest | ||
needs: [run-integration-tests] | ||
strategy: | ||
matrix: | ||
arch: [ cpu, cpu-full, pytorch-inf2, pytorch-gpu, tensorrt-llm, lmi ] | ||
steps: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we're a binary success/fail situation here for all containers. Integ tests for all containers must pass for any new nightly publish. We should think through what it takes to make this more granular, which may involve splitting up the current integration.yml file into a few separate worfklows, or adding specific steps here that run tests on a container by container basis |
||
- name: Login to Docker | ||
uses: docker/login-action@v3 | ||
with: | ||
username: ${{ secrets.DOCKER_USERNAME }} | ||
password: ${{ secrets.DOCKER_PASSWORD }} | ||
- name: Pull Image from ECR and Push it to Dockerhub | ||
run: | | ||
aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin ${{env.AWS_ECR_REPO}} | ||
tempTag=" ${{env.AWS_ECR_REPO}}:${{ matrix.arch }}-${GITHUB_SHA}" | ||
docker pull $tempTag | ||
docker tag $tempTag ${{ env.DOCKER_HUB_REPO }}:${{ matrix.arch }}-nightly | ||
docker push ${{ env.DOCKER_HUB_REPO }}:${{ matrix.arch }}-nightly | ||
|
||
create-runner: | ||
runs-on: [ self-hosted, scheduler ] | ||
steps: | ||
|
@@ -186,7 +219,7 @@ jobs: | |
aarch64 | ||
docker compose push aarch64 | ||
- name: Build and push temp image | ||
if: ${{ inputs.mode == 'temp' }} | ||
if: ${{ inputs.mode == 'temp' || inputs.mode == 'nightly' }} | ||
working-directory: serving/docker | ||
run: | | ||
export NIGHTLY="-nightly" | ||
|
@@ -195,8 +228,8 @@ jobs: | |
--build-arg djl_serving_version=${{ env.SERVING_VERSION }}-SNAPSHOT \ | ||
aarch64 | ||
repo="185921645874.dkr.ecr.us-east-1.amazonaws.com/djl-ci-temp" | ||
aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin $repo | ||
tempTag="$repo:aarch64-${GITHUB_SHA}" | ||
aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin ${{env.AWS_ECR_REPO}} | ||
tempTag="${{env.AWS_ECR_REPO}}:aarch64-${GITHUB_SHA}" | ||
docker tag deepjavalibrary/djl-serving:aarch64-nightly $tempTag | ||
docker push $tempTag | ||
- name: Build and push release docker image | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can move this as a command under Build temp docker image? I don't think we need a separate step for this only