-
Notifications
You must be signed in to change notification settings - Fork 672
add support for ips
in ipam config
#3754
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -128,6 +128,9 @@ func (c *claim) Try(alloc *Allocator) bool { | |
// but we also cannot prove otherwise, so we let it reclaim the address: | ||
alloc.debugln("Re-Claimed", c.cidr, "for ID", c.ident, "having existing ID as", existingIdent) | ||
c.sendResult(nil) | ||
case c.ident == "weave:expose": | ||
alloc.debugln("Ignoring weave:expose") | ||
c.sendResult(nil) | ||
Comment on lines
+131
to
+133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bboreham I am not entirely sure what's going on here. Without this case that ignores
I added the Looks like |
||
default: | ||
// Addr already owned by container on this machine | ||
c.sendResult(fmt.Errorf("address %s is already owned by %s", c.cidr.String(), existingIdent)) | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,6 +2,7 @@ package ipamplugin | |||||
|
||||||
import ( | ||||||
"encoding/json" | ||||||
"errors" | ||||||
"fmt" | ||||||
"net" | ||||||
|
||||||
|
@@ -35,31 +36,80 @@ func (i *Ipam) Allocate(args *skel.CmdArgs) (types.Result, error) { | |||||
if containerID == "" { | ||||||
return nil, fmt.Errorf("Weave CNI Allocate: blank container name") | ||||||
} | ||||||
var ipnet *net.IPNet | ||||||
|
||||||
if conf.Subnet == "" { | ||||||
ipnet, err = i.weave.AllocateIP(containerID, false) | ||||||
var ipconfigs []*current.IPConfig | ||||||
|
||||||
if len(conf.IPs) > 0 { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bboreham I am a bit confused with how to support >1 IPs passed to the CNI via a
and
Can a given interface (i.e. I hope that makes sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, an interface can have many IP addresses. These days one IPv4 and one IPv6 is quite common (though not in Weave Net). What you did is OK, though the Weave Net CNI plugin will currently only use the first address: Lines 85 to 86 in b04217d
|
||||||
// configuration includes desired IPs | ||||||
|
||||||
var ips []net.IP | ||||||
for _, ip := range conf.IPs { | ||||||
ip4 := net.ParseIP(ip).To4() | ||||||
ip16 := net.ParseIP(ip).To16() | ||||||
|
||||||
if ip4 == nil && ip16 == nil { | ||||||
return nil, errors.New("provided value is not an IP") | ||||||
} | ||||||
|
||||||
if ip4 == nil && ip16 != nil { | ||||||
return nil, errors.New("allocation of ipv6 addresses is not implemented") | ||||||
nonsense marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
ips = append(ips, ip4) | ||||||
} | ||||||
|
||||||
for j := range ips { | ||||||
ipnet := &net.IPNet{ | ||||||
IP: ips[j], | ||||||
Mask: ips[j].DefaultMask(), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems wrong; the IPs are coming back as |
||||||
} | ||||||
|
||||||
err := i.weave.ClaimIP(containerID, ipnet, false) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
|
||||||
ipconfigs = append(ipconfigs, ¤t.IPConfig{ | ||||||
Version: "4", | ||||||
Address: *ipnet, | ||||||
Gateway: conf.Gateway, | ||||||
}) | ||||||
} | ||||||
} else if conf.Subnet == "" { | ||||||
// configuration doesn't include Subnet or IPs, so ask the allocator for an IP | ||||||
ipnet, err := i.weave.AllocateIP(containerID, false) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
|
||||||
ipconfigs = append(ipconfigs, ¤t.IPConfig{ | ||||||
Version: "4", | ||||||
Address: *ipnet, | ||||||
Gateway: conf.Gateway, | ||||||
}) | ||||||
} else { | ||||||
var subnet *net.IPNet | ||||||
subnet, err = types.ParseCIDR(conf.Subnet) | ||||||
// configuration includes desired Subnet | ||||||
|
||||||
subnet, err := types.ParseCIDR(conf.Subnet) | ||||||
if err != nil { | ||||||
return nil, fmt.Errorf("subnet given in config, but not parseable: %s", err) | ||||||
} | ||||||
ipnet, err = i.weave.AllocateIPInSubnet(containerID, subnet, false) | ||||||
} | ||||||
ipnet, err := i.weave.AllocateIPInSubnet(containerID, subnet, false) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
|
||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
result := ¤t.Result{ | ||||||
IPs: []*current.IPConfig{{ | ||||||
ipconfigs = append(ipconfigs, ¤t.IPConfig{ | ||||||
Version: "4", | ||||||
Address: *ipnet, | ||||||
Gateway: conf.Gateway, | ||||||
}}, | ||||||
Routes: conf.Routes, | ||||||
}) | ||||||
} | ||||||
return result, nil | ||||||
|
||||||
return ¤t.Result{ | ||||||
IPs: ipconfigs, | ||||||
Routes: conf.Routes, | ||||||
}, nil | ||||||
} | ||||||
|
||||||
func (i *Ipam) CmdDel(args *skel.CmdArgs) error { | ||||||
|
@@ -74,6 +124,7 @@ type ipamConf struct { | |||||
Subnet string `json:"subnet,omitempty"` | ||||||
Gateway net.IP `json:"gateway,omitempty"` | ||||||
Routes []*types.Route `json:"routes"` | ||||||
IPs []string `json:"ips,omitempty"` | ||||||
} | ||||||
|
||||||
type netConf struct { | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
#! /bin/bash | ||
|
||
. "$(dirname "$0")/config.sh" | ||
|
||
start_suite "Test CNI plugin with static IP allocation" | ||
|
||
cni_connect() { | ||
pid=$(container_pid $1 $2) | ||
id=$(docker_on $1 inspect -f '{{.Id}}' $2) | ||
run_on $1 sudo CNI_COMMAND=ADD CNI_CONTAINERID=$id CNI_IFNAME=eth0 \ | ||
CNI_NETNS=/proc/$pid/ns/net CNI_PATH=/opt/cni/bin /opt/cni/bin/weave-net | ||
} | ||
|
||
run_on $HOST1 sudo mkdir -p /opt/cni/bin | ||
# setup-cni is a subset of 'weave setup', without doing any 'docker pull's | ||
weave_on $HOST1 setup-cni --log-level=debug | ||
weave_on $HOST1 launch --log-level=debug | ||
|
||
C0=$(docker_on $HOST1 run --net=none --name=c0 --privileged -dt $SMALL_IMAGE /bin/sh) | ||
C1=$(docker_on $HOST1 run --net=none --name=c1 --privileged -dt $SMALL_IMAGE /bin/sh) | ||
C2=$(docker_on $HOST1 run --net=none --name=c2 --privileged -dt $SMALL_IMAGE /bin/sh) | ||
|
||
# Enable unsolicited ARPs so that ping after the address reuse does not time out | ||
exec_on $HOST1 c1 sysctl -w net.ipv4.conf.all.arp_accept=1 | ||
|
||
cni_connect $HOST1 c0 <<EOF | ||
{ | ||
"name": "weave", | ||
"type": "weave-net", | ||
"ipam": { | ||
"type": "weave-ipam", | ||
"ips": [ | ||
"10.32.1.30" | ||
] | ||
}, | ||
"hairpinMode": true | ||
} | ||
EOF | ||
|
||
cni_connect $HOST1 c1 <<EOF | ||
{ | ||
"name": "weave", | ||
"type": "weave-net", | ||
"ipam": { | ||
"type": "weave-ipam", | ||
"ips": [ | ||
"10.32.1.40" | ||
] | ||
}, | ||
"hairpinMode": true | ||
} | ||
EOF | ||
|
||
cni_connect $HOST1 c2 <<EOF | ||
{ | ||
"name": "weave", | ||
"type": "weave-net", | ||
"ipam": { | ||
"type": "weave-ipam", | ||
"ips": [ | ||
"10.32.1.42" | ||
] | ||
}, | ||
"hairpinMode": true | ||
} | ||
EOF | ||
|
||
|
||
C0IP=$(container_ip $HOST1 c0) | ||
C1IP=$(container_ip $HOST1 c1) | ||
C2IP=$(container_ip $HOST1 c2) | ||
|
||
assert_raises "[ "10.32.1.30" == $C0IP ]" | ||
assert_raises "[ "10.32.1.40" == $C1IP ]" | ||
assert_raises "[ "10.32.1.42" == $C2IP ]" | ||
|
||
BRIP=$(container_ip $HOST1 weave:expose) | ||
# Check the bridge IP is different from the container IPs | ||
assert_raises "[ $BRIP != $C0IP ]" | ||
assert_raises "[ $BRIP != $C1IP ]" | ||
assert_raises "[ $BRIP != $C2IP ]" | ||
|
||
# Containers should be able to reach one another | ||
assert_raises "exec_on $HOST1 c0 $PING $C1IP" | ||
assert_raises "exec_on $HOST1 c1 $PING $C2IP" | ||
assert_raises "exec_on $HOST1 c2 $PING $C1IP" | ||
|
||
# Containers should not have a default route to the world | ||
assert_raises "exec_on $HOST1 c0 sh -c '! $PING 8.8.8.8'" | ||
assert_raises "exec_on $HOST1 c1 sh -c '! $PING 8.8.8.8'" | ||
assert_raises "exec_on $HOST1 c2 sh -c '! $PING 8.8.8.8'" | ||
|
||
# Ensure existing containers can reclaim their IP addresses after CNI has been used -- see #2548 | ||
stop_weave_on $HOST1 | ||
|
||
# Ensure no warning is printed to the standard error: | ||
ACTUAL_OUTPUT=$(CHECKPOINT_DISABLE="$CHECKPOINT_DISABLE" DOCKER_HOST=tcp://$HOST1:$DOCKER_PORT $WEAVE launch 2>&1) | ||
EXPECTED_OUTPUT=$($SSH $HOST1 docker inspect --format="{{.Id}}" weave) | ||
|
||
assert_raises "[ $EXPECTED_OUTPUT == $ACTUAL_OUTPUT ]" | ||
|
||
assert "$SSH $HOST1 \"curl -s -X GET 127.0.0.1:6784/ip/$C1 | grep -o -E '[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}'\"" "$C1IP" | ||
assert "$SSH $HOST1 \"curl -s -X GET 127.0.0.1:6784/ip/$C2 | grep -o -E '[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}'\"" "$C2IP" | ||
assert "$SSH $HOST1 \"curl -s -X GET 127.0.0.1:6784/ip/$C3 | grep -o -E '[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}'\"" "$C3IP" | ||
|
||
|
||
end_suite |
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.
@bboreham Is
weave:expose
even a valid identifier for this function? AFAICT this is supposed to be a container ID, so it seems like there is a bug whereweave:expose
calls this function?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
weave expose
command allocates an address for the local machine on the Weave network.Rather than have a whole different path for that, we pretend we are allocating an address for a container with ID
weave:expose
.