-
Notifications
You must be signed in to change notification settings - Fork 99
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
Create a dedicated docker image for ArcticDB development #2086
Open
G-D-Petrov
wants to merge
12
commits into
master
Choose a base branch
from
2081-create-a-dedicated-docker-image-for-arcticdb-development
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
36b9ed9
Add Dockerfile and flow to publish it
G-D-Petrov 2c3f5fa
Fix name
G-D-Petrov 3f8740f
Add the necessary permissions
G-D-Petrov 03ade9c
Also tag and push as latest
G-D-Petrov 79e1407
Test adding using the docker container in the flows
G-D-Petrov 399b70e
remove unnecessary step
G-D-Petrov 1d5a384
Remove the deps action as it is no longer needed
G-D-Petrov cb303ce
Add steps to publish Docker image as latest and versioned to GHCR
G-D-Petrov 26d4de6
Add option to override the tag used by the flows
G-D-Petrov cb43547
Remove check for tags
G-D-Petrov 1d026dd
Add a proper default value
G-D-Petrov 4bcb251
Merge branch 'master' into 2081-create-a-dedicated-docker-image-for-a…
G-D-Petrov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ on: | |
run_all_benchmarks: {required: true, type: boolean, description: Run all benchmarks or just the one for the given commit} | ||
commit: {required: true, type: string, description: commit hash that will be benchmarked} | ||
run_on_pr_head: {required: false, default: false, type: boolean, description: Specifies if the benchmark should run on PR head branch} | ||
dev_image_tag: {required: false, default: 'latest', type: string, description: Tag of the ArcticDB development image} | ||
jobs: | ||
start_ec2_runner: | ||
uses: ./.github/workflows/ec2_runner_jobs.yml | ||
|
@@ -19,7 +20,7 @@ jobs: | |
always() && | ||
!cancelled() | ||
runs-on: ${{ needs.start_ec2_runner.outputs.label }} | ||
container: quay.io/pypa/manylinux_2_28_x86_64:latest | ||
container: ghcr.io/man-group/arcticdb-dev:${{ inputs.dev_image_tag }} | ||
env: | ||
# this is potentially overflowing the cache, so should be looked into after we address issue #1057 | ||
SCCACHE_GHA_VERSION: ${{vars.SCCACHE_GHA_VERSION || 1}} # Setting this env var enables the caching | ||
|
@@ -31,11 +32,6 @@ jobs: | |
defaults: | ||
run: {shell: bash} | ||
steps: | ||
- name: Initialize LFS | ||
shell: bash -l {0} | ||
run: | | ||
dnf install -y git-lfs | ||
|
||
- uses: actions/[email protected] | ||
with: | ||
lfs: 'true' | ||
|
@@ -48,9 +44,6 @@ jobs: | |
uses: mozilla-actions/[email protected] | ||
with: | ||
version: "v0.4.0" | ||
|
||
- name: Install deps | ||
uses: ./.github/actions/setup_deps | ||
|
||
- name: Extra envs | ||
shell: bash -l {0} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
name: Docker Image for Development | ||
on: | ||
# should support manual trigger and PRs to the Dockerfile | ||
push: | ||
paths: | ||
- 'docker/**' | ||
- '.github/workflows/dev_docker_image.yml' | ||
workflow_dispatch: | ||
|
||
jobs: | ||
build_and_push: | ||
runs-on: ubuntu-latest | ||
permissions: | ||
contents: read | ||
packages: write | ||
steps: | ||
- name: Checkout | ||
uses: actions/[email protected] | ||
|
||
- name: Choose image tag | ||
run: | | ||
sha=$(git rev-parse --short HEAD) | ||
image_ver="$(date '+%Y%m%d')-${sha}" | ||
image_name="ghcr.io/man-group/arcticdb-dev" | ||
echo -e "image_ver=$image_ver\nimage_name=$image_name\noutput_tag=$image_name:$image_ver" | tee -a $GITHUB_ENV | ||
|
||
- name: Build Docker image | ||
run: | | ||
docker build -t $image_name . -f docker/Dockerfile | ||
|
||
- name: Login to GHCR | ||
run: docker login ghcr.io -u token -p "${{secrets.GITHUB_TOKEN}}" | ||
|
||
- name: Publish Docker versioned image to GHCR | ||
run: | | ||
docker tag $image_name $image_name:$image_ver | ||
docker push $image_name:$image_ver | ||
|
||
- name: Publish Docker image to GHCR as latest | ||
if: github.ref == 'refs/heads/master' | ||
run: | | ||
docker tag $image_name $image_name:latest | ||
docker push $image_name:latest |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
FROM quay.io/pypa/manylinux_2_28_x86_64 | ||
|
||
RUN dnf update -y | ||
RUN dnf remove -y 'gcc-toolset-*' | ||
RUN dnf install -y zip flex bison gcc-toolset-10 gcc-toolset-10-gdb gcc-toolset-10-libatomic-devel krb5-devel cyrus-sasl-devel openssl-devel \ | ||
unzip tar epel-release jq wget libcurl-devel git-lfs \ | ||
python3.11-devel python3.11-pip perl-IPC-Cmd | ||
|
||
RUN dnf groupinstall -y 'Development Tools' | ||
RUN dnf install -y mono-complete | ||
|
||
RUN dnf clean all | ||
|
||
RUN wget -nv https://github.com/mozilla/sccache/releases/download/v0.8.2/sccache-v0.8.2-x86_64-unknown-linux-musl.tar.gz | ||
RUN tar xvf sccache*.tar.gz | ||
RUN mv sccache-*/sccache . | ||
RUN chmod 555 sccache | ||
|
||
RUN cp sccache /usr/local/bin/ | ||
|
||
ENV CC=/opt/rh/gcc-toolset-10/root/bin/gcc | ||
ENV CMAKE_C_COMPILER=/opt/rh/gcc-toolset-10/root/bin/gcc | ||
ENV CXX=/opt/rh/gcc-toolset-10/root/bin/g++ | ||
ENV CMAKE_CXX_COMPILER=/opt/rh/gcc-toolset-10/root/bin/g++ | ||
ENV LD_LIBRARY_PATH=/opt/rh/gcc-toolset-10/root/usr/lib64:/opt/rh/gcc-toolset-10/root/usr/lib:/opt/rh/gcc-toolset-10/root/usr/lib64/dyninst | ||
ENV PATH=/opt/rh/devtoolset-10/root/usr/bin:/opt/python/cp311-cp311/bin:${PATH} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I don't see the other jobs depend on
Docker Image for Development
job. So changes in the dev docker creation won't be reflected until the image got pushed inDocker Image for Development
job.I feel like the checksum checking method in
cibw_docker_image.yml
for triggering the creation of new docker image a better alternativeThere 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 make changes to this docker very infrequently so I prefer to keep the flows a bit simpler and to not add dependencies between them unless it is needed.
Also this image will be used in other repos as well so it has to be tested manually in any case.
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.
Yes I am torn. I understand this docker image won't be changed frequently so it's hard to justify complicating already complicated wheel building process.
At the same time, if any changes made to this docker image, the CI can't tell us whether it works until the change is merged and tag
latest
is updated. Only manual test evidence can be relied on. This kinda defy the purpose of CI.I'll approve it and leave it to @poodlewars for thoughts
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.
It would be nice to be able to run the CI with a candidate new image. How about a run parameter to choose the image tag rather than hardcoding
latest
? Would then be easy to try out a CI run with a candidate image.Or having downstream jobs depend on this one sounds reasonable too. Either way works as long as it's all documented and easy for people to figure out.
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.
good point , I've added a parameter to override the default tag