Skip to content

Create containers with initial network attachments - #198

Merged
David Negstad (danegsta) merged 9 commits into
mainfrom
danegsta-initial-network-create
Jul 1, 2026
Merged

Create containers with initial network attachments#198
David Negstad (danegsta) merged 9 commits into
mainfrom
danegsta-initial-network-create

Conversation

@danegsta

Copy link
Copy Markdown
Member

Initial container startup should not create on the default network and then wait for JIT attach just to reach the desired topology. This changes create-time networking so Docker and Podman receive the full initial network and alias set in the container create command once all requested networks are ready.

Summary

  • Add CreateContainerOptions.Networks as the only create-time networking API and emit runtime-specific multi-network arguments for Docker and Podman.
  • Resolve initial ContainerNetworks before creation, remove the startup-time JIT connection delay, and keep dynamic attach/detach reconciliation for later network changes.
  • Update the test orchestrator and integration coverage, including preserving unmanaged runtime connections so the network controller does not disconnect create-time attachments before connection objects exist.

Validation

  • go test -count 1 -parallel 32 ./internal/containers ./internal/docker ./internal/podman ./internal/testutil/ctrlutil ./controllers ./test/integration -run 'TestApplyCreateContainerOptionsNetworks|TestContainerNetworkKeepsUnmanagedConnections|TestContainerNetworkDoesNotStartUntilNetworkExists|TestContainerNetworkWithAliases|TestContainerFastExitingWithNetwork|TestResourceHarvesting' -timeout 180s
  • Manual Docker smoke test with kubectl-created ContainerNetwork and Container resources.
  • Manual Podman smoke test with DCP forced to --container-runtime podman.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reworks container create-time networking so that Docker/Podman receive the full set of initial networks (and per-network aliases) directly in the create command, rather than the previous approach of creating on the default network and then relying on just-in-time (JIT) attach/detach to reach the desired topology. It defers container creation until all requested ContainerNetworks are ready, and adjusts the network controller so create-time ("unmanaged") connections are not disconnected before their ContainerNetworkConnection objects exist.

Changes:

  • Replace CreateContainerOptions.Network/NetworkAliases with a single Networks []CreateContainerNetworkOptions API, and emit runtime-specific multi-network arguments (Docker: name=<net>,alias=<a>; Podman: <net>:alias=<a>).
  • Resolve initial networks before creation (getInitialCreateContainerNetworks, retryable errInitialContainerNetworksNotReady), remove the startup-time JIT detach logic in finishCreatedContainerStartup, and count create-time attachments as valid connected networks.
  • Simplify the network reconciler to only disconnect connections it previously managed (removing shouldKeepUnmanagedConnections), and update the test orchestrator plus integration/unit tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
internal/containers/container_orchestrator.goIntroduces Networks/CreateContainerNetworkOptions, replacing the single-network create-time API.
internal/docker/cli_orchestrator.goEmits per-network --network name=,alias= args for Docker.
internal/podman/cli_orchestrator.goEmits per-network --network <net>:alias= args for Podman.
internal/docker/cli_orchestrator_test.goAdds unit tests for Docker multi-network arg formatting.
internal/podman/cli_orchestrator_test.goAdds unit tests for Podman multi-network arg formatting.
controllers/container_controller.goResolves initial networks pre-creation, removes JIT detach, adds retryable not-ready error, marks create-time networks valid.
controllers/network_controller.goOnly disconnects reconciler-managed connections; removes shouldKeepUnmanagedConnections.
controllers/container_network_tunnel_proxy_controller.goUpdates client proxy creation to the new Networks API.
internal/testutil/ctrlutil/test_container_orchestrator.goAdds resolveCreateContainerNetworks to mirror multi-network create behavior.
test/integration/container_network_connection_test.goAdds/updates tests for not-starting-until-ready and keeping unmanaged connections.
test/integration/resource_harvesting_test.goMigrates test call sites to the new Networks field.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadtest/integration/container_network_connection_test.go
Comment threadcontrollers/container_controller.go Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comment threadcontrollers/network_controller.go
Comment threadinternal/docker/cli_orchestrator.go
Comment threadcontrollers/container_controller.go
Comment threadcontrollers/container_controller.go Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Require a supported Docker CLI version before probing daemon health so create-time multi-network support is gated on a fast client-only check.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danegsta
David Negstad (danegsta) enabled auto-merge (squash) July 1, 2026 21:20
@danegsta
David Negstad (danegsta) merged commit 08af559 into mainJul 1, 2026
13 of 14 checks passed
@danegsta
David Negstad (danegsta) deleted the danegsta-initial-network-create branch July 1, 2026 22:18
@danegsta

Copy link
Copy Markdown
MemberAuthor

/backport to release/0.25

@github-actions

Copy link
Copy Markdown

Started backporting to release/0.25: https://github.com/microsoft/dcp/actions/runs/28552264712

@github-actions

Copy link
Copy Markdown

David Negstad (@danegsta) backport PR couldn't be created automatically, please create the backport PR manually!

Open backport PR into release/0.25.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danegsta@karolz-ms