Releases/dind network host patch v2.325.0 #3894
Conversation
…nd-network-host-patch-v2.320.0
…v2.320.0 Upgrade dind-network-host-patch to v2.320.0
chore(IDX): sync main
Sync with main
Update releases/dind-network-host-patch-v2.325.0
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the Docker-in-Docker behavior to use the host network, bumps the release version, and updates the runner download source.
- Switch container networking to
hostand remove local network setup/teardown - Bump
releaseVersion - Point Dockerfile download to the
dfinity/runnerrepository
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Runner.Worker/ContainerOperationProvider.cs | Comment out local network creation/removal and set containerNetwork to "host" |
| releaseVersion | Updated release version from placeholder to 2.324.0 |
| images/Dockerfile | Changed runner download URL to https://github.com/dfinity/runner/... |
Comments suppressed due to low confidence (1)
images/Dockerfile:16
- Confirm that the new
dfinity/runnerrepository and path are correct; using an invalid URL will break the Docker build.
&& curl -f -L -o runner.tar.gz https://github.com/dfinity/runner/releases/download/v${RUNNER_VERSION}/actions-runner-${TARGETOS}-${RUNNER_ARCH}-${RUNNER_VERSION}.tar.gz \
| /* | ||
| XXX: don't add network as we'll use --network host | ||
| executionContext.Output("##[group]Create local container network"); | ||
| var containerNetwork = $"github_network_{Guid.NewGuid().ToString("N")}"; | ||
| await CreateContainerNetworkAsync(executionContext, containerNetwork); | ||
| executionContext.JobContext.Container["network"] = new StringContextData(containerNetwork); | ||
| executionContext.Output("##[endgroup]"); | ||
|
|
||
| */ |
There was a problem hiding this comment.
[nitpick] Consider removing or extracting this large commented-out block if the local network code is no longer needed, to keep the codebase clean.
Copilot uses AI. Check for mistakes.
| executionContext.Output("##[group]Create local container network"); | ||
| var containerNetwork = $"github_network_{Guid.NewGuid().ToString("N")}"; | ||
| await CreateContainerNetworkAsync(executionContext, containerNetwork); | ||
| executionContext.JobContext.Container["network"] = new StringContextData(containerNetwork); | ||
| executionContext.Output("##[endgroup]"); | ||
|
|
||
| */ | ||
| var containerNetwork = "host"; |
There was a problem hiding this comment.
[nitpick] Introduce a named constant or config setting for the "host" network string instead of using a magic literal, to improve readability and ease future changes.
| var containerNetwork = "host"; | |
| const string DefaultContainerNetwork = "host"; | |
| var containerNetwork = DefaultContainerNetwork; |
Copilot uses AI. Check for mistakes.
| // XXX: we're using --network host | ||
| //await RemoveContainerNetworkAsync(executionContext, containers.First().ContainerNetwork); |
There was a problem hiding this comment.
[nitpick] Rather than commenting out the teardown call, consider adding a conditional check (e.g., skip only for host) so that non-host networks are still cleaned up and intent is explicit.
| // XXX: we're using --network host | |
| //await RemoveContainerNetworkAsync(executionContext, containers.First().ContainerNetwork); | |
| if (containers.First().ContainerNetwork != "host") | |
| { | |
| await RemoveContainerNetworkAsync(executionContext, containers.First().ContainerNetwork); | |
| } |
Copilot uses AI. Check for mistakes.
| @@ -1 +1 @@ | |||
| <Update to ./src/runnerversion when creating release> | |||
| 2.324.0 | |||
There was a problem hiding this comment.
The PR title references version v2.325.0, but releaseVersion is set to 2.324.0; please update to the correct release number.
| 2.324.0 | |
| 2.325.0 |
Copilot uses AI. Check for mistakes.
No description provided.