From d09b18dac4f5ebf8e64e87c9ff18863cd7409a88 Mon Sep 17 00:00:00 2001 From: Lukasz Zajaczkowski Date: Fri, 29 Jul 2016 13:02:32 +0200 Subject: [PATCH 1/3] plugins: reconfigure bridge IP address Add possibility to reconfigure bridge IP address when there is a new value. New boolean flag added to net configuration to force IP change if it is need. Otherwise code behaves as previously and throws error --- plugins/main/bridge/bridge.go | 45 +++++++++++++++---- plugins/main/bridge/bridge_test.go | 70 ++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 9 deletions(-) diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index d4fc89c3..7309c403 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -35,12 +35,13 @@ const defaultBrName = "cni0" type NetConf struct { types.NetConf - BrName string `json:"bridge"` - IsGW bool `json:"isGateway"` - IsDefaultGW bool `json:"isDefaultGateway"` - IPMasq bool `json:"ipMasq"` - MTU int `json:"mtu"` - HairpinMode bool `json:"hairpinMode"` + BrName string `json:"bridge"` + IsGW bool `json:"isGateway"` + IsDefaultGW bool `json:"isDefaultGateway"` + ForceAddress bool `json:"forceAddress"` + IPMasq bool `json:"ipMasq"` + 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 { @@ -258,7 +285,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 } diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index 14fef56b..6fa45b3e 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/plugins/main/bridge/bridge_test.go @@ -236,4 +236,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()) + }) + }) From ee64ac74bcbd3a8597b5c89db4693218a2672c3d Mon Sep 17 00:00:00 2001 From: Lukasz Zajaczkowski Date: Mon, 1 Aug 2016 12:53:46 +0200 Subject: [PATCH 2/3] documentation: add description for forceAddress parameter --- Documentation/bridge.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/bridge.md b/Documentation/bridge.md index dcf52b70..3abe99fd 100644 --- a/Documentation/bridge.md +++ b/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. From 8cb3a9323a6643b6353c4d529e676ab93651acbd Mon Sep 17 00:00:00 2001 From: Lukasz Zajaczkowski Date: Fri, 5 Aug 2016 10:45:58 +0200 Subject: [PATCH 3/3] libcni: add util function InjectConf --- libcni/conf.go | 25 ++++++++++++ libcni/conf_test.go | 93 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/libcni/conf.go b/libcni/conf.go index 029769e4..984b1592 100644 --- a/libcni/conf.go +++ b/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) +} diff --git a/libcni/conf_test.go b/libcni/conf_test.go index 9460aca4..acbbfc04 100644 --- a/libcni/conf_test.go +++ b/libcni/conf_test.go @@ -27,8 +27,9 @@ import ( var _ = Describe("Loading configuration from disk", func() { var ( - configDir string - pluginConfig []byte + configDir string + 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, + })) + }) + }) + }) })