-
Notifications
You must be signed in to change notification settings - Fork 601
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
Setup ipv6 hostip #127
Setup ipv6 hostip #127
Conversation
d80f5b4
to
d28d4bf
Compare
Doesn't seem related to #78, but I agree we should have this. |
436fce9
to
25fc4ca
Compare
@AkihiroSuda as we discussed on slack the |
@@ -328,7 +328,7 @@ func runAction(clicontext *cli.Context) error { | |||
portSlice := strutil.DedupeStrSlice(clicontext.StringSlice("p")) | |||
netSlice := strutil.DedupeStrSlice(clicontext.StringSlice("net")) | |||
|
|||
ports := make([]gocni.PortMapping, len(portSlice)) | |||
ports := make([]gocni.PortMapping, 0) |
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.
Unneeded change
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.
the length of ports slice should be 0 to avoid zero value (nil), nil value in this slice lead nil pointer execption.
run_test.go
Outdated
@@ -137,3 +137,20 @@ func TestRunExitCode(t *testing.T) { | |||
assert.Equal(base.T, "exited", inspect123.State.Status) | |||
assert.Equal(base.T, 123, inspect123.State.ExitCode) | |||
} | |||
|
|||
func TestRunPortMappingWithEmptyIP(t *testing.T) { |
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 think these tests can be just UT, but if you prefer to have them as integration tests, these should be in port_test.go
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.
you mean moving these tests into a port_test.go
file will make them integration tests :o ?
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.
@AkihiroSuda can you check plz ?
Setting up port forwarding for IPv6 does not require IPv6 routing |
.github/workflows/test.yml
Outdated
sudo cp /etc/docker/daemon.json /tmp/daemon.json.tmp | ||
sudo truncate -s 0 /etc/docker/daemon.json | ||
jq '. |= . + { "ipv6": true, "fixed-cidr-v6": "fd00::/64" }' /tmp/daemon.json.tmp | sudo tee -a /etc/docker/daemon.json | ||
sudo rm /tmp/daemon.json.tmp |
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 suggest using sponge
https://stackoverflow.com/questions/36565295/jq-to-replace-text-directly-on-file-like-sed-i
ecce5c6
to
5c772cd
Compare
Signed-off-by: fahed dorgaa <[email protected]>
5c772cd
to
99d3447
Compare
$ sudo nerdctl run -d --name nginx -p [::1]:80:80 nginx:alpine
585c292270edc4b81150df5f0bc02ca5fde1048c9610ca18823b474c9a1123ff
$ curl http://[::1]
curl: (7) Failed to connect to ::1 port 80: Connection refused Doesn't work |
hi @AkihiroSuda You should configure your cni to attribue an ipv6 to the container.
then I create a container with
Check the container ipv6
And then I exec into the container to listen on 80
then try to curl this container on its ipv6
the curl is ok :
|
Doesn't seem related to port forwarding |
Merged another PR: |
fixing bug #78 (comment)