Skip to content

Commit

Permalink
global fwd and subnet error msgs fix & test cases (#1080)
Browse files Browse the repository at this point in the history 
* global fwd and subnet error msgs fix & test cases

Fixed the error message to indicate correctly the state of vlans and
vxlans instead of printing an empty string instead of a number when 0.

The test cases are testing too many things in one test case, split out
the tests for forward mode and subnet as they are doing more validation.

Signed-off-by: Chris Plock <chrisplo@cisco.com>

* simplified

* err no longer a pointer to error
  • Loading branch information
@chrisplo @dseevr
chrisplo authored and dseevr committed Nov 21, 2017
1 parent 297f20b commit 50fd8b3
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 17 deletions.
34 changes: 24 additions & 10 deletions netmaster/objApi/apiController.go
100755 → 100644
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import (
"strings"

"encoding/json"
"github.com/contiv/contivmodel"
"io/ioutil"
"net/http"

contivModel "github.com/contiv/contivmodel"
"github.com/contiv/netplugin/core"
"github.com/contiv/netplugin/drivers"
"github.com/contiv/netplugin/netmaster/docknet"
Expand All @@ -34,8 +37,6 @@ import (
"github.com/contiv/netplugin/objdb/modeldb"
"github.com/contiv/netplugin/utils"
"github.com/contiv/netplugin/utils/netutils"
"io/ioutil"
"net/http"

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

gCfg := &gstate.Cfg{}
gCfg.StateDriver = stateDriver
numVlans, vlansInUse := gCfg.GetVlansInUse()
numVxlans, vxlansInUse := gCfg.GetVxlansInUse()
numVlans, _ := gCfg.GetVlansInUse()
numVxlans, _ := gCfg.GetVxlansInUse()

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

// Generate helpful error message when networks exist
errExistingNetworks := func(optionLabel string) error {
msgs := []string{}
if numVlans > 0 {
msgs = append(msgs, fmt.Sprintf("%d vlans", numVlans))
}
if numVxlans > 0 {
msgs = append(msgs, fmt.Sprintf("%d vxlans", numVxlans))
}
msg := fmt.Sprintf("Unable to update %s due to existing %s",
optionLabel, strings.Join(msgs, " and "))
log.Errorf(msg)
return fmt.Errorf(msg)
}

//check for change in forwarding mode
if global.FwdMode != params.FwdMode {
//check if there exists any non default network and tenants
if numVlans+numVxlans > 0 {
log.Errorf("Unable to update forwarding mode due to existing %d vlans and %d vxlans", numVlans, numVxlans)
return fmt.Errorf("please delete %v vlans and %v vxlans before changing forwarding mode", vlansInUse, vxlansInUse)
return errExistingNetworks("forwarding mode")
}
if global.FwdMode == "routing" {
//check if any bgp configurations exists.
//check if any bgp configurations exists.
bgpCfgs := &mastercfg.CfgBgpState{}
bgpCfgs.StateDriver = stateDriver
cfgs, _ := bgpCfgs.ReadAll()
Expand All @@ -264,8 +279,7 @@ func (ac *APIController) GlobalUpdate(global, params *contivModel.Global) error
}
if global.PvtSubnet != params.PvtSubnet {
if (global.PvtSubnet != "" || params.PvtSubnet != defHostPvtNet) && numVlans+numVxlans > 0 {
log.Errorf("Unable to update provate subnet due to existing networks")
return fmt.Errorf("please delete %v vlans and %v vxlans before changing private subnet", vlansInUse, vxlansInUse)
return errExistingNetworks("private subnet")
}
globalCfg.PvtSubnet = params.PvtSubnet
}
Expand Down
86 changes: 79 additions & 7 deletions netmaster/objApi/objapi_test.go
100755 → 100644
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"testing"
"time"

"github.com/contiv/contivmodel"
contivModel "github.com/contiv/contivmodel"
"github.com/contiv/contivmodel/client"
"github.com/contiv/netplugin/core"
"github.com/contiv/netplugin/netmaster/gstate"
Expand Down Expand Up @@ -297,7 +297,10 @@ func checkInspectGlobal(t *testing.T, expError bool, allocedVlans, allocedVxlans
}

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

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

}

// TestGlobalSetting tests global REST api
func TestGlobalSettingFwdMode(t *testing.T) {
// set to default values (no-op)
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")

// Create a vxlan network to verify you cannot change forward mode
checkCreateNetwork(t, false, "default", "contiv7", "", "vxlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")

// Should fail when changing the forwarding mode whenever there is a network
var err error
var expErr string
err = checkGlobalSet(t, true, "default", "1-4094", "1-10000", "routing", "proxy", "172.19.0.0/16")
expErr = "Unable to update forwarding mode due to existing 1 vxlans"
if strings.TrimSpace(err.Error()) != expErr {
t.Fatalf("Wrong error message, expected: '%v', got '%v'", expErr, err.Error())
}
// remove the vxlan network and add a vlan network
checkDeleteNetwork(t, false, "default", "contiv7")
checkCreateNetwork(t, false, "default", "contiv7", "", "vlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")
err = checkGlobalSet(t, true, "default", "1-4094", "1-10000", "routing", "proxy", "172.19.0.0/16")
expErr = "Unable to update forwarding mode due to existing 1 vlans"
if strings.TrimSpace(err.Error()) != expErr {
t.Fatalf("Wrong error message, expected: '%v', got '%v'", expErr, err.Error())
}

// remove the vlan network
checkDeleteNetwork(t, false, "default", "contiv7")

// make sure can change forwarding mode after network deleted
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "routing", "proxy", "172.21.0.0/16")

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

func TestGlobalSettingSubnet(t *testing.T) {
// set to default values (no-op)
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
// Create a network to see if global subnet changes are blocked
checkCreateNetwork(t, false, "default", "contiv7", "", "vxlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")

var err error
var expErr string
// This should fail
err = checkGlobalSet(t, true, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/16")
expErr = "Unable to update private subnet due to existing 1 vxlans"
if strings.TrimSpace(err.Error()) != expErr {
t.Fatalf("Wrong error message, expected: '%v', got '%v'", expErr, err.Error())
}

// remove the vxlan network and add a vlan network
checkDeleteNetwork(t, false, "default", "contiv7")
checkCreateNetwork(t, false, "default", "contiv7", "", "vlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")

// This should still fail
err = checkGlobalSet(t, true, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/16")
expErr = "Unable to update private subnet due to existing 1 vlans"
if strings.TrimSpace(err.Error()) != expErr {
t.Fatalf("Wrong error message, expected: '%v', got '%v'", expErr, err.Error())
}

// remove the network
checkDeleteNetwork(t, false, "default", "contiv7")
// make sure can change subnet after network deleted
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/16")
// reset back to default values
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
}

// TestGlobalSetting tests global REST api
func TestGlobalSetting(t *testing.T) {
// set to default values (no-op)
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")

// try basic modification
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.20.0.0/16")
// set aci mode
Expand All @@ -1350,11 +1427,6 @@ func TestGlobalSetting(t *testing.T) {
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/16")
// Try invalid pvt subnet
checkGlobalSet(t, true, "default", "1-4094", "1-10000", "bridge", "proxy", "172.21.0.0/24")
// Try changing subnet with active network
checkCreateNetwork(t, false, "default", "contiv7", "", "vxlan", "16.1.1.1/24", "16.1.1.254", 3200, "", "", "")
checkGlobalSet(t, true, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")
checkDeleteNetwork(t, false, "default", "contiv7")
checkGlobalSet(t, false, "default", "1-4094", "1-10000", "bridge", "proxy", "172.19.0.0/16")

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

0 comments on commit 50fd8b3

Please sign in to comment.