Skip to content

feature: Populate NetworkSettings.Gateway - #4771

Open
must108 wants to merge 1 commit into
containerd:mainfrom
must108:inspect-gateway
Open

feature: Populate NetworkSettings.Gateway#4771
must108 wants to merge 1 commit into
containerd:mainfrom
must108:inspect-gateway

Conversation

@must108

@must108 must108 commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Resolves #415

Issue

NetworkSettings.Gateway is expected to be present in inspect output. nerdctl did not expose this prior to this change, even when a default gateway existed.

Changes

This pull request adds a feature that allows for NetworkSettings.Gateway to be populated in nerdctl inspect output.

How it works:

  • Collects default IPv4 gateway from the container network namespace during netns inspection

  • Stores the gateway in inspect network data

  • Propagates the value to DefaultNetworkSettings.Gateway

  • Unit tests have also been adjusted to reflect this change.

Testing

go test ./pkg/containerinspector ./pkg/inspecttypes/dockercompat

Notes

  • I have limited the scope to only IPv4, but considered also implementing a solution for IPv6. I can create a new issue for this if it is deemed necessary.

haytok
haytok reviewed Mar 5, 2026
View reviewed changes

@haytok haytok left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for creating this PR! Would it be possible to have e2e tests using Tigron for this feature ?

@must108

must108 commented Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for creating this PR! Would it be possible to have e2e tests using Tigron for this feature ?

Good suggestion, thanks! I'll add a Tigron e2e test that verifies that NetworkSettings.Gateway is populated on inspect.

@must108

must108 commented Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

@haytok added the test

AkihiroSuda
AkihiroSuda reviewed Mar 6, 2026
View reviewed changes
Comment thread cmd/nerdctl/container/container_inspect_gateway_linux_test.go Outdated
AkihiroSuda
AkihiroSuda requested changes Mar 6, 2026
View reviewed changes

@AkihiroSuda AkihiroSuda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please squash the commits

@must108
must108 force-pushed the inspect-gateway branch 3 times, most recently from fc3cd88 to c8e2272 Compare March 6, 2026 08:52
haytok
haytok suggested changes Mar 6, 2026
View reviewed changes

@haytok haytok left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When checking this CI Log, the test TestContainerInspectGateway you have added has failed, so please check your fix.

=== Failing tests ===
TestContainerInspectGateway

Comment thread cmd/nerdctl/container/container_inspect_gateway_linux_test.go Outdated
@must108
must108 force-pushed the inspect-gateway branch 3 times, most recently from 2b0bd48 to ec66442 Compare March 7, 2026 10:32
@must108

must108 commented Mar 15, 2026

Copy link
Copy Markdown
Contributor Author

@AkihiroSuda @haytok

haytok
haytok suggested changes Mar 15, 2026
View reviewed changes

@haytok haytok left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Before requesting a review, please make sure that the tests for the changes you’ve made have passed 🙏

Comment thread cmd/nerdctl/container/container_inspect_linux_test.go
testCase.Run(t)
}

func TestContainerInspectGateway(t *testing.T) {

@haytok haytok Mar 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Have you already checked CI failures related to this fix?

@AkihiroSuda

AkihiroSuda commented Mar 31, 2026

Copy link
Copy Markdown
Member 
 container_inspect_linux_test.go:643: 	🔗 
        
        <<<<<<<<<<<<<<<<<<<<
        	🖊️ Unmarshalling JSON from stdout must succeed
        	👀 testing:		`json: cannot unmarshal object into Go struct field RuntimeInfo.Container.Runtime.Options of type typeurl.Any`
        	❌ FAILED!		is `<nil>`
        >>>>>>>>>>>>>>>>>>>>

https://github.com/containerd/nerdctl/actions/runs/23119762664/job/67191519034?pr=4771

@AkihiroSuda

AkihiroSuda commented May 14, 2026

Copy link
Copy Markdown
Member

Marking as a draft due to the CI failure.
Please click "Ready for review" when ready

@AkihiroSuda
AkihiroSuda marked this pull request as draft May 14, 2026 17:43
@must108
feature: populate NetworkSettings.Gateway
a07fe83 
NetworkSettings.Gateway is expected to be present in inspect output.
nerdctl did not expose this prior to this change, even when a default
gateway existed.

Collect the IPv4 default gateway from the container network namespace
during netns inspection, store it in the native inspect data, and
propagate it to DefaultNetworkSettings.Gateway.

Resolves containerd#415

Signed-off-by: Mustaeen Ahmed <contact@mustaeen.dev>
@must108
must108 marked this pull request as ready for review July 25, 2026 08:04
@must108

must108 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

@AkihiroSuda

@AkihiroSuda AkihiroSuda added this to the v2.4.0 milestone Jul 25, 2026
AkihiroSuda
AkihiroSuda reviewed Aug 4, 2026
View reviewed changes
Comment thread pkg/inspecttypes/native/container.go
@@ -44,6 +44,7 @@ type NetNS struct {
// Zero means unset.
PrimaryInterface int `json:"PrimaryInterface,omitempty"`
Interfaces []NetInterface `json:"Interfaces,omitempty"`
Gateway string `json:"Gateway,omitempty"`

@AkihiroSuda AkihiroSuda Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Gateway string `json:"Gateway,omitempty"`
Gateway net.IP `json:"Gateway,omitempty"`

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

Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda requested changes
+1 more reviewer
@haytok haytok haytok requested changes
Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.4.0

Development

Successfully merging this pull request may close these issues.

Supporting Gateway in DefaultNetworkSettings in docker compatible inspect

3 participants

@must108 @AkihiroSuda @haytok