ci: automate quic-network-simulator docker image build & push #1554

mxinden · 2024-01-15T11:37:15Z

Changes

Publish ghcr.io/mozilla/neqo-qns docker image on a daily schedule and test build on related pull requests.
Remove qns/update.sh. Is this still needed?
Remove /neqo-reference indirection in qns/Dockerfile.

Follow-ups

Update quic-interop/quic-interop neqo image name.

mxinden · 2024-01-15T11:39:22Z

.github/workflows/qns.yml

+
+on:
+  push:
+    branches: ["main", "docker"] # TODO: remove docker


Suggested change

branches: ["main", "docker"] # TODO: remove docker

branches: ["main"]

To be removed before merge.

mxinden · 2024-01-15T11:43:40Z

qns/Dockerfile

@@ -13,7 +13,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
 ENV RUSTUP_HOME=/usr/local/rustup \
    CARGO_HOME=/usr/local/cargo \
    PATH=/usr/local/cargo/bin:$PATH \
-    RUST_VERSION=1.45.2
+    RUST_VERSION=1.70.0


Same as version in check.yml:

neqo/.github/workflows/check.yml

Line 20 in 6fb7a6c

rust-toolchain: [1.70.0, stable, beta]

codecov-commenter · 2024-01-15T11:45:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6fb7a6c) 86.52% compared to head (0578504) 86.52%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1554   +/-   ##
=======================================
  Coverage   86.52%   86.52%           
=======================================
  Files         117      117           
  Lines       38170    38170           
=======================================
  Hits        33025    33025           
  Misses       5145     5145

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

larseggert

Do you think this could be made a "reusable workflow" contingent on the ubuntu-latest/1.70.0 run of the main CI succeeding?

mxinden · 2024-01-15T14:57:14Z

Thank you for the quick review!

Do you think this could be made a "reusable workflow" contingent on the ubuntu-latest/1.70.0 run of the main CI succeeding?

@larseggert can you expand on why a reusable workflow would be helpful here? Where would we be reusing this logic?

For the sake of not pushing a faulty image, I have now made the qns-docker-image job dependent on the check job.

Note that it does not depend on the matrix variant ubuntu-latest/1.70.0 in particular, but on the success of the entire matrix. In case we want to optimize the job's runtime, I suggest looking into further caching mechanisms first. The last execution of the new job took 3m 35s.

mxinden · 2024-01-15T15:04:17Z

.github/workflows/check.yml

Given that this workflow goes beyond checking now, I suggest renaming the file to e.g. main.yml. Thoughts?

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).

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

martinthomson · 2024-01-16T01:12:05Z

.github/workflows/check.yml

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).

martinthomson · 2024-01-16T01:13:44Z

.github/workflows/check.yml

+      - 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"


That's going to run this with our MSRV, rather than the latest. I would just run on stable.

a98aa1e updates the workflow to use stable.

martinthomson · 2024-01-16T01:23:57Z

qns/Dockerfile

-    cp target/release/neqo-client target; \
-    cp target/release/neqo-server target; \
-    rm -rf target/release
+      --bin neqo-client --bin neqo-server;


Suggested change

--bin neqo-client --bin neqo-server;

--bin neqo-client --bin neqo-server

👍 thanks for the catch! ae21e35

martinthomson · 2024-01-16T01:25:50Z

.github/workflows/check.yml

+      - name: Checkout
+        uses: actions/checkout@v3
+


Suggested change

- name: Checkout

uses: actions/checkout@v3

https://github.com/docker/build-push-action#git-context

Cool. This was news to me.

e2af258

martinthomson · 2024-01-16T01:26:34Z

qns/Dockerfile

 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; \


It might be a good idea to update the version of rustup-init we use.

👍 5d10ede

martinthomson · 2024-01-16T01:27:07Z

qns/Dockerfile

@@ -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


I think that we can keep this change, but I would prefer if our builds use 'stable'.

mxinden · 2024-01-16T09:51:53Z

Thanks @martinthomson. Mind taking another look?

martinthomson

Let's give this a spin. We'll need to update the interop runner repo with the outcome.

.github/workflows/qns.yml

OBE

mxinden · 2024-01-17T08:44:28Z

We'll need to update the interop runner repo with the outcome.

👍 see quic-interop/quic-interop-runner#344.

mxinden requested review from martinthomson and KershawChang as code owners January 15, 2024 11:37

ci: automate quic-network-simulator docker image build & push

3be5848

mxinden force-pushed the docker branch from d9ad3e3 to 3be5848 Compare January 15, 2024 11:39

mxinden commented Jan 15, 2024

View reviewed changes

mxinden mentioned this pull request Jan 15, 2024

consider building and pushing Interop Runner Docker image in CI #1552

Closed

mxinden commented Jan 15, 2024

View reviewed changes

larseggert previously requested changes Jan 15, 2024

View reviewed changes

mxinden marked this pull request as draft January 15, 2024 14:08

mxinden added 5 commits January 15, 2024 15:21

Merge workflow files and extract version from Cargo.toml

2bd522e

Disable needs for debugging

6de9005

Fix Dockerfile syntax

4cf6386

Don't push on pull request

8abd0c8

Only run after check

6677bcb

mxinden marked this pull request as ready for review January 15, 2024 14:57

mxinden commented Jan 15, 2024

View reviewed changes

martinthomson requested changes Jan 16, 2024

View reviewed changes

mxinden added 10 commits January 16, 2024 10:09

Move to separate file qns.yml

be2d481

Run daily and on related pull requests

598e12a

Always run on stable Rust

a98aa1e

Don't ignore build errors

ae21e35

Depend on build-push-action git context

e2af258

Update rustup-init

5d10ede

Typo

7b2cbae

Reduce diff

015f1c8

Newline

2b2fada

Fix typo in path

0578504

larseggert requested a review from martinthomson January 16, 2024 09:52

martinthomson approved these changes Jan 16, 2024

View reviewed changes

.github/workflows/qns.yml Outdated Show resolved Hide resolved

Fix name

de72236

martinthomson merged commit ba59d54 into mozilla:main Jan 16, 2024
10 checks passed

mxinden mentioned this pull request Feb 5, 2024

ci: Build docker image for multiple architectures #1619

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: automate quic-network-simulator docker image build & push #1554

ci: automate quic-network-simulator docker image build & push #1554

mxinden commented Jan 15, 2024 •

edited

Loading

mxinden Jan 15, 2024

mxinden Jan 15, 2024 •

edited

Loading

codecov-commenter commented Jan 15, 2024 •

edited

Loading

larseggert left a comment

mxinden commented Jan 15, 2024

mxinden Jan 15, 2024 •

edited

Loading

martinthomson Jan 16, 2024

mxinden Jan 16, 2024 •

edited

Loading

martinthomson Jan 16, 2024

martinthomson Jan 16, 2024

mxinden Jan 16, 2024

martinthomson Jan 16, 2024

mxinden Jan 16, 2024

martinthomson Jan 16, 2024

mxinden Jan 16, 2024

martinthomson Jan 16, 2024

mxinden Jan 16, 2024

martinthomson Jan 16, 2024

mxinden Jan 16, 2024

mxinden commented Jan 16, 2024

martinthomson left a comment

mxinden commented Jan 17, 2024

	branches: ["main", "docker"] # TODO: remove docker
	branches: ["main"]

	--bin neqo-client --bin neqo-server;
	--bin neqo-client --bin neqo-server

ci: automate quic-network-simulator docker image build & push #1554

ci: automate quic-network-simulator docker image build & push #1554

Conversation

mxinden commented Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

mxinden Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Jan 15, 2024 • edited Loading

Codecov Report

larseggert left a comment

Choose a reason for hiding this comment

mxinden commented Jan 15, 2024

mxinden Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxinden Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxinden commented Jan 16, 2024

martinthomson left a comment

Choose a reason for hiding this comment

mxinden commented Jan 17, 2024

mxinden commented Jan 15, 2024 •

edited

Loading

mxinden Jan 15, 2024 •

edited

Loading

codecov-commenter commented Jan 15, 2024 •

edited

Loading

mxinden Jan 15, 2024 •

edited

Loading

mxinden Jan 16, 2024 •

edited

Loading