feature: enable ipv6 networking #2417

houstar · 2018-11-03T05:19:57Z

Ⅰ. Describe what this PR did

add ipv6 and fixed-cidr-v6 into BridgeConfig
add ipv6 and fixed-cidr-v6 into DaemonConfig
add IPv6 network create inside BridgeMode

Ⅱ. Does this pull request fix one issue?

fixes #2415

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

These two commits already included the integration test cases

Ⅳ. Describe how to verify it

[root@qing-for-dev$ ]pouchd --ipv6 true --debug
Create container

[root@qing-for-dev$ ]pouch run -it gentoo/stage3-x86 bash
6cddc3c089e0 / # ip -6 a s
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 state UNKNOWN qlen 1
    inet6 ::1/128 scope host
       valid_lft forever preferred_lft forever
109: eth0@if110: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 state UP
    inet6 2002:db8:1::2/64 scope global nodad
       valid_lft forever preferred_lft forever
    inet6 fe80::42:c0ff:fea8:502/64 scope link dadfailed tentative
       valid_lft forever preferred_lft forever
6cddc3c089e0 / # ip -6 r s
2002:db8:1::/64 dev eth0 proto kernel metric 256
fe80::/64 dev eth0 proto kernel metric 256
default via 2002:db8:1::1 dev eth0 metric 1024
6cddc3c089e0 / # ping 2002:db8:1::1
PING 2002:db8:1::1(2002:db8:1::1) 56 data bytes
64 bytes from 2002:db8:1::1: icmp_seq=1 ttl=64 time=0.111 ms
64 bytes from 2002:db8:1::1: icmp_seq=2 ttl=64 time=0.058 ms
^C
--- 2002:db8:1::1 ping statistics ---
2 packets transmitted, 2 received, 0% packet loss, time 999ms
rtt min/avg/max/mdev = 0.058/0.084/0.111/0.028 ms

Ⅴ. Special notes for reviews

There is issue with vendor package `github.com/docker/libnetwork' when assign IPv6 address:

failed to create endpoint: invalid bit range [0, 18446744073709551615]

codecov · 2018-11-03T05:19:59Z

Codecov Report

Merging #2417 into master will decrease coverage by 0.18%.
The diff coverage is 44.06%.

@@            Coverage Diff             @@
##           master    #2417      +/-   ##
==========================================
- Coverage   68.97%   68.79%   -0.19%     
==========================================
  Files         277      277              
  Lines       18298    18330      +32     
==========================================
- Hits        12621    12610      -11     
- Misses       4253     4281      +28     
- Partials     1424     1439      +15

Flag	Coverage Δ
#criv1alpha1test	`31.55% <44.06%> (-0.06%)`	⬇️
#criv1alpha2test	`35.65% <44.06%> (+0.06%)`	⬆️
#integrationtest	`40.4% <44.06%> (ø)`	⬆️
#nodee2etest	`32.78% <44.06%> (-0.28%)`	⬇️
#unittest	`26.69% <ø> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
network/mode/bridge/bridge.go	`44.44% <44.06%> (-7.35%)`	⬇️
cri/ocicni/cni_manager.go	`58.82% <0%> (-11.77%)`	⬇️
cri/v1alpha2/cri.go	`69.02% <0%> (-1.27%)`	⬇️
cri/v1alpha2/cri_wrapper.go	`62.4% <0%> (-1.21%)`	⬇️
ctrd/container.go	`58.1% <0%> (-0.8%)`	⬇️
daemon/mgr/container.go	`58.36% <0%> (-0.44%)`	⬇️
daemon/mgr/network.go	`72.96% <0%> (+0.47%)`	⬆️
daemon/logger/jsonfile/utils.go	`73.17% <0%> (+1.62%)`	⬆️
pkg/streams/utils.go	`89.28% <0%> (+7.14%)`	⬆️

allencloud · 2018-11-05T04:56:45Z

Is it OK to add some test of this? @houstar
I think test case could cover the features you added mostly. WDYT?

houstar · 2018-11-05T06:40:11Z

@allencloud OK. I"m working in progress with ipv6 testcases.

houstar · 2018-11-05T07:41:38Z

@allencloud @rudyfly

Done with the IPv6 validation. Welcome to raise any comments.

rudyfly · 2018-11-06T07:01:03Z

network/mode/bridge/bridge.go

 	}

 	ipam := &types.IPAM{
 		Driver: "default",
 		Config: []types.IPAMConfig{ipamV4Conf},
 	}

+	if config.IPv6 {


I think if we support ipv6, we should modify common function, instead of this way:

if ipv6 { ...... } else { ...... }

WDYT?

Updated according to your comments :-)

rudyfly · 2018-11-06T07:02:11Z

vendor/github.com/docker/libnetwork/bitseq/sequence.go

@@ -206,7 +206,7 @@ func (h *Handle) getCopy() *Handle {

 // SetAnyInRange atomically sets the first unset bit in the specified range in the sequence and returns the corresponding ordinal
 func (h *Handle) SetAnyInRange(start, end uint64, serial bool) (uint64, error) {
-	if end < start || end >= h.bits {
+	if end < start || end > h.bits {


Have you update the vendor libnetwork?

We have vendored the libnetwork is the alibaba libnetwork, you can see: https://github.com/alibaba/libnetwork, if you want to modify it, you can send PR to our libnetwork`

Done. please help to merge. thanks.

rudyfly · 2018-11-07T06:20:28Z

network/mode/bridge/bridge.go


 	// init host bridge network.
-	_, err = initBridgeDevice(bridgeName, ipNet)
+	_, err = initBridgeDevice(bridgeName, ipv4Net)


bridge device should support ipv6

@rudyfly

Bridge device already supported IPv6 since it deferred the IPv6 allocation by libnetwork IPAM module. So I think that there is no need to add assign bridge IPv6 address in here. WDYT ?

Intergration-test might be blocked by other test suite. So I'll work on the integration-test if upstream integration-test is ready.

Currently could you help to merge the PR to alibaba/libnetwork firstly? then I can vendor the new commit to build the integration-test.

For the failure of the upstream pouch integration-test. I also Cc'ed @allencloud
Steps:

$mkdir -p $GOPATH/src/github.com/alibaba && git clone https://github.com/alibaba/pouch $cd $GOPATH/src/github.com/alibaba/pouch && make build-integration-test $make integration-test integration-test ./hack/testing/run_daemon_integration.sh stop local-persist volume plugin... start local-persist volume plugin... stop pouch daemon... start pouch daemon... failed to start pouch daemon in background! make: *** [integration-test] Error 1

rudyfly · 2018-11-07T06:28:13Z

Could you add integration test for ipv6? and rebase and merge your commits?

houstar · 2018-11-07T07:20:10Z

Could you add integration test for ipv6?

OK. I will cover these integration test.

and rebase and merge your commits?

Is it rebase and merge above commits into one commit ?

rudyfly · 2018-11-07T09:19:45Z

@houstar
and rebase and merge your commits?

Is it rebase and merge above commits into one commit ?

yes

houstar · 2018-11-08T06:49:56Z

Modified as your recommendation. Thanks for your carefully review

/assign

houstar · 2018-11-09T02:08:16Z

@rudyfly Done with the network integration-test, could you help review and merge ?

  272 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:111: PouchNetworkSuite.TestNetworkBridgeWorks 22.402s
  273 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:441: PouchNetworkSuite.TestNetworkConnect 5.349s
  274 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:519: PouchNetworkSuite.TestNetworkConnectWithRestart  3.788s
  275 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:349: PouchNetworkSuite.TestNetworkCreateDup   0.255s
  276 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:247: PouchNetworkSuite.TestNetworkCreateWithLabel 0.416s
  277 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:298: PouchNetworkSuite.TestNetworkCreateWithOption    0.411s
  278 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:227: PouchNetworkSuite.TestNetworkCreateWrongDriver   15.099s
  279 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:59: PouchNetworkSuite.TestNetworkDefault  6.031s
  280 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:486: PouchNetworkSuite.TestNetworkDisconnect  4.313s
  281 PASS: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:35: PouchNetworkSuite.TestNetworkInspectFormat    0.186s
  282
  283 ----------------------------------------------------------------------
  284 FAIL: /root/src/github.com/alibaba/pouch/test/cli_network_test.go:383: PouchNetworkSuite.TestNetworkPortMapping
  285
  286 /root/src/github.com/alibaba/pouch/test/cli_network_test.go:415:
  287     c.Assert(success, check.Equals, true)
  288 ... obtained bool = false
  289 ... expected bool = true

houstar · 2018-11-15T05:18:17Z

@rudyfly Any new comments?

rudyfly · 2018-11-19T09:16:01Z

network/config.go

 	GatewayIPv4 string `json:"default-gateway,omitempty"`
+	IPv6        bool   `json:"ipv6,omitempty"`


s/IPv6/EnableIPv6
and also change it in json format
WDYT？

rudyfly · 2018-11-19T09:55:44Z

@houstar I'm sorry to late to review this pr, because of kubecon.
I have tested it on my ubuntu, it works,
Also need rebase master, and then LGTM.

Signed-off-by: Leno Hou <[email protected]>

* add ipv6 and fixed-cidr-v6 Daemon flag * add IPv6 validation and integration test Signed-off-by: Leno Hou <[email protected]>

houstar · 2018-11-19T11:50:47Z

@rudyfly Take it easy, man . ~~~~ . :~)

rudyfly · 2018-11-19T12:36:57Z

LGTM

pouchrobot added areas/network kind/feature size/L labels Nov 3, 2018

houstar force-pushed the feature/ipv6 branch from 4f7e363 to 847f289 Compare November 3, 2018 12:03

allencloud assigned rudyfly Nov 5, 2018

houstar force-pushed the feature/ipv6 branch from 847f289 to 813eb7b Compare November 5, 2018 07:38

rudyfly reviewed Nov 6, 2018

View reviewed changes

houstar force-pushed the feature/ipv6 branch from 813eb7b to 72de7de Compare November 6, 2018 18:06

pouchrobot added size/XL and removed size/L labels Nov 6, 2018

rudyfly reviewed Nov 7, 2018

View reviewed changes

houstar force-pushed the feature/ipv6 branch from 72de7de to 6b9131c Compare November 8, 2018 03:00

houstar force-pushed the feature/ipv6 branch from 6b9131c to 5bd661d Compare November 8, 2018 14:31

rudyfly reviewed Nov 19, 2018

View reviewed changes

houstar force-pushed the feature/ipv6 branch from 5bd661d to 33df349 Compare November 19, 2018 09:40

houstar added 2 commits November 19, 2018 11:46

vendor: update 'github.com/alibaba/libnetwork/bitseq'

10233e6

Signed-off-by: Leno Hou <[email protected]>

feature: enable IPv6 networking

c2f8e75

* add ipv6 and fixed-cidr-v6 Daemon flag * add IPv6 validation and integration test Signed-off-by: Leno Hou <[email protected]>

houstar force-pushed the feature/ipv6 branch from 33df349 to c2f8e75 Compare November 19, 2018 11:49

rudyfly merged commit e7dc811 into AliyunContainerService:master Nov 19, 2018

pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Nov 19, 2018

houstar mentioned this pull request Nov 22, 2018

integration issue: failed to start pouch daemon in background #2444

Closed

pouchrobot mentioned this pull request Nov 23, 2018

WeeklyReport of PouchContainer from 2018-11-16 to 2018-11-23 #2493

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: enable ipv6 networking #2417

feature: enable ipv6 networking #2417

houstar commented Nov 3, 2018 •

edited

Loading

codecov bot commented Nov 3, 2018 •

edited

Loading

allencloud commented Nov 5, 2018

houstar commented Nov 5, 2018

houstar commented Nov 5, 2018

rudyfly Nov 6, 2018

houstar Nov 6, 2018

rudyfly Nov 6, 2018

rudyfly Nov 6, 2018 •

edited

Loading

houstar Nov 6, 2018

rudyfly Nov 7, 2018

houstar Nov 7, 2018

rudyfly commented Nov 7, 2018

houstar commented Nov 7, 2018

rudyfly commented Nov 7, 2018

houstar commented Nov 8, 2018

houstar commented Nov 9, 2018

houstar commented Nov 15, 2018

rudyfly Nov 19, 2018 •

edited

Loading

houstar Nov 19, 2018

rudyfly commented Nov 19, 2018 •

edited

Loading

houstar commented Nov 19, 2018

rudyfly commented Nov 19, 2018

		GatewayIPv4 string `json:"default-gateway,omitempty"`
		IPv6 bool `json:"ipv6,omitempty"`

feature: enable ipv6 networking #2417

feature: enable ipv6 networking #2417

Conversation

houstar commented Nov 3, 2018 • edited Loading

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov bot commented Nov 3, 2018 • edited Loading

Codecov Report

allencloud commented Nov 5, 2018

houstar commented Nov 5, 2018

houstar commented Nov 5, 2018

rudyfly Nov 6, 2018

Choose a reason for hiding this comment

houstar Nov 6, 2018

Choose a reason for hiding this comment

rudyfly Nov 6, 2018

Choose a reason for hiding this comment

rudyfly Nov 6, 2018 • edited Loading

Choose a reason for hiding this comment

houstar Nov 6, 2018

Choose a reason for hiding this comment

rudyfly Nov 7, 2018

Choose a reason for hiding this comment

houstar Nov 7, 2018

Choose a reason for hiding this comment

rudyfly commented Nov 7, 2018

houstar commented Nov 7, 2018

rudyfly commented Nov 7, 2018

houstar commented Nov 8, 2018

houstar commented Nov 9, 2018

houstar commented Nov 15, 2018

rudyfly Nov 19, 2018 • edited Loading

Choose a reason for hiding this comment

houstar Nov 19, 2018

Choose a reason for hiding this comment

rudyfly commented Nov 19, 2018 • edited Loading

houstar commented Nov 19, 2018

rudyfly commented Nov 19, 2018

houstar commented Nov 3, 2018 •

edited

Loading

codecov bot commented Nov 3, 2018 •

edited

Loading

rudyfly Nov 6, 2018 •

edited

Loading

rudyfly Nov 19, 2018 •

edited

Loading

rudyfly commented Nov 19, 2018 •

edited

Loading