fix: Add used port check for publish flag

[vsiravar]

Add check for used ports while allocating host port in -p/--publish to make it compatible with docker.

Fixes: https://github.com/containerd/nerdctl/issues/2179

[junnplus]

Test failed https://github.com/containerd/nerdctl/actions/runs/4814951537/jobs/8573169644?pr=2190#step:6:982

time="2023-04-27T01:30:38Z" level=fatal msg="failed to load networking flags: bind for :8080 failed: port is already allocated"

[vsiravar]

Test failed https://github.com/containerd/nerdctl/actions/runs/4814951537/jobs/8573169644?pr=2190#step:6:982

time="2023-04-27T01:30:38Z" level=fatal msg="failed to load networking flags: bind for :8080 failed: port is already allocated"

This is a test seems is testing TestComposeUpBuild in compose up , not sure if this is related to my change or a flaky test.

[junnplus]

Can you test this case locally? It seems different from what was expected?

$ sudo nerdctl run -p 8080:80 --name=test -d nginx
7f4a2be3bd30eba9d9b5c45ea83213af9728fa456bd3b23783a3352282afc078
$ sudo nerdctl stop test
test
$ sudo nerdctl run -p 8080:80 -d nginx
FATA[0000] failed to load networking flags: bind for :8080 failed: port is already allocated

[fahedouch]

Test failed https://github.com/containerd/nerdctl/actions/runs/4814951537/jobs/8573169644?pr=2190#step:6:982

time="2023-04-27T01:30:38Z" level=fatal msg="failed to load networking flags: bind for :8080 failed: port is already allocated"

This is a test seems is testing TestComposeUpBuild in compose up , not sure if this is related to my change or a flaky test.

Not related. It is a flaky test

[AkihiroSuda]

@vsiravar Could you confirm this? https://github.com/containerd/nerdctl/pull/2190#issuecomment-1529213083

[vsiravar]

@vsiravar Could you confirm this? #2190 (comment)

Sorry I have been a bit busy. I will have all the comments addressed this weekend.

Can you test this case locally? It seems different from what was expected?

$ sudo nerdctl run -p 8080:80 --name=test -d nginx
7f4a2be3bd30eba9d9b5c45ea83213af9728fa456bd3b23783a3352282afc078
$ sudo nerdctl stop test
test
$ sudo nerdctl run -p 8080:80 -d nginx
FATA[0000] failed to load networking flags: bind for :8080 failed: port is already allocated

I think this is because nerdctl stop does not clean up the network unlike nerdctl remove . I can make a follow up PR to fix this, if it's the behavior we want. cc @junnplus

Edit: Tagged Jun for feeback.

[fahedouch]

@vsiravar Could you confirm this? #2190 (comment)

Sorry I have been a bit busy. I will have all the comments addressed this weekend.

Can you test this case locally? It seems different from what was expected?

$ sudo nerdctl run -p 8080:80 --name=test -d nginx
7f4a2be3bd30eba9d9b5c45ea83213af9728fa456bd3b23783a3352282afc078
$ sudo nerdctl stop test
test
$ sudo nerdctl run -p 8080:80 -d nginx
FATA[0000] failed to load networking flags: bind for :8080 failed: port is already allocated

I think this is because nerdctl stop does not clean up the network unlike nerdctl remove . I can make a follow up PR to fix this, if it's the behavior we want. cc @junnplus

Edit: Tagged Jun for feeback.

LGTM it is the nerdctl stop responsibility to binded free port(s)

[AkihiroSuda]

@junnplus LGTY?

[junnplus]

I think this is because nerdctl stop does not clean up the network unlike nerdctl remove . I can make a follow up PR to fix this, if it's the behavior we want. cc @junnplus

I apologize for just seeing the new update, thank you for your hard work, I will review it this week.

[junnplus]

LGTM it is the nerdctl stop responsibility to binded free port(s)

I concur with it, but hold reservations about merging this PR. I believe we should first have nerdctl stop manage the release of ports.

The rest LGTM.

[fahedouch]

LGTM it is the nerdctl stop responsibility to binded free port(s)

I concur with it, but hold reservations about merging this PR. I believe we should first have nerdctl stop manage the release of ports.

The rest LGTM.

cc vsiravar, could you please fix the stop ( it can be a different PR) so we can merge this accordingly? Thanks

[vsiravar]

LGTM it is the nerdctl stop responsibility to binded free port(s)

I concur with it, but hold reservations about merging this PR. I believe we should first have nerdctl stop manage the release of ports. The rest LGTM.

cc vsiravar, could please fix the stop ( it can be a different PR) so we can merge this accordingly? Thanks

Sure, I'll work on it in the next couple of weeks.

[vsiravar]

LGTM it is the nerdctl stop responsibility to binded free port(s)

I concur with it, but hold reservations about merging this PR. I believe we should first have nerdctl stop manage the release of ports. The rest LGTM.

cc vsiravar, could you please fix the stop ( it can be a different PR) so we can merge this accordingly? Thanks

Sure. I will have a PR out this week. Cheers!

[AkihiroSuda]

What's the current status of this?

[vsiravar]

What's the current status of this?

I think this PR is good. I am still working on the PR to fix the stop to free binded ports on stop.

[AkihiroSuda]

@junnplus PTAL