fix: allow containers to start using a large numbers of ports #4290

haytok · 2025-05-25T15:08:27Z

Suppose we have a compose.yaml that allocates a large numbers of ports as follows.

> cat compose.yaml
services:
  svc0:
    image: alpine
    command: "sleep infinity"
    ports:
      - '32000-32060:32000-32060'

When we run nerdctl compose up -d using this compose.yaml, we will get the following error.

FATA[0000] create container failed validation: containers.Labels: label key and value length (4711 bytes) greater than maximum size (4096 bytes), key: nerdctl/ports: invalid argument
FATA[0000] error while creating container haytok-svc0-1: error while creating container haytok-svc0-1: exit status 1

This issue is reported in the following issue.

nerdctl compose file map multiple ports (label key and value length (30968 bytes) greater than maximum size (4096 bytes)) #4027

This issue is considered to be the same as the one with errors when trying to perform many port mappings, such as nerdctl run -p 80:80 -p 81:81 ~ -p 1000:1000 ...

The current implementation is processing to create a container with the information specified in -p to the label.
And as can be seen from the error message, as the number of ports to be port mapped increases, the creation of the container fails because it violates the limit of the maximum number of bytes on the containerd side that can be allocated for a label.

Therefore, this PR modifies the container creation process so that containers can be launched without having to assign the information specified in the -p option to the labels.

Specifically, port mapping information is stored in the following path, and when port mapping information is required, it is retrieved from this file.

<DATAROOT>/<ADDRHASH>/containers/<NAMESPACE>/<CID>/port-mappings.json

haytok · 2025-05-25T15:56:58Z

Because of my modifications, CI fails.
So, I'll check the root cause of CI failures.

apostasie · 2025-05-25T21:28:10Z

Why are we not storing the range itself in the label?

haytok · 2025-05-27T16:28:50Z

Hi, @apostasie Thanks for commnets!

Why are we not storing the range itself in the label?

I also think of a way to keep the range of ports in the label, but the following edge case would violate the upper limit of the number of labels.
Specifically, when host, port and protocol are random, it is difficult to combine ranges.

nerdctl run -p 127.0.0.1:32000:32000 -p 127.0.0.2:32001:32001 ~ -p 127.0.0.254:32253:32253 nginx

Considering these cases, I think that when the port mapping information is given to the Label, it would violate the upper limit, so I implemented the way of this PR so that the port mapping information is not retained in the Label.

When nerdctl compose up is executed, the ports specified in compose.yaml are expanded and executed by the following function:

https://github.com/compose-spec/compose-go/blob/main/cli/options.go#L448-L449

The ports are expanded and executed as nerdctl run -p 32000:32000 ~ -p 32060:32060 ....

pkg/portutil/portutil.go

apostasie · 2025-05-30T20:32:28Z

@AkihiroSuda so, this PR does move some of the network configuration information from labels (or annotations) to the filesystem.

Future work to support per-network IP assignment (and options) will require significant re-jiggle of other annotations (nerdctl/network, nerdctl/ip, etc).

That overall does beg the question: should we stop using labels/annotations to store network configuration altogether, and just use a filesystem store for that instead?
Was there a specific reason we wanted network config to be stored on the container in the first place?

Answer to this question should inform whether we should consider expanding the scope of this PR a little bit to come up with a generic net-config-store that would in the future accommodate for other changes and additions ^.

AkihiroSuda · 2025-05-31T05:27:33Z

That overall does beg the question: should we stop using labels/annotations to store network configuration altogether, and just use a filesystem store for that instead?

👍

We have to keep existing containers functional, though, if we are going to merge this in v2.1.x.

haytok · 2025-06-01T16:28:07Z

Thanks for comments !!!

I understand that it is acceptable to implement a policy of storing port mapping information in a local file.
Therefore, I'll proceed with the implementation in this PR and make corrections so that file writing can be performed atomically.

Suppose we have a compose.yaml that allocates a large numbers of ports as follows. ``` > cat compose.yaml services: svc0: image: alpine command: "sleep infinity" ports: - '32000-32060:32000-32060' ``` When we run `nerdctl compose up -d` using this compose.yaml, we will get the following error. ``` FATA[0000] create container failed validation: containers.Labels: label key and value length (4711 bytes) greater than maximum size (4096 bytes), key: nerdctl/ports: invalid argument FATA[0000] error while creating container haytok-svc0-1: error while creating container haytok-svc0-1: exit status 1 ``` This issue is reported in the following issue. - containerd#4027 This issue is considered to be the same as the one with errors when trying to perform many port mappings, such as `nerdctl run -p 80:80 -p 81:81 ~ -p 1000:1000 ...` The current implementation is processing to create a container with the information specified in -p to the label. And as can be seen from the error message, as the number of ports to be port mapped increases, the creation of the container fails because it violates the limit of the maximum number of bytes on the containerd side that can be allocated for a label. Therefore, this PR modifies the container creation process so that containers can be launched without having to assign the information specified in the -p option to the labels. Specifically, port mapping information is stored in the following path, and when port mapping information is required, it is retrieved from this file. ``` <DATAROOT>/<ADDRHASH>/containers/<NAMESPACE>/<CID>/network-config.json ``` Signed-off-by: Hayato Kiwata <[email protected]>

haytok · 2025-06-06T10:56:44Z

Hi @AkihiroSuda @apostasie

Could you review when you have time ? 🙏

AkihiroSuda · 2025-06-06T16:41:23Z

pkg/labels/labels.go

@@ -57,9 +57,6 @@ const (
 	// Currently, the length of the slice must be 1.
 	Networks = Prefix + "networks"

-	// Ports is a JSON-marshalled string of []cni.PortMapping .
-	Ports = Prefix + "ports"


This change seems to break ports of containers created with an existing version of nerdctl ?

Then probably we should postpone this to v2.2.0

Thanks for comments!!!

This change is not backward compatible, so these backward incompatible changes should be postponed until the update from 2.1 to 2.2.
If so, is it okay to leave this PR open for now until the release of 2.2 approaches?
I'll rebase as necessary and ask you to review this PR when the release of 2.2 approaches.

Yes, please.

There should be also a human-friendly warning message when an incompatible container exists.

I'll add handling for cases where there are containers that are not backward compatible (e.g., when containers created using the -p option are already exist).

apostasie · 2025-06-06T18:25:08Z

cmd/nerdctl/container/container_run_network_linux_test.go

@@ -366,8 +365,8 @@ func TestRunWithInvalidPortThenCleanUp(t *testing.T) {
 			},
 			Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
 				return &test.Expected{
-					ExitCode: 1,
-					Errors:   []error{errdefs.ErrInvalidArgument},
+					ExitCode: 0,


So, is this test now working for both nerdctl and docker?
Should we lift the requirement?

apostasie · 2025-06-06T18:26:51Z

pkg/containerutil/container_network_manager.go

@@ -892,12 +892,6 @@ func NetworkOptionsFromSpec(spec *specs.Spec) (types.NetworkOptions, error) {
 	}
 	opts.NetworkSlice = networks

-	if portsJSON := spec.Annotations[labels.Ports]; portsJSON != "" {


PortMappings no longer set here?

apostasie · 2025-06-06T18:35:03Z

pkg/netutil/networkstore/networkstore.go

+	if err != nil {
+		return err
+	}
+	if _, err := os.Stat(loc); err != nil {


ns.SafeStore.Exists() instead of using os.Stat.
Also, do not ignore errors.

And move ns.SafeStore.Exists() inside the locked section.

apostasie · 2025-06-06T18:36:25Z

@haytok

What if we make this change backward compatible?

eg:

func LoadPortMappings(dataStore, namespace, id string) ([]cni.PortMapping, error) {
      // if there is no port information from NetworkStore, look into the labels

apostasie · 2025-06-06T18:37:43Z

@haytok a few comments above - also I think we should make this backward compatible (at least for reading the ports from the labels if they are there).

haytok · 2025-06-07T00:42:10Z

Hi @apostasie

Thanks for commens!!!

I hadn't considered backward compatibility at all, so I'll update based on your advices to make backward compatible.

haytok force-pushed the issue_4027 branch 2 times, most recently from 02db289 to c6e55d1 Compare May 28, 2025 01:01

AkihiroSuda added this to the v2.1.3 milestone May 30, 2025

apostasie reviewed May 30, 2025

View reviewed changes

pkg/portutil/portutil.go Outdated Show resolved Hide resolved

haytok force-pushed the issue_4027 branch 2 times, most recently from 854dce9 to 16ddd16 Compare June 2, 2025 14:28

haytok force-pushed the issue_4027 branch from 16ddd16 to 68534ed Compare June 2, 2025 14:37

AkihiroSuda reviewed Jun 6, 2025

View reviewed changes

AkihiroSuda modified the milestones: v2.1.3, v2.2.0 (?) Jun 6, 2025

apostasie reviewed Jun 6, 2025

View reviewed changes

fix: allow containers to start using a large numbers of ports #4290

Are you sure you want to change the base?

fix: allow containers to start using a large numbers of ports #4290

Conversation

haytok commented May 25, 2025

Uh oh!

haytok commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apostasie commented May 25, 2025

Uh oh!

haytok commented May 27, 2025

Uh oh!

Uh oh!

apostasie commented May 30, 2025

Uh oh!

AkihiroSuda commented May 31, 2025

Uh oh!

haytok commented Jun 1, 2025

Uh oh!

haytok commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AkihiroSuda Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

haytok Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

haytok Jun 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apostasie Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

apostasie Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

apostasie Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

apostasie Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

apostasie commented Jun 6, 2025

Uh oh!

apostasie commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haytok commented Jun 7, 2025

Uh oh!

Uh oh!

haytok commented May 25, 2025 •

edited

Loading

haytok commented Jun 6, 2025 •

edited

Loading

haytok Jun 7, 2025 •

edited

Loading

apostasie commented Jun 6, 2025 •

edited

Loading