diff --git a/netmaster/objApi/apiController.go b/netmaster/objApi/apiController.go old mode 100755 new mode 100644 index 70a5faad2..a09654e83 --- a/netmaster/objApi/apiController.go +++ b/netmaster/objApi/apiController.go @@ -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" @@ -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" @@ -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() @@ -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 } diff --git a/netmaster/objApi/objapi_test.go b/netmaster/objApi/objapi_test.go old mode 100755 new mode 100644 index d2ca29f91..2c3cb7668 --- a/netmaster/objApi/objapi_test.go +++ b/netmaster/objApi/objapi_test.go @@ -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" @@ -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, @@ -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 @@ -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 @@ -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")