Merge pull request #273 from zreigz/reconfigure-bridge-ip

Reconfigure bridge IP address

Documentation/bridge.md

@@ -18,6 +18,7 @@ If the bridge is missing, the plugin will create one on first use and, if gatewa
 	"type": "bridge",
 	"bridge": "mynet0",
 	"isDefaultGateway": true,
+	"forceAddress": false,
 	"ipMasq": true,
 	"hairpinMode": true,
 	"ipam": {
@@ -34,6 +35,7 @@ If the bridge is missing, the plugin will create one on first use and, if gatewa
 * `bridge` (string, optional): name of the bridge to use/create. Defaults to "cni0".
 * `isGateway` (boolean, optional): assign an IP address to the bridge. Defaults to false.
 * `isDefaultGateway` (boolean, optional): Sets isGateway to true and makes the assigned IP the default route. Defaults to false.
+* `forceAddress` (boolean, optional): Indicates if a new IP address should be set if the previous value has been changed. Defaults to false.
 * `ipMasq` (boolean, optional): set up IP Masquerade on the host for traffic originating from this network and destined outside of it. Defaults to false.
 * `mtu` (integer, optional): explicitly set MTU to the specified value. Defaults to the value chosen by the kernel.
 * `hairpinMode` (boolean, optional): set hairpin mode for interfaces on the bridge. Defaults to false.

libcni/conf.go

@@ -83,3 +83,28 @@ func LoadConf(dir, name string) (*NetworkConfig, error) {
 	}
 	return nil, fmt.Errorf(`no net configuration with name "%s" in %s`, name, dir)
 }
+
+func InjectConf(original *NetworkConfig, key string, newValue interface{}) (*NetworkConfig, error) {
+	config := make(map[string]interface{})
+	err := json.Unmarshal(original.Bytes, &config)
+	if err != nil {
+		return nil, fmt.Errorf("unmarshal existing network bytes: %s", err)
+	}
+
+	if key == "" {
+		return nil, fmt.Errorf("key value can not be empty")
+	}
+
+	if newValue == nil {
+		return nil, fmt.Errorf("newValue must be specified")
+	}
+
+	config[key] = newValue
+
+	newBytes, err := json.Marshal(config)
+	if err != nil {
+		return nil, err
+	}
+
+	return ConfFromBytes(newBytes)
+}

libcni/conf_test.go

@@ -27,8 +27,9 @@ import (
 
 var _ = Describe("Loading configuration from disk", func() {
 	var (
-		configDir    string
+		configDir     string
-		pluginConfig []byte
+		pluginConfig  []byte
+		testNetConfig *libcni.NetworkConfig
 	)
 
 	BeforeEach(func() {
@@ -38,6 +39,9 @@ var _ = Describe("Loading configuration from disk", func() {
 
 		pluginConfig = []byte(`{ "name": "some-plugin", "some-key": "some-value" }`)
 		Expect(ioutil.WriteFile(filepath.Join(configDir, "50-whatever.conf"), pluginConfig, 0600)).To(Succeed())
+
+		testNetConfig = &libcni.NetworkConfig{Network: &types.NetConf{Name: "some-plugin"},
+			Bytes: []byte(`{ "name": "some-plugin" }`)}
 	})
 
 	AfterEach(func() {
@@ -107,4 +111,89 @@ var _ = Describe("Loading configuration from disk", func() {
 			})
 		})
 	})
+
+	Describe("InjectConf", func() {
+		Context("when function parameters are incorrect", func() {
+			It("returns unmarshal error", func() {
+				conf := &libcni.NetworkConfig{Network: &types.NetConf{Name: "some-plugin"},
+					Bytes: []byte(`{ cc cc cc}`)}
+
+				_, err := libcni.InjectConf(conf, "", nil)
+				Expect(err).To(MatchError(HavePrefix(`unmarshal existing network bytes`)))
+			})
+
+			It("returns key  error", func() {
+				_, err := libcni.InjectConf(testNetConfig, "", nil)
+				Expect(err).To(MatchError(HavePrefix(`key value can not be empty`)))
+			})
+
+			It("returns newValue  error", func() {
+				_, err := libcni.InjectConf(testNetConfig, "test", nil)
+				Expect(err).To(MatchError(HavePrefix(`newValue must be specified`)))
+			})
+		})
+
+		Context("when new string value added", func() {
+			It("adds the new key & value to the config", func() {
+				newPluginConfig := []byte(`{"name":"some-plugin","test":"test"}`)
+
+				resultConfig, err := libcni.InjectConf(testNetConfig, "test", "test")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(resultConfig).To(Equal(&libcni.NetworkConfig{
+					Network: &types.NetConf{Name: "some-plugin"},
+					Bytes:   newPluginConfig,
+				}))
+			})
+
+			It("adds the new value for exiting key", func() {
+				newPluginConfig := []byte(`{"name":"some-plugin","test":"changedValue"}`)
+
+				resultConfig, err := libcni.InjectConf(testNetConfig, "test", "test")
+				Expect(err).NotTo(HaveOccurred())
+
+				resultConfig, err = libcni.InjectConf(resultConfig, "test", "changedValue")
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(resultConfig).To(Equal(&libcni.NetworkConfig{
+					Network: &types.NetConf{Name: "some-plugin"},
+					Bytes:   newPluginConfig,
+				}))
+			})
+
+			It("adds existing key & value", func() {
+				newPluginConfig := []byte(`{"name":"some-plugin","test":"test"}`)
+
+				resultConfig, err := libcni.InjectConf(testNetConfig, "test", "test")
+				Expect(err).NotTo(HaveOccurred())
+
+				resultConfig, err = libcni.InjectConf(resultConfig, "test", "test")
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(resultConfig).To(Equal(&libcni.NetworkConfig{
+					Network: &types.NetConf{Name: "some-plugin"},
+					Bytes:   newPluginConfig,
+				}))
+			})
+
+			It("adds sub-fields of NetworkConfig.Network to the config", func() {
+
+				expectedPluginConfig := []byte(`{"dns":{"domain":"local","nameservers":["server1","server2"]},"name":"some-plugin","type":"bridge"}`)
+				servers := []string{"server1", "server2"}
+				newDNS := &types.DNS{Nameservers: servers, Domain: "local"}
+
+				// inject DNS
+				resultConfig, err := libcni.InjectConf(testNetConfig, "dns", newDNS)
+				Expect(err).NotTo(HaveOccurred())
+
+				// inject type
+				resultConfig, err = libcni.InjectConf(resultConfig, "type", "bridge")
+				Expect(err).NotTo(HaveOccurred())
+
+				Expect(resultConfig).To(Equal(&libcni.NetworkConfig{
+					Network: &types.NetConf{Name: "some-plugin", Type: "bridge", DNS: types.DNS{Nameservers: servers, Domain: "local"}},
+					Bytes:   expectedPluginConfig,
+				}))
+			})
+		})
+	})
 })

plugins/main/bridge/bridge.go

@@ -35,12 +35,13 @@ const defaultBrName = "cni0"
 
 type NetConf struct {
 	types.NetConf
-	BrName      string `json:"bridge"`
+	BrName       string `json:"bridge"`
-	IsGW        bool   `json:"isGateway"`
+	IsGW         bool   `json:"isGateway"`
-	IsDefaultGW bool   `json:"isDefaultGateway"`
+	IsDefaultGW  bool   `json:"isDefaultGateway"`
-	IPMasq      bool   `json:"ipMasq"`
+	ForceAddress bool   `json:"forceAddress"`
-	MTU         int    `json:"mtu"`
+	IPMasq       bool   `json:"ipMasq"`
-	HairpinMode bool   `json:"hairpinMode"`
+	MTU          int    `json:"mtu"`
+	HairpinMode  bool   `json:"hairpinMode"`
 }
 
 func init() {
@@ -60,7 +61,7 @@ func loadNetConf(bytes []byte) (*NetConf, error) {
 	return n, nil
 }
 
-func ensureBridgeAddr(br *netlink.Bridge, ipn *net.IPNet) error {
+func ensureBridgeAddr(br *netlink.Bridge, ipn *net.IPNet, forceAddress bool) error {
 	addrs, err := netlink.AddrList(br, syscall.AF_INET)
 	if err != nil && err != syscall.ENOENT {
 		return fmt.Errorf("could not get list of IP addresses: %v", err)
@@ -74,8 +75,16 @@ func ensureBridgeAddr(br *netlink.Bridge, ipn *net.IPNet) error {
 			if a.IPNet.String() == ipnStr {
 				return nil
 			}
+
+			// If forceAddress is set to true then reconfigure IP address otherwise throw error
+			if forceAddress {
+				if err = deleteBridgeAddr(br, a.IPNet); err != nil {
+					return err
+				}
+			} else {
+				return fmt.Errorf("%q already has an IP address different from %v", br.Name, ipn.String())
+			}
 		}
-		return fmt.Errorf("%q already has an IP address different from %v", br.Name, ipn.String())
 	}
 
 	addr := &netlink.Addr{IPNet: ipn, Label: ""}
@@ -85,6 +94,24 @@ func ensureBridgeAddr(br *netlink.Bridge, ipn *net.IPNet) error {
 	return nil
 }
 
+func deleteBridgeAddr(br *netlink.Bridge, ipn *net.IPNet) error {
+	addr := &netlink.Addr{IPNet: ipn, Label: ""}
+
+	if err := netlink.LinkSetDown(br); err != nil {
+		return fmt.Errorf("could not set down bridge %q: %v", br.Name, err)
+	}
+
+	if err := netlink.AddrDel(br, addr); err != nil {
+		return fmt.Errorf("could not remove IP address from %q: %v", br.Name, err)
+	}
+
+	if err := netlink.LinkSetUp(br); err != nil {
+		return fmt.Errorf("could not set up bridge %q: %v", br.Name, err)
+	}
+
+	return nil
+}
+
 func bridgeByName(name string) (*netlink.Bridge, error) {
 	l, err := netlink.LinkByName(name)
 	if err != nil {
@@ -266,7 +293,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 			Mask: result.IP4.IP.Mask,
 		}
 
-		if err = ensureBridgeAddr(br, gwn); err != nil {
+		if err = ensureBridgeAddr(br, gwn, n.ForceAddress); err != nil {
 			return err
 		}
 

plugins/main/bridge/bridge_test.go

@@ -251,4 +251,74 @@ var _ = Describe("bridge Operations", func() {
 		})
 		Expect(err).NotTo(HaveOccurred())
 	})
+
+	It("ensure bridge address", func() {
+
+		const IFNAME = "bridge0"
+		const EXPECTED_IP = "10.0.0.0/8"
+		const CHANGED_EXPECTED_IP = "10.1.2.3/16"
+
+		conf := &NetConf{
+			NetConf: types.NetConf{
+				Name: "testConfig",
+				Type: "bridge",
+			},
+			BrName: IFNAME,
+			IsGW:   true,
+			IPMasq: false,
+			MTU:    5000,
+		}
+
+		gwnFirst := &net.IPNet{
+			IP:   net.IPv4(10, 0, 0, 0),
+			Mask: net.CIDRMask(8, 32),
+		}
+
+		gwnSecond := &net.IPNet{
+			IP:   net.IPv4(10, 1, 2, 3),
+			Mask: net.CIDRMask(16, 32),
+		}
+
+		err := originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			bridge, err := setupBridge(conf)
+			Expect(err).NotTo(HaveOccurred())
+			// Check if ForceAddress has default value
+			Expect(conf.ForceAddress).To(Equal(false))
+
+			err = ensureBridgeAddr(bridge, gwnFirst, conf.ForceAddress)
+			Expect(err).NotTo(HaveOccurred())
+
+			//Check if IP address is set correctly
+			addrs, err := netlink.AddrList(bridge, syscall.AF_INET)
+			Expect(len(addrs)).To(Equal(1))
+			addr := addrs[0].IPNet.String()
+			Expect(addr).To(Equal(EXPECTED_IP))
+
+			//The bridge IP address has been changed. Error expected when ForceAddress is set to false.
+			err = ensureBridgeAddr(bridge, gwnSecond, false)
+			Expect(err).To(HaveOccurred())
+
+			//The IP address should stay the same.
+			addrs, err = netlink.AddrList(bridge, syscall.AF_INET)
+			Expect(len(addrs)).To(Equal(1))
+			addr = addrs[0].IPNet.String()
+			Expect(addr).To(Equal(EXPECTED_IP))
+
+			//Reconfigure IP when ForceAddress is set to true and IP address has been changed.
+			err = ensureBridgeAddr(bridge, gwnSecond, true)
+			Expect(err).NotTo(HaveOccurred())
+
+			//Retrieve the IP address after reconfiguration
+			addrs, err = netlink.AddrList(bridge, syscall.AF_INET)
+			Expect(len(addrs)).To(Equal(1))
+			addr = addrs[0].IPNet.String()
+			Expect(addr).To(Equal(CHANGED_EXPECTED_IP))
+
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
 })