Skip to content

Commit 5bc4d36

Browse files
dseevrdseevr
authored
Merge pull request #1051 from chrisplo/fix_nm_output_msg
global fwd and subnet error msgs fix & test cases
2 parents 297f20b + 5e4519f commit 5bc4d36

File tree

2 files changed

+103
-17
lines changed

2 files changed

+103
-17
lines changed

‎netmaster/objApi/apiController.go

100755100644
+24-10
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ import (
2222
"strings"
2323

2424
"encoding/json"
25-
"github.com/contiv/contivmodel"
25+
"io/ioutil"
26+
"net/http"
27+
28+
contivModel "github.com/contiv/contivmodel"
2629
"github.com/contiv/netplugin/core"
2730
"github.com/contiv/netplugin/drivers"
2831
"github.com/contiv/netplugin/netmaster/docknet"
@@ -34,8 +37,6 @@ import (
3437
"github.com/contiv/netplugin/objdb/modeldb"
3538
"github.com/contiv/netplugin/utils"
3639
"github.com/contiv/netplugin/utils/netutils"
37-
"io/ioutil"
38-
"net/http"
3940

4041
log "github.com/Sirupsen/logrus"
4142
"github.com/gorilla/mux"
@@ -225,21 +226,35 @@ func (ac *APIController) GlobalUpdate(global, params *contivModel.Global) error
225226

226227
gCfg := &gstate.Cfg{}
227228
gCfg.StateDriver = stateDriver
228-
numVlans, vlansInUse := gCfg.GetVlansInUse()
229-
numVxlans, vxlansInUse := gCfg.GetVxlansInUse()
229+
numVlans, _ := gCfg.GetVlansInUse()
230+
numVxlans, _ := gCfg.GetVxlansInUse()
230231

231232
// Build global config
232233
globalCfg := intent.ConfigGlobal{}
233234

235+
// Generate helpful error message when networks exist
236+
errExistingNetworks := func(optionLabel string) error {
237+
msgs := []string{}
238+
if numVlans > 0 {
239+
msgs = append(msgs, fmt.Sprintf("%d vlans", numVlans))
240+
}
241+
if numVxlans > 0 {
242+
msgs = append(msgs, fmt.Sprintf("%d vxlans", numVxlans))
243+
}
244+
msg := fmt.Sprintf("Unable to update %s due to existing %s",
245+
optionLabel, strings.Join(msgs, " and "))
246+
log.Errorf(msg)
247+
return fmt.Errorf(msg)
248+
}
249+
234250
//check for change in forwarding mode
235251
if global.FwdMode != params.FwdMode {
236252
//check if there exists any non default network and tenants
237253
if numVlans+numVxlans > 0 {
238-
log.Errorf("Unable to update forwarding mode due to existing %d vlans and %d vxlans", numVlans, numVxlans)
239-
return fmt.Errorf("please delete %v vlans and %v vxlans before changing forwarding mode", vlansInUse, vxlansInUse)
254+
return errExistingNetworks("forwarding mode")
240255
}
241256
if global.FwdMode == "routing" {
242-
//check if any bgp configurations exists.
257+
//check if any bgp configurations exists.
243258
bgpCfgs := &mastercfg.CfgBgpState{}
244259
bgpCfgs.StateDriver = stateDriver
245260
cfgs, _ := bgpCfgs.ReadAll()
@@ -264,8 +279,7 @@ func (ac *APIController) GlobalUpdate(global, params *contivModel.Global) error
264279
}
265280
if global.PvtSubnet != params.PvtSubnet {
266281
if (global.PvtSubnet != "" || params.PvtSubnet != defHostPvtNet) && numVlans+numVxlans > 0 {
267-
log.Errorf("Unable to update provate subnet due to existing networks")
268-
return fmt.Errorf("please delete %v vlans and %v vxlans before changing private subnet", vlansInUse, vxlansInUse)
282+
return errExistingNetworks("private subnet")
269283
}
270284
globalCfg.PvtSubnet = params.PvtSubnet
271285
}

‎netmaster/objApi/objapi_test.go

100755100644
+79-7
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"testing"
2525
"time"
2626

27-
"github.com/contiv/contivmodel"
27+
contivModel "github.com/contiv/contivmodel"
2828
"github.com/contiv/contivmodel/client"
2929
"github.com/contiv/netplugin/core"
3030
"github.com/contiv/netplugin/netmaster/gstate"
@@ -297,7 +297,10 @@ func checkInspectGlobal(t *testing.T, expError bool, allocedVlans, allocedVxlans
297297
}
298298

299299
// checkGlobalSet sets global state and verifies state
300-
func checkGlobalSet(t *testing.T, expError bool, fabMode, vlans, vxlans, fwdMode, arpMode, pvtSubnet string) {
300+
// the return error can be used for validating the error produced
301+
// by contivClient.GlobalPost, a non-nil return is not an error, 'expError'
302+
// parameter determines if an err for GlobalPost is a failure for the test
303+
func checkGlobalSet(t *testing.T, expError bool, fabMode, vlans, vxlans, fwdMode, arpMode, pvtSubnet string) error {
301304
gl := client.Global{
302305
Name: "global",
303306
NetworkInfraType: fabMode,
@@ -358,7 +361,9 @@ func checkGlobalSet(t *testing.T, expError bool, fabMode, vlans, vxlans, fwdMode
358361
if err := vlanRsrc.Read("global"); err != nil {
359362
t.Fatalf("Error reading vlan resource. Err: %v", err)
360363
}
364+
return nil
361365
}
366+
return err
362367
}
363368

364369
// checkAciGwSet sets AciGw state and verifies verifies it
@@ -1325,8 +1330,80 @@ func TestDynamicGlobalVxlanRange(t *testing.T) {
13251330

13261331
}
13271332

1333+
// TestGlobalSetting tests global REST api
1334+
func TestGlobalSettingFwdMode(t *testing.T) {
1335+
// set to default values (no-op)
1336+
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
1337+
1338+
// Create a vxlan network to verify you cannot change forward mode
1339+
checkCreateNetwork(t, false, "default", "contiv7", "", "vxlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")
1340+
1341+
// Should fail when changing the forwarding mode whenever there is a network
1342+
var err error
1343+
var expErr string
1344+
err = checkGlobalSet(t, true, "default", "1-4094", "1-10000", "routing", "proxy", "172.19.0.0/16")
1345+
expErr = "Unable to update forwarding mode due to existing 1 vxlans"
1346+
if strings.TrimSpace(err.Error()) != expErr {
1347+
t.Fatalf("Wrong error message, expected: '%v', got '%v'", expErr, err.Error())
1348+
}
1349+
// remove the vxlan network and add a vlan network
1350+
checkDeleteNetwork(t, false, "default", "contiv7")
1351+
checkCreateNetwork(t, false, "default", "contiv7", "", "vlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")
1352+
err = checkGlobalSet(t, true, "default", "1-4094", "1-10000", "routing", "proxy", "172.19.0.0/16")
1353+
expErr = "Unable to update forwarding mode due to existing 1 vlans"
1354+
if strings.TrimSpace(err.Error()) != expErr {
1355+
t.Fatalf("Wrong error message, expected: '%v', got '%v'", expErr, err.Error())
1356+
}
1357+
1358+
// remove the vlan network
1359+
checkDeleteNetwork(t, false, "default", "contiv7")
1360+
1361+
// make sure can change forwarding mode after network deleted
1362+
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "routing", "proxy", "172.21.0.0/16")
1363+
1364+
// reset back to default values
1365+
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
1366+
}
1367+
1368+
func TestGlobalSettingSubnet(t *testing.T) {
1369+
// set to default values (no-op)
1370+
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
1371+
// Create a network to see if global subnet changes are blocked
1372+
checkCreateNetwork(t, false, "default", "contiv7", "", "vxlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")
1373+
1374+
var err error
1375+
var expErr string
1376+
// This should fail
1377+
err = checkGlobalSet(t, true, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/16")
1378+
expErr = "Unable to update private subnet due to existing 1 vxlans"
1379+
if strings.TrimSpace(err.Error()) != expErr {
1380+
t.Fatalf("Wrong error message, expected: '%v', got '%v'", expErr, err.Error())
1381+
}
1382+
1383+
// remove the vxlan network and add a vlan network
1384+
checkDeleteNetwork(t, false, "default", "contiv7")
1385+
checkCreateNetwork(t, false, "default", "contiv7", "", "vlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")
1386+
1387+
// This should still fail
1388+
err = checkGlobalSet(t, true, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/16")
1389+
expErr = "Unable to update private subnet due to existing 1 vlans"
1390+
if strings.TrimSpace(err.Error()) != expErr {
1391+
t.Fatalf("Wrong error message, expected: '%v', got '%v'", expErr, err.Error())
1392+
}
1393+
1394+
// remove the network
1395+
checkDeleteNetwork(t, false, "default", "contiv7")
1396+
// make sure can change subnet after network deleted
1397+
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/16")
1398+
// reset back to default values
1399+
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
1400+
}
1401+
13281402
// TestGlobalSetting tests global REST api
13291403
func TestGlobalSetting(t *testing.T) {
1404+
// set to default values (no-op)
1405+
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
1406+
13301407
// try basic modification
13311408
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.20.0.0/16")
13321409
// set aci mode
@@ -1350,11 +1427,6 @@ func TestGlobalSetting(t *testing.T) {
13501427
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/16")
13511428
// Try invalid pvt subnet
13521429
checkGlobalSet(t, true, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/24")
1353-
// Try changing subnet with active network
1354-
checkCreateNetwork(t, false, "default", "contiv7", "", "vxlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")
1355-
checkGlobalSet(t, true, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
1356-
checkDeleteNetwork(t, false, "default", "contiv7")
1357-
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
13581430

13591431
// reset back to default values
13601432
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")

0 commit comments

Comments
 (0)