Skip to content

Commit 1aec7ce

Browse files
authored
feat: Use nerdctl parsing logic for port publishing (runfinch#265)
Signed-off-by: Swagat Bora <[email protected]>
1 parent 8fc3a1e commit 1aec7ce

File tree

3 files changed

+60
-18
lines changed

3 files changed

+60
-18
lines changed

api/handlers/container/create.go

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ import (
99
"net/http"
1010
"os"
1111
"path/filepath"
12-
"strconv"
1312
"strings"
1413

1514
"github.com/containerd/containerd/v2/pkg/namespaces"
1615
gocni "github.com/containerd/go-cni"
1716
ncTypes "github.com/containerd/nerdctl/v2/pkg/api/types"
1817
"github.com/containerd/nerdctl/v2/pkg/defaults"
18+
"github.com/containerd/nerdctl/v2/pkg/portutil"
1919
"github.com/docker/go-connections/nat"
2020
"github.com/moby/moby/api/types/blkiodev"
2121
"github.com/sirupsen/logrus"
@@ -320,7 +320,6 @@ func (h *handler) create(w http.ResponseWriter, r *http.Request) {
320320
// #endregion
321321

322322
}
323-
324323
portMappings, err := translatePortMappings(req.HostConfig.PortBindings)
325324
if err != nil {
326325
logrus.Debugf("failed to parse port mappings: %s", err)
@@ -393,25 +392,30 @@ func translatePortMappings(portMappings nat.PortMap) ([]gocni.PortMapping, error
393392
if portMappings == nil {
394393
return ports, nil
395394
}
395+
seenPortMappings := make(map[string]bool)
396396
for portName, portBindings := range portMappings {
397397
for _, portBinding := range portBindings {
398-
hostPort, err := strconv.ParseInt(portBinding.HostPort, 10, 32)
399-
if err != nil {
400-
return []gocni.PortMapping{}, fmt.Errorf("failed to parse host port (%s) to integer: %w", portBinding.HostPort, err)
398+
portStr := ""
399+
if portBinding.HostIP != "" {
400+
portStr += portBinding.HostIP + ":"
401401
}
402-
// Cannot use portName.Int() because it assumes nat.NewPort() was used
403-
// for error handling.
404-
containerPort, err := strconv.ParseInt(portName.Port(), 10, 32)
405-
if err != nil {
406-
return []gocni.PortMapping{}, fmt.Errorf("failed to parse container port (%s) to integer: %w", portName, err)
402+
403+
// Add host port (or empty string if not specified)
404+
portStr += portBinding.HostPort + ":"
405+
406+
// Add container port and protocol
407+
portStr += portName.Port() + "/" + portName.Proto()
408+
409+
if seenPortMappings[portStr] {
410+
continue
407411
}
408-
portMap := gocni.PortMapping{
409-
HostPort: int32(hostPort),
410-
ContainerPort: int32(containerPort),
411-
Protocol: portName.Proto(),
412-
HostIP: portBinding.HostIP,
412+
seenPortMappings[portStr] = true
413+
414+
portMaps, err := portutil.ParseFlagP(portStr)
415+
if err != nil {
416+
return []gocni.PortMapping{}, fmt.Errorf("failed to parse port mapping %q: %w", portStr, err)
413417
}
414-
ports = append(ports, portMap)
418+
ports = append(ports, portMaps...)
415419
}
416420
}
417421
return ports, nil

api/handlers/container/create_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ var _ = Describe("Container Create API ", func() {
101101
HostPort: 8001,
102102
ContainerPort: 8000,
103103
Protocol: "tcp",
104-
HostIP: "",
104+
HostIP: "0.0.0.0",
105105
},
106106
{
107107
HostPort: 9001,
@@ -140,7 +140,7 @@ var _ = Describe("Container Create API ", func() {
140140
"ExposedPorts": {"8000/tcp": {}},
141141
"HostConfig": {
142142
"PortBindings": {
143-
"8000/tcp": [{"HostIp": "", "HostPort": ""}],
143+
"8000/tcp": [{"HostIp": "", "HostPort": "invalid-port"}]
144144
}
145145
}
146146
}`)

e2e/tests/container_create.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,44 @@ func ContainerCreate(opt *option.Option, pOpt util.NewOpt) {
210210
Expect(portMap[udpPort][0]).Should(Equal(udpPortBinding))
211211
})
212212

213+
It("should create a container with automatic port allocation on the host", func() {
214+
ctrPort := "8080"
215+
216+
// define options with empty host port for automatic allocation
217+
tcpPort := nat.Port(fmt.Sprintf("%s/tcp", ctrPort))
218+
tcpPortBinding := nat.PortBinding{HostIP: "0.0.0.0", HostPort: ""}
219+
220+
options.Cmd = []string{"sleep", "Infinity"}
221+
options.HostConfig.PortBindings = nat.PortMap{
222+
tcpPort: []nat.PortBinding{tcpPortBinding},
223+
}
224+
225+
// create and start container
226+
statusCode, ctr := createContainer(uClient, url, testContainerName, options)
227+
Expect(statusCode).Should(Equal(http.StatusCreated))
228+
Expect(ctr.ID).ShouldNot(BeEmpty())
229+
command.Run(opt, "start", testContainerName)
230+
231+
// inspect container
232+
resp := command.Stdout(opt, "inspect", testContainerName)
233+
var inspect []*dockercompat.Container
234+
err := json.Unmarshal(resp, &inspect)
235+
Expect(err).Should(BeNil())
236+
Expect(inspect).Should(HaveLen(1))
237+
238+
// verify port mappings with automatic allocation
239+
Expect(inspect[0].NetworkSettings).ShouldNot(BeNil())
240+
portMap := *inspect[0].NetworkSettings.Ports
241+
Expect(portMap).Should(HaveLen(1))
242+
Expect(portMap[tcpPort]).Should(HaveLen(1))
243+
Expect(portMap[tcpPort][0].HostIP).Should(Equal("0.0.0.0"))
244+
Expect(portMap[tcpPort][0].HostPort).ShouldNot(BeEmpty())
245+
// Verify that a port was actually allocated (not empty string)
246+
port, err := strconv.Atoi(portMap[tcpPort][0].HostPort)
247+
Expect(err).Should(BeNil())
248+
Expect(port).Should(BeNumerically(">", 0))
249+
})
250+
213251
// Volume Mounts
214252

215253
It("should create a container with a directory mounted from the host", func() {

0 commit comments

Comments
 (0)