Always include Ports within inspect response

[chews93319]

Resolves #3310

New behavior will always initialize a NetworkSettings containing initialized PortMap for the inspect response.

• If published ports are available, then values will be included in the response.
• If there are no published ports, then NetworkSettings will include a Ports with empty struct.
• If the container had been stopped/Exited, then NetworkSettings will be an initialized struct will all entities mapping to "Zero-value" values.

Background

Research related to Scenario 1:
According to dockercompat line189 and the logical evaluation of Port Annotations, the Ports are not assigned within the response when values are present within the Annotations.
Therefore, containers without published ports yield an NetworkSettings structure without a Ports key-value pair.

Research related to Scenario 2:
According to dockercompat line401, the native.NetNS of a stopped/exited container is a null pointer, which would return nil.

Solution

To resolve this issue, the NetworkSettings and PortMap are initialized at the beginning of the method to ensure that at least "Empty-value" structs are always included within the inspect response.
When "Port Annotations" are available, the new logic will add the key-value pairs into the initialized PortMap of the response.

Since Ports should not be omitted, this change also removes the json:",omitempty" decorator for the NetworkSettings struct.

Additional Tasks

This bug is also present within v1.7.6 and the change can be easily reconciled with the released version.

• Please consider this PR for Back Porting into Release/1.7 branch.

[apostasie]

@chews93319 Nice!

Two things:

• CI failures are unrelated - canary is bust for now - and the other is network related.
• maybe we could have a test or two?

[chews93319]

@apostasie Thanks for taking a look.

I actually need to iterate on my PR implementation for another use-case (Refer to my updated Issue 3310 Description ), so I will also try to introduce a test against the method networkSettingsFromNative.
Interestingly, there were no tests against the method before, so it will be a good step to introduce something new.

[apostasie]

@apostasie Thanks for taking a look.

I actually need to iterate on my PR implementation for another use-case (Refer to my updated Issue 3310 Description ), so I will also try to introduce a test against the method networkSettingsFromNative. Interestingly, there were no tests against the method before, so it will be a good step to introduce something new.

That's very welcome. Inspect needs a lot of love...

There is a bunch of additional issues with it.
If you see anything low-hanging fruit in these while patching that code region:

• nerdctl inspect should not return a stream of json array, but a single array #3006
• nerdctl inspect does not allow inspecting networks, volumes #3005
• nerdctl inspect does not list networks name properly #2999
• nerdctl network inspect does not list containers using the network #2998
• nerdctl container inspect should show HostConfig #2905
• Supporting Gateway in DefaultNetworkSettings in docker compatible inspect #415

CI Canary failure got fixed (rebase should bring in the fix).

I am not a maintainer here - but lmk if I can help you with anything.