Init global settings from object store

Currently netplugin is trying to init from default settings if global
setting is not ready or wrong, and when the settings don't match it
will re-init netplugin. This is very easy to go wrong when there are
resources created before global settings are ready.
This commit makes netplugin wait until the global settings ready, and
initiate itself according to it. If the global settings are mis-match,
it will return error.
Also set default fwd mode to empty on netmaster and Refactor CIDRToMask

Signed-off-by: Wei Tie <[email protected]>

Author: Wei Tie

netmaster/objApi/apiController.go

@@ -116,7 +116,7 @@ func NewAPIController(router *mux.Router, objdbClient objdb.API, storeURL string
 			NetworkInfraType: "default",
 			Vlans:            "1-4094",
 			Vxlans:           "1-10000",
-			FwdMode:          "bridge",
+			FwdMode:          "", // set empty fwd mode by default
 			ArpMode:          "proxy",
 			PvtSubnet:        defHostPvtNet,
 		})

netplugin/plugin/netplugin.go

@@ -16,12 +16,14 @@ limitations under the License.
 package plugin
 
 import (
+	"fmt"
 	"github.com/Sirupsen/logrus"
 	"github.com/contiv/netplugin/core"
 	"github.com/contiv/netplugin/netmaster/mastercfg"
 	"github.com/contiv/netplugin/utils"
 	"github.com/contiv/netplugin/utils/netutils"
 	"sync"
+	"time"
 )
 
 // implements the generic Plugin interface
@@ -50,8 +52,6 @@ type NetPlugin struct {
 	PluginConfig  Config
 }
 
-const defaultPvtSubnet = 0xac130000
-
 // Init initializes the NetPlugin instance via the configuration string passed.
 func (p *NetPlugin) Init(pluginConfig Config) error {
 	var err error
@@ -75,8 +75,10 @@ func (p *NetPlugin) Init(pluginConfig Config) error {
 
 	// set state driver in instance info
 	pluginConfig.Instance.StateDriver = p.StateDriver
-
-	InitGlobalSettings(p.StateDriver, &pluginConfig.Instance)
+	err = InitGlobalSettings(p.StateDriver, &pluginConfig.Instance)
+	if err != nil {
+		return err
+	}
 
 	// initialize network driver
 	p.NetworkDriver, err = utils.NewNetworkDriver(pluginConfig.Drivers.Network, &pluginConfig.Instance)
@@ -303,27 +305,51 @@ func (p *NetPlugin) Reinit(cfg Config) {
 }
 
 //InitGlobalSettings initializes cluster-wide settings (e.g. fwd-mode)
-func InitGlobalSettings(stateDriver core.StateDriver, inst *core.InstanceInfo) {
+func InitGlobalSettings(stateDriver core.StateDriver, inst *core.InstanceInfo) error {
+
+	/*
+		Query global settings from state store
+		1. if forward mode or private net is empty, retry after 1 seccond
+		2. if forward mode doesn't match, return error
+		3. if stored private net is wrong, return error
+	*/
 
 	gCfg := mastercfg.GlobConfig{}
 	gCfg.StateDriver = stateDriver
-	err := gCfg.Read("")
-	if err != nil {
-		logrus.Errorf("Error reading forwarding mode from cluster store")
-		inst.FwdMode = "bridge"
-		inst.HostPvtNW = defaultPvtSubnet
-		return
-	}
 
+	// wait until able to get fwd mode and private subnet
+	for {
+		if err := gCfg.Read(""); err != nil {
+			logrus.Errorf("Error reading global settings from cluster store, error: %v", err.Error())
+		} else {
+			if gCfg.FwdMode == "" || gCfg.PvtSubnet == "" {
+				if gCfg.FwdMode == "" {
+					logrus.Warnf("No forwarding mode found from cluster store")
+				}
+				if gCfg.PvtSubnet == "" {
+					logrus.Warnf("No private subnet found from cluster store")
+				}
+
+			} else {
+				logrus.Infof("Got global forwarding mode: %v", gCfg.FwdMode)
+				logrus.Infof("Got global private subnet: %v", gCfg.PvtSubnet)
+				break
+			}
+		}
+		logrus.Warnf("Sleep 1 second and retry pulling global settings")
+		time.Sleep(1 * time.Second)
+	}
 	inst.FwdMode = gCfg.FwdMode
+	logrus.Infof("Using forwarding mode: %v", inst.FwdMode)
 	net, err := netutils.CIDRToMask(gCfg.PvtSubnet)
 	if err != nil {
-		logrus.Errorf("%s %v, will use default", gCfg.PvtSubnet, err)
-		inst.HostPvtNW = defaultPvtSubnet
-	} else {
-		inst.HostPvtNW = net
-		logrus.Infof("HostPvtNW: %v", net)
+		err := fmt.Errorf("Error convert private subnet %v from CIDR to mask, error %v", gCfg.PvtSubnet, err.Error())
+		logrus.Errorf(err.Error())
+		return err
 	}
+	inst.HostPvtNW = net
+	logrus.Infof("Using host private subnet: %v", gCfg.PvtSubnet)
+	return nil
 }
 
 //AddSvcSpec adds k8 service spec

netplugin/plugin/netplugin_test.go

@@ -18,25 +18,52 @@ package plugin
 import (
 	"encoding/json"
 	"fmt"
-	"testing"
-
+	"github.com/contiv/netplugin/core"
+	"github.com/contiv/netplugin/netmaster/mastercfg"
 	"github.com/contiv/netplugin/state"
+	"github.com/contiv/netplugin/utils"
+	"testing"
 )
 
 var fakeStateDriver *state.FakeStateDriver
 
+func initFakeStateDriver(t *testing.T) {
+	// init fake state driver
+	instInfo := core.InstanceInfo{}
+	d, err := utils.NewStateDriver("fakedriver", &instInfo)
+	if err != nil {
+		t.Fatalf("failed to init statedriver. Error: %s", err)
+	}
+
+	fakeStateDriver = d.(*state.FakeStateDriver)
+}
+
+func deinitFakeStateDriver() {
+	// release fake state driver
+	utils.ReleaseStateDriver()
+}
+
 func TestNetPluginInit(t *testing.T) {
+	// Testing init NetPlugin
+	initFakeStateDriver(t)
+	defer deinitFakeStateDriver()
+	gCfg := mastercfg.GlobConfig{
+		FwdMode:   "bridge",
+		PvtSubnet: "172.19.0.0/16"}
+	gCfg.StateDriver = fakeStateDriver
+	gCfg.Write()
+
 	configStr := `{
-                    "drivers" : {
-                       "network": "ovs",
-                       "endpoint": "ovs",
-                       "state": "fakedriver"
-                    },
-                    "plugin-instance": {
-                       "host-label": "testHost",
-		       		   "fwd-mode":"bridge"
-                    }
-                  }`
+					"drivers" : {
+						"network": "ovs",
+						"endpoint": "ovs",
+						"state": "fakedriver"
+					},
+					"plugin-instance": {
+						"host-label": "testHost",
+						"fwd-mode":"bridge"
+					}
+				}`
 
 	// Parse the config
 	pluginConfig := Config{}
@@ -56,6 +83,7 @@ func TestNetPluginInit(t *testing.T) {
 }
 
 func TestNetPluginInitInvalidConfigEmptyString(t *testing.T) {
+	// Test NetPlugin init failure when no config provided
 	pluginConfig := Config{}
 
 	plugin := NetPlugin{}
@@ -66,13 +94,14 @@ func TestNetPluginInitInvalidConfigEmptyString(t *testing.T) {
 }
 
 func TestNetPluginInitInvalidConfigMissingInstance(t *testing.T) {
+	// Test NetPlugin init failure when missing instance config
 	configStr := `{
-                    "drivers" : {
-                       "network": "ovs",
-                       "endpoint": "ovs",
-                       "state": "fakedriver"
-                    }
-                  }`
+					"drivers" : {
+						"network": "ovs",
+						"endpoint": "ovs",
+						"state": "fakedriver"
+					}
+				}`
 
 	// Parse the config
 	pluginConfig := Config{}
@@ -86,18 +115,19 @@ func TestNetPluginInitInvalidConfigMissingInstance(t *testing.T) {
 }
 
 func TestNetPluginInitInvalidConfigEmptyHostLabel(t *testing.T) {
+	// Test NetPlugin init failure when empty HostLabel provided
 	configStr := `{
-                    "drivers" : {
-                       "network": "ovs",
-                       "endpoint": "ovs",
-                       "state": "fakedriver"
-                    },
-                    "plugin-instance": {
-                       "host-label": "",
-					   "fwd-mode":"bridge",
-					   "db-url": "etcd://127.0.0.1:4001"
-                    }
-                  }`
+					"drivers" : {
+						"network": "ovs",
+						"endpoint": "ovs",
+						"state": "fakedriver"
+					},
+					"plugin-instance": {
+						"host-label": "",
+						"fwd-mode":"bridge",
+						"db-url": "etcd://127.0.0.1:4001"
+					}
+				}`
 
 	// Parse the config
 	pluginConfig := Config{}
@@ -114,17 +144,18 @@ func TestNetPluginInitInvalidConfigEmptyHostLabel(t *testing.T) {
 }
 
 func TestNetPluginInitInvalidConfigMissingStateDriverName(t *testing.T) {
+	// Test NetPlugin init failure when missing state driver name
 	configStr := `{
-                    "drivers" : {
-                       "network": "ovs",
-                       "endpoint": "ovs"
-                    },
-                    "plugin-instance": {
-                       "host-label": "testHost",
-		       		   "fwd-mode":"bridge",
-					   "db-url": "etcd://127.0.0.1:4001"
-                    }
-                  }`
+					"drivers" : {
+						"network": "ovs",
+						"endpoint": "ovs"
+					},
+					"plugin-instance": {
+						"host-label": "testHost",
+						"fwd-mode":"bridge",
+						"db-url": "etcd://127.0.0.1:4001"
+					}
+				}`
 
 	// Parse the config
 	pluginConfig := Config{}
@@ -141,17 +172,18 @@ func TestNetPluginInitInvalidConfigMissingStateDriverName(t *testing.T) {
 }
 
 func TestNetPluginInitInvalidConfigMissingStateDriverURL(t *testing.T) {
+	// Test NetPlugin init failure when missing state driver url
 	configStr := `{
-                    "drivers" : {
-                       "network": "ovs",
-                       "endpoint": "ovs",
-                       "state": "etcd"
-                    },
-                    "plugin-instance": {
-                       "host-label": "testHost",
-           	           "fwd-mode":"bridge"
-                    }
-                  }`
+					"drivers" : {
+						"network": "ovs",
+						"endpoint": "ovs",
+						"state": "etcd"
+					},
+					"plugin-instance": {
+						"host-label": "testHost",
+						"fwd-mode":"bridge"
+					}
+				}`
 
 	// Parse the config
 	pluginConfig := Config{}
@@ -169,18 +201,26 @@ func TestNetPluginInitInvalidConfigMissingStateDriverURL(t *testing.T) {
 }
 
 func TestNetPluginInitInvalidConfigMissingNetworkDriverName(t *testing.T) {
+	// Test NetPlugin init failure when missing network driver name
+	initFakeStateDriver(t)
+	defer deinitFakeStateDriver()
+	gCfg := mastercfg.GlobConfig{
+		FwdMode:   "bridge",
+		PvtSubnet: "172.19.0.0/16"}
+	gCfg.StateDriver = fakeStateDriver
+	gCfg.Write()
 	configStr := `{
-                    "drivers" : {
-                       "endpoint": "ovs",
-                       "state": "fakedriver",
-                       "container": "docker"
-                    },
-                    "plugin-instance": {
-                       "host-label": "testHost",
-		       		   "fwd-mode":"bridge",
-					   "db-url": "etcd://127.0.0.1:4001"
-                    }
-                  }`
+					"drivers" : {
+						"endpoint": "ovs",
+						"state": "fakedriver",
+						"container": "docker"
+					},
+					"plugin-instance": {
+						"host-label": "testHost",
+						"fwd-mode":"bridge",
+						"db-url": "etcd://127.0.0.1:4001"
+					}
+				}`
 
 	// Parse the config
 	pluginConfig := Config{}
@@ -196,19 +236,28 @@ func TestNetPluginInitInvalidConfigMissingNetworkDriverName(t *testing.T) {
 	}
 }
 
-func TestNetPluginInitInvalidConfigMissingFwdMode(t *testing.T) {
+func TestNetPluginInitInvalidConfigInvalidPrivateSubnet(t *testing.T) {
+	// Test NetPlugin init failure when private subnet is not valid
+	initFakeStateDriver(t)
+	defer deinitFakeStateDriver()
+	gCfg := mastercfg.GlobConfig{
+		FwdMode:   "routing",
+		PvtSubnet: "172.19.0.0"}
+	gCfg.StateDriver = fakeStateDriver
+	gCfg.Write()
 	configStr := `{
-                    "drivers" : {
-                       "network": "ovs",
-                       "endpoint": "ovs",
-                       "state": "fakedriver",
-                       "container": "docker",
-                    },
+					"drivers" : {
+						"network": "ovs",
+						"endpoint": "ovs",
+						"state": "fakedriver",
+						"container": "docker",
+					},
 					"plugin-instance": {
-						 "host-label": "testHost",
-						 "db-url": "etcd://127.0.0.1:4001"
+						"host-label": "testHost",
+						"db-url": "etcd://127.0.0.1:4001",
+						"fwd-mode":"routing",
 					}
-                  }`
+				}`
 
 	// Parse the config
 	pluginConfig := Config{}