From 48169ce736995c9b06b7c7d23841c2e0cdc19433 Mon Sep 17 00:00:00 2001 From: Minhan Xia Date: Wed, 22 Jun 2016 15:57:01 -0700 Subject: [PATCH] avoid deleting cbr0 address due to subnet string mismatch --- pkg/kubelet/network/kubenet/kubenet_linux.go | 9 +-- pkg/util/net/util.go | 36 +++++++++++ pkg/util/net/util_test.go | 68 ++++++++++++++++++++ 3 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 pkg/util/net/util.go create mode 100644 pkg/util/net/util_test.go diff --git a/pkg/kubelet/network/kubenet/kubenet_linux.go b/pkg/kubelet/network/kubenet/kubenet_linux.go index bc90b7b3d40..9df74596612 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux.go @@ -40,6 +40,7 @@ import ( utilerrors "k8s.io/kubernetes/pkg/util/errors" utilexec "k8s.io/kubernetes/pkg/util/exec" utiliptables "k8s.io/kubernetes/pkg/util/iptables" + utilnet "k8s.io/kubernetes/pkg/util/net" utilsets "k8s.io/kubernetes/pkg/util/sets" utilsysctl "k8s.io/kubernetes/pkg/util/sysctl" @@ -228,7 +229,7 @@ func (plugin *kubenetNetworkPlugin) Event(name string, details map[string]interf // Ensure cbr0 has no conflicting addresses; CNI's 'bridge' // plugin will bail out if the bridge has an unexpected one - plugin.clearBridgeAddressesExcept(cidr.IP.String()) + plugin.clearBridgeAddressesExcept(cidr) } } @@ -237,7 +238,7 @@ func (plugin *kubenetNetworkPlugin) Event(name string, details map[string]interf } } -func (plugin *kubenetNetworkPlugin) clearBridgeAddressesExcept(keep string) { +func (plugin *kubenetNetworkPlugin) clearBridgeAddressesExcept(keep *net.IPNet) { bridge, err := netlink.LinkByName(BridgeName) if err != nil { return @@ -249,8 +250,8 @@ func (plugin *kubenetNetworkPlugin) clearBridgeAddressesExcept(keep string) { } for _, addr := range addrs { - if addr.IPNet.String() != keep { - glog.V(5).Infof("Removing old address %s from %s", addr.IPNet.String(), BridgeName) + if !utilnet.IPNetEqual(addr.IPNet, keep) { + glog.V(2).Infof("Removing old address %s from %s", addr.IPNet.String(), BridgeName) netlink.AddrDel(bridge, &addr) } } diff --git a/pkg/util/net/util.go b/pkg/util/net/util.go new file mode 100644 index 00000000000..92d5d0b98d1 --- /dev/null +++ b/pkg/util/net/util.go @@ -0,0 +1,36 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package net + +import ( + "net" + "reflect" +) + +// IPNetEqual checks if the two input IPNets are representing the same subnet. +// For example, +// 10.0.0.1/24 and 10.0.0.0/24 are the same subnet. +// 10.0.0.1/24 and 10.0.0.0/25 are not the same subnet. +func IPNetEqual(ipnet1, ipnet2 *net.IPNet) bool { + if ipnet1 == nil || ipnet2 == nil { + return false + } + if reflect.DeepEqual(ipnet1.Mask, ipnet2.Mask) && ipnet1.Contains(ipnet2.IP) && ipnet2.Contains(ipnet1.IP) { + return true + } + return false +} diff --git a/pkg/util/net/util_test.go b/pkg/util/net/util_test.go new file mode 100644 index 00000000000..ead640934fe --- /dev/null +++ b/pkg/util/net/util_test.go @@ -0,0 +1,68 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package net + +import ( + "net" + "testing" +) + +func getIPNet(cidr string) *net.IPNet { + _, ipnet, _ := net.ParseCIDR(cidr) + return ipnet +} + +func TestIPNetEqual(t *testing.T) { + testCases := []struct { + ipnet1 *net.IPNet + ipnet2 *net.IPNet + expect bool + }{ + //null case + { + getIPNet("10.0.0.1/24"), + getIPNet(""), + false, + }, + { + getIPNet("10.0.0.0/24"), + getIPNet("10.0.0.0/24"), + true, + }, + { + getIPNet("10.0.0.0/24"), + getIPNet("10.0.0.1/24"), + true, + }, + { + getIPNet("10.0.0.0/25"), + getIPNet("10.0.0.0/24"), + false, + }, + { + getIPNet("10.0.1.0/24"), + getIPNet("10.0.0.0/24"), + false, + }, + } + + for _, tc := range testCases { + if tc.expect != IPNetEqual(tc.ipnet1, tc.ipnet2) { + t.Errorf("Expect equality of %s and %s be to %v", tc.ipnet1.String(), tc.ipnet2.String(), tc.expect) + } + } +}