Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Init global settings from object store #1043

Merged
merged 1 commit into from
Nov 9, 2017
+199 −138
Merged

Init global settings from object store #1043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion netmaster/objApi/apiController.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed, variables are initialized by default

Copy link
Contributor Author

@tiewei tiewei Nov 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would leave as empty string here and in next commit to make it accept from CLI, and I think defined explicitly is always better

ArpMode: "proxy",
PvtSubnet: defHostPvtNet,
})
Expand Down
60 changes: 43 additions & 17 deletions netplugin/plugin/netplugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -50,8 +52,6 @@ type NetPlugin struct {
PluginConfig Config
}

const defaultPvtSubnet = 0xac130000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you are removing the default svc net,
can you add it in netmaster/netctl so that it uses this svc subnet by default ?

Copy link
Contributor Author

@tiewei tiewei Nov 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it does, i'll add more stuff on the netmaster side after this PR, it currently still having default value to init global settings as bridge instead of wait until it set

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


// Init initializes the NetPlugin instance via the configuration string passed.
func (p *NetPlugin) Init(pluginConfig Config) error {
var err error
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log error?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if global setting != instant setting, are we going to do anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will be handled here

	// Init the driver plugins..
	err = netPlugin.Init(*pluginConfig)
	if err != nil {
		log.Fatalf("Failed to initialize the plugin. Error: %s", err)
	}

which will exit(1) after log fatal

}

// initialize network driver
p.NetworkDriver, err = utils.NewNetworkDriver(pluginConfig.Drivers.Network, &pluginConfig.Instance)
Expand Down Expand Up @@ -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
Expand Down
189 changes: 119 additions & 70 deletions netplugin/plugin/netplugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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{}
Expand All @@ -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{}
Expand All @@ -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" : {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is due to tabs before the chars, if we think spaces are better, i can switch it

"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{}
Expand All @@ -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{}
Expand All @@ -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{}
Expand All @@ -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{}
Expand All @@ -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{}
Expand Down
Loading