-
Notifications
You must be signed in to change notification settings - Fork 720
Default net.ipv4.ip_unprivileged_port_start to 0 inside containers #4615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Default net.ipv4.ip_unprivileged_port_start to 0 inside containers #4615
Conversation
pkg/cmd/container/create.go
Outdated
| opts = append(opts, umaskOpts...) | ||
|
|
||
| if !isHostNetwork(netLabelOpts) { | ||
| opts = append(opts, withDefaultUnprivilegedPortSysctl(options.Sysctl)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
conflicts with:
nerdctl/pkg/cmd/container/run_linux.go
Line 100 in 1d69ed9
| opts = append(opts, WithSysctls(strutil.ConvertKVStringsToMap(options.Sysctl))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the defaulting for net.ipv4.ip_unprivileged_port_start in create.go, building on top of the behavior in run_linux.go. Does this still conflict?
|
Please fix the lint errors, squash the commits, and sign off the DCO |
9d214b1 to
56f05ed
Compare
|
Hi @AkihiroSuda, should all the tests be passing? |
|
The tests for this update has failed. === Failed
=== FAIL: cmd/nerdctl/container TestContainerInspectHostConfigDefaults (0.67s)
container_inspect_linux_test.go:343: HostConfig in TestContainerInspectHostConfigDefaults: &{ContainerIDFile: LogConfig:{Driver:json-file Opts:map[] LogURI: Address:/run/containerd/containerd.sock} PortBindings:map[] CgroupnsMode:private DNS:[] DNSOptions:[] DNSSearch:[] ExtraHosts:[] GroupAdd:[1 2 3 4 6 10 11 20 26 27] IpcMode:private OomScoreAdj:0 PidMode: ReadonlyRootfs:false Tmpfs:map[] UTSMode: ShmSize:0 Sysctls:map[net.ipv4.ip_unprivileged_port_start:0] Runtime:io.containerd.runc.v2 CPUSetMems: CPUSetCPUs: CPUQuota:0 CPUShares:0 CPUPeriod:0 CPURealtimePeriod:0 CPURealtimeRuntime:0 Memory:0 MemorySwap:0 OomKillDisable:false Devices:[] BlkioSettings:{BlkioWeight:0 BlkioWeightDevice:[] BlkioDeviceReadBps:[] BlkioDeviceWriteBps:[] BlkioDeviceReadIOps:[] BlkioDeviceWriteIOps:[]}}
container_inspect_linux_test.go:365: assertion failed: 0 (int) != 1 (int)Therefore, at least the following fixes are required:
|
a8eb601 to
96455de
Compare
|
Hi, the only failing job is in-host - Windows. Looking at the logs, all the
This seems like an environment issue in the Windows runner rather than something caused by my PR. Does this test need to be resolved before merging my PR? |
|
Can you try rebasing with the current main branch ? |
96455de to
0a591e8
Compare
|
Doesn't look rebased well |
…iners Signed-off-by: Yash Kukrecha <[email protected]>
b262ece to
bc45754
Compare
|
I rebased and it resulted in some failing flaky tests but also some failures unrelated to my PR, such as logging and binding to host port 5000. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why Windows CI is still failing.
Maybe GHA is now deterministically picking a broken runner instance by the PR number?
This PR makes nerdctl default the container's
net.ipv4.ip_unprivileged_port_startsysctl to0, unless the user has explicitly set this sysctl via--sysctl.Key changes:
withDefaultUnprivilegedPortSysctlinpkg/cmd/container/container.go.--sysctlfornet.ipv4.ip_unprivileged_port_start, nerdctl does not override it.Note: Host-wide sysctl configuration and containerd-rootless-setuptool.sh were intentionally left unchanged in this PR to keep the scope focused on the container namespace default requested in the issue.
Fixes #4595