-
Notifications
You must be signed in to change notification settings - Fork 124
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
ci: automate quic-network-simulator docker image build & push #1554
Changes from 6 commits
3be5848
2bd522e
6de9005
4cf6386
8abd0c8
6677bcb
be2d481
598e12a
a98aa1e
ae21e35
e2af258
5d10ede
7b2cbae
015f1c8
2b2fada
0578504
de72236
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 |
---|---|---|
@@ -0,0 +1,3 @@ | ||
nss | ||
nspr | ||
target |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -127,3 +127,51 @@ jobs: | |||||
file: lcov.info | ||||||
fail_ci_if_error: false | ||||||
token: ${{ secrets.CODECOV_TOKEN }} | ||||||
|
||||||
qns-docker-image: # Docker image for QUIC network simulator i.e. interop runner (https://github.com/quic-interop/quic-interop-runner) | ||||||
needs: check | ||||||
runs-on: ubuntu-latest | ||||||
steps: | ||||||
- name: Checkout | ||||||
uses: actions/checkout@v3 | ||||||
|
||||||
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.
Suggested change
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. Cool. This was news to me. |
||||||
- name: Extract Rust version | ||||||
id: rust-version | ||||||
run: | | ||||||
RUST_VERSION=$(grep -m1 'rust-version' neqo-common/Cargo.toml | cut -d '"' -f 2) | ||||||
echo "::set-output name=version::$RUST_VERSION" | ||||||
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. That's going to run this with our MSRV, rather than the latest. I would just run on stable. 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. a98aa1e updates the workflow to use |
||||||
|
||||||
- name: Set up Docker Buildx | ||||||
uses: docker/setup-buildx-action@v3 | ||||||
|
||||||
- name: Login to GitHub Container Registry | ||||||
uses: docker/login-action@v3 | ||||||
with: | ||||||
registry: ghcr.io | ||||||
username: ${{ github.actor }} | ||||||
password: ${{ github.token }} | ||||||
|
||||||
- name: Docker meta | ||||||
id: meta | ||||||
uses: docker/metadata-action@v5 | ||||||
with: | ||||||
images: ghcr.io/${{ github.repository }}-qns | ||||||
tags: | | ||||||
# default | ||||||
type=schedule | ||||||
type=ref,event=branch | ||||||
type=ref,event=tag | ||||||
type=ref,event=pr | ||||||
# set latest tag for default branch | ||||||
type=raw,value=latest,enable={{is_default_branch}} | ||||||
|
||||||
- name: Build and push | ||||||
uses: docker/build-push-action@v5 | ||||||
with: | ||||||
push: ${{ github.event_name != 'pull_request' }} | ||||||
tags: ${{ steps.meta.outputs.tags }} | ||||||
file: qns/Dockerfile | ||||||
build-args: | | ||||||
RUST_VERSION=${{ steps.rust-version.outputs.version }} | ||||||
cache-from: type=gha | ||||||
cache-to: type=gha,mode=max |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -10,10 +10,11 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ | |||||
&& apt-get autoremove -y && apt-get clean -y \ | ||||||
&& rm -rf /var/lib/apt/lists/* | ||||||
|
||||||
ARG RUST_VERSION | ||||||
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 think that we can keep this change, but I would prefer if our builds use 'stable'. 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. |
||||||
|
||||||
ENV RUSTUP_HOME=/usr/local/rustup \ | ||||||
CARGO_HOME=/usr/local/cargo \ | ||||||
PATH=/usr/local/cargo/bin:$PATH \ | ||||||
RUST_VERSION=1.45.2 | ||||||
PATH=/usr/local/cargo/bin:$PATH | ||||||
|
||||||
RUN set -eux; \ | ||||||
curl -sSLf "https://static.rust-lang.org/rustup/archive/1.22.1/x86_64-unknown-linux-gnu/rustup-init" -o rustup-init; \ | ||||||
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 might be a good idea to update the version of rustup-init we use. 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. 👍 5d10ede |
||||||
|
@@ -33,35 +34,22 @@ RUN set -eux; \ | |||||
|
||||||
RUN "$NSS_DIR"/build.sh --static -Ddisable_tests=1 -o | ||||||
|
||||||
# Copy the .git directory from the local clone so that it is possible to create | ||||||
# an image that includes local updates. | ||||||
RUN mkdir -p /neqo-reference | ||||||
ADD . /neqo-reference | ||||||
RUN if [ -d /neqo-reference/.git ]; then \ | ||||||
source=/neqo-reference; \ | ||||||
else \ | ||||||
source=https://github.com/mozilla/neqo; \ | ||||||
fi; \ | ||||||
git clone --depth 1 --branch "$NEQO_BRANCH" "$source" /neqo; \ | ||||||
rm -rf /neqo-reference | ||||||
ADD . /neqo | ||||||
|
||||||
RUN set -eux; \ | ||||||
cd /neqo; \ | ||||||
RUSTFLAGS="-g -C link-arg=-fuse-ld=lld" cargo build --release \ | ||||||
--bin neqo-client --bin neqo-server; \ | ||||||
cp target/release/neqo-client target; \ | ||||||
cp target/release/neqo-server target; \ | ||||||
rm -rf target/release | ||||||
--bin neqo-client --bin neqo-server; | ||||||
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.
Suggested change
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. 👍 thanks for the catch! ae21e35 |
||||||
|
||||||
# Copy only binaries to the final image to keep it small. | ||||||
|
||||||
FROM martenseemann/quic-network-simulator-endpoint:latest | ||||||
|
||||||
ENV LD_LIBRARY_PATH=/neqo/lib | ||||||
COPY --from=buildimage /neqo/target/neqo-client /neqo/target/neqo-server /neqo/bin/ | ||||||
COPY --from=buildimage /neqo/target/release/neqo-client /neqo/target/release/neqo-server /neqo/bin/ | ||||||
COPY --from=buildimage /dist/Release/lib/*.so /neqo/lib/ | ||||||
COPY --from=buildimage /dist/Release/bin/certutil /dist/Release/bin/pk12util /neqo/bin/ | ||||||
|
||||||
COPY interop.sh /neqo/ | ||||||
COPY qns/interop.sh /neqo/ | ||||||
RUN chmod +x /neqo/interop.sh | ||||||
ENTRYPOINT [ "/neqo/interop.sh" ] |
This file was deleted.
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.
Given that this workflow goes beyond
check
ing now, I suggest renaming the file to e.g.main.yml
. 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.
I would put your additions in a new file instead. Call it qns.yml
The conditions for this should be different. We want pushes to occur at most daily on main.
We should not build this for pull requests on the expectation that if tests pass, this build will work, so we don't need to run this process unless someone changes the qns.yml file or the Dockerfile (conditions you might add for a pull_request in that file).
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 moved the job into a separate file and adjusted the triggers. It will now run on a daily schedule. In addition, pull requests touching related files will build the image but not push it. Let me know if that is good enough.
be2d481 598e12a