Support the delete funcion for network

Also refine the testing approach of listing network.

src/networkprovider/fake.go

@@ -23,3 +23,10 @@ func (fake FakeNetworkProvider) CreateNetwork(sp *serviceprovider.Container, net
 	}
 	return nil
 }
+
+func (fake FakeNetworkProvider) DeleteNetwork(sp *serviceprovider.Container, net entity.Network) error {
+	if fake.IWantFail {
+		return fmt.Errorf("Fail to delete network but don't worry, I'm fake network")
+	}
+	return nil
+}

src/networkprovider/fake_test.go

@@ -8,7 +8,7 @@ import (
 	"github.com/linkernetworks/vortex/src/entity"
 )
 
-func TestFakeNetworkValidateBeforeCreating(t *testing.T) {
+func TestNetworkValidateBeforeCreating(t *testing.T) {
 	fake, err := GetNetworkProvider(&entity.Network{
 		Type: "fake",
 		Fake: entity.FakeNetwork{
@@ -56,3 +56,25 @@ func TestFakeNetworkCreatingFail(t *testing.T) {
 	err = fake.CreateNetwork(nil, entity.Network{})
 	assert.Error(t, err)
 }
+
+func TestFakeNetworkDelete(t *testing.T) {
+	fake, err := GetNetworkProvider(&entity.Network{
+		Type: "fake",
+		Fake: entity.FakeNetwork{},
+	})
+	assert.NoError(t, err)
+	err = fake.DeleteNetwork(nil, entity.Network{})
+	assert.NoError(t, err)
+}
+
+func TestFakeNetworkDeleteFail(t *testing.T) {
+	fake, err := GetNetworkProvider(&entity.Network{
+		Type: "fake",
+		Fake: entity.FakeNetwork{
+			IWantFail: true,
+		},
+	})
+	assert.NoError(t, err)
+	err = fake.DeleteNetwork(nil, entity.Network{})
+	assert.Error(t, err)
+}

src/networkprovider/network.go

@@ -9,6 +9,7 @@ import (
 type NetworkProvider interface {
 	ValidateBeforeCreating(sp *serviceprovider.Container, net entity.Network) error
 	CreateNetwork(sp *serviceprovider.Container, net entity.Network) error
+	DeleteNetwork(sp *serviceprovider.Container, net entity.Network) error
 }
 
 func GetNetworkProvider(network *entity.Network) (NetworkProvider, error) {

src/networkprovider/network_test.go

@@ -29,5 +29,12 @@ func TestGetNetworkProvider(t *testing.T) {
 			assert.Equal(t, a, tc.netProviderType)
 		})
 	}
+}
 
+func TestGetNetworkProviderFail(t *testing.T) {
+	_, err := GetNetworkProvider(
+		&entity.Network{
+			Type: "Unknown",
+		})
+	assert.Error(t, err)
 }

src/networkprovider/ovs.go

@@ -52,3 +52,18 @@ func (ovs OVSNetworkProvider) CreateNetwork(sp *serviceprovider.Container, netwo
 
 	return nc.CreateOVSNetwork(ovs.BridgeName, ovs.PhysicalPorts)
 }
+
+func (ovs OVSNetworkProvider) DeleteNetwork(sp *serviceprovider.Container, network entity.Network) error {
+	nodeIP, err := sp.KubeCtl.GetNodeExternalIP(network.NodeName)
+	if err != nil {
+		return err
+	}
+
+	target := net.JoinHostPort(nodeIP, networkcontroller.DEFAULT_CONTROLLER_PORT)
+	nc, err := networkcontroller.New(target)
+	if err != nil {
+		return err
+	}
+
+	return nc.DeleteOVSNetwork(ovs.BridgeName)
+}

src/networkprovider/ovs_test.go

@@ -42,7 +42,7 @@ type NetworkTestSuite struct {
 	sp       *serviceprovider.Container
 	fakeName string //Use for non-connectivity node
 	np       OVSNetworkProvider
-	network  *entity.Network
+	network  entity.Network
 }
 
 func (suite *NetworkTestSuite) SetupSuite() {
@@ -89,7 +89,7 @@ func (suite *NetworkTestSuite) SetupSuite() {
 	suite.NoError(err)
 
 	tName := namesgenerator.GetRandomName(0)
-	suite.network = &entity.Network{
+	suite.network = entity.Network{
 		Name: tName,
 		OVS: entity.OVSNetwork{
 			BridgeName:    tName,
@@ -99,7 +99,7 @@ func (suite *NetworkTestSuite) SetupSuite() {
 		NodeName: nodeName,
 	}
 
-	np, err := GetNetworkProvider(suite.network)
+	np, err := GetNetworkProvider(&suite.network)
 	suite.NoError(err)
 	suite.np = np.(OVSNetworkProvider)
 }
@@ -122,19 +122,19 @@ func TestNetworkSuite(t *testing.T) {
 
 func (suite *NetworkTestSuite) TestCreateNetwork() {
 	//Parameters
-	err := suite.np.CreateNetwork(suite.sp, *(suite.network))
+	err := suite.np.CreateNetwork(suite.sp, suite.network)
 	suite.NoError(err)
 	defer exec.Command("ovs-vsctl", "del-br", suite.np.BridgeName).Run()
 }
 
 func (suite *NetworkTestSuite) TestCreateNetworkFail() {
-	network := suite.network
+	network := entity.Network{}
 	network.NodeName = "non-exist"
-	err := suite.np.CreateNetwork(suite.sp, *network)
+	err := suite.np.CreateNetwork(suite.sp, network)
 	suite.Error(err)
 
 	network.NodeName = suite.fakeName
-	err = suite.np.CreateNetwork(suite.sp, *network)
+	err = suite.np.CreateNetwork(suite.sp, network)
 	suite.Error(err)
 }
 
@@ -148,7 +148,7 @@ func (suite *NetworkTestSuite) TestValidateBeforeCreating() {
 
 	ovsProvider := suite.np
 	ovsProvider.PhysicalPorts = []entity.PhysicalPort{eth1}
-	err := ovsProvider.ValidateBeforeCreating(suite.sp, *suite.network)
+	err := ovsProvider.ValidateBeforeCreating(suite.sp, suite.network)
 	suite.NoError(err)
 }
 
@@ -161,7 +161,7 @@ func (suite *NetworkTestSuite) TestValidateBeforeCreatingFail() {
 	err := session.C(entity.NetworkCollectionName).Insert(suite.network)
 	defer session.C(entity.NetworkCollectionName).Remove(suite.network)
 	suite.NoError(err)
-	err = ovsProvider.ValidateBeforeCreating(suite.sp, *suite.network)
+	err = ovsProvider.ValidateBeforeCreating(suite.sp, suite.network)
 	suite.Error(err)
 
 	//Test wrong vlan ID
@@ -172,6 +172,25 @@ func (suite *NetworkTestSuite) TestValidateBeforeCreatingFail() {
 	}
 
 	ovsProvider.PhysicalPorts = []entity.PhysicalPort{eth1}
-	err = ovsProvider.ValidateBeforeCreating(suite.sp, *suite.network)
+	err = ovsProvider.ValidateBeforeCreating(suite.sp, suite.network)
+	suite.Error(err)
+}
+
+func (suite *NetworkTestSuite) TestDeleteNetwork() {
+	//Parameters
+	exec.Command("ovs-vsctl", "add-br", suite.np.BridgeName).Run()
+	//FIXME we need a function to check the bridge is exist
+	err := suite.np.DeleteNetwork(suite.sp, suite.network)
+	suite.NoError(err)
+}
+
+func (suite *NetworkTestSuite) TestDeleteNetworkFail() {
+	network := entity.Network{}
+	network.NodeName = "non-exist"
+	err := suite.np.DeleteNetwork(suite.sp, network)
+	suite.Error(err)
+
+	network.NodeName = suite.fakeName
+	err = suite.np.DeleteNetwork(suite.sp, network)
 	suite.Error(err)
 }

src/server/handler_network.go

@@ -5,7 +5,6 @@ import (
 	"math"
 	"strconv"
 
-	"github.com/linkernetworks/logger"
 	"github.com/linkernetworks/utils/timeutils"
 	"github.com/linkernetworks/vortex/src/entity"
 	response "github.com/linkernetworks/vortex/src/net/http"
@@ -152,15 +151,13 @@ func deleteNetworkHandler(ctx *web.Context) {
 		return
 	}
 
-	nc, err := networkcontroller.New(sp.KubeCtl, network)
+	networkProvider, err := np.GetNetworkProvider(&network)
 	if err != nil {
-		logger.Errorf("Failed to new network controller: %s", err.Error())
-		response.InternalServerError(req.Request, resp.ResponseWriter, err)
+		response.BadRequest(req.Request, resp.ResponseWriter, err)
 		return
 	}
 
-	if err := nc.DeleteNetwork(); err != nil {
-		logger.Errorf("Failed to delete network: %s", err.Error())
+	if err := networkProvider.DeleteNetwork(sp, network); err != nil {
 		response.InternalServerError(req.Request, resp.ResponseWriter, err)
 		return
 	}

src/server/handler_network_test.go

@@ -169,6 +169,48 @@ func (suite *NetworkTestSuite) TestDeleteEmptyNetwork() {
 	assertResponseCode(suite.T(), http.StatusNotFound, httpWriter)
 }
 
+func (suite *NetworkTestSuite) TestDeleteNetworkFail() {
+	testCases := []struct {
+		cases     string
+		network   entity.Network
+		errorCode int
+	}{
+		{"DeleteNetwork", entity.Network{
+			ID:   bson.NewObjectId(),
+			Name: namesgenerator.GetRandomName(0),
+			Type: "fake",
+			Fake: entity.FakeNetwork{
+				FakeParameter: "Yo",
+				IWantFail:     true,
+			}},
+			http.StatusInternalServerError},
+		{"NetworkTypeError", entity.Network{
+			ID:   bson.NewObjectId(),
+			Name: namesgenerator.GetRandomName(0),
+			Type: "non-exist",
+			Fake: entity.FakeNetwork{
+				FakeParameter: "Yo",
+				IWantFail:     true,
+			}},
+			http.StatusBadRequest},
+	}
+
+	for _, tc := range testCases {
+		suite.T().Run(tc.cases, func(t *testing.T) {
+			suite.session.C(entity.NetworkCollectionName).Insert(tc.network)
+			defer suite.session.Remove(entity.NetworkCollectionName, "name", tc.network.Name)
+
+			httpRequest, err := http.NewRequest("DELETE", "http://localhost:7890/v1/networks/"+tc.network.ID.Hex(), nil)
+			suite.NoError(err)
+
+			httpRequest.Header.Add("Content-Type", "application/json")
+			httpWriter := httptest.NewRecorder()
+			suite.wc.Dispatch(httpWriter, httpRequest)
+			assertResponseCode(suite.T(), tc.errorCode, httpWriter)
+		})
+	}
+}
+
 //Fot Get/List, we only return mongo document
 func (suite *NetworkTestSuite) TestGetNetwork() {
 	tName := namesgenerator.GetRandomName(0)
@@ -212,7 +254,9 @@ func (suite *NetworkTestSuite) TestGetNetworkWithInvalidID() {
 
 func (suite *NetworkTestSuite) TestListNetwork() {
 	networks := []entity.Network{}
-	for i := 0; i < 3; i++ {
+
+	count := 3
+	for i := 0; i < count; i++ {
 		networks = append(networks, entity.Network{
 			Name:     namesgenerator.GetRandomName(0),
 			Fake:     entity.FakeNetwork{},
@@ -221,64 +265,49 @@ func (suite *NetworkTestSuite) TestListNetwork() {
 		})
 	}
 
+	testCases := []struct {
+		page       string
+		pageSize   string
+		expectSize int
+	}{
+		{"", "", count},
+		{"1", "1", count},
+		{"1", "3", count},
+	}
+
 	for _, v := range networks {
 		err := suite.session.C(entity.NetworkCollectionName).Insert(v)
 		defer suite.session.Remove(entity.NetworkCollectionName, "name", v.Name)
 		suite.NoError(err)
 	}
 
-	//list data by default page and page_size
-	httpRequest, err := http.NewRequest("GET", "http://localhost:7890/v1/networks/", nil)
-	suite.NoError(err)
-
-	httpWriter := httptest.NewRecorder()
-	suite.wc.Dispatch(httpWriter, httpRequest)
-	assertResponseCode(suite.T(), http.StatusOK, httpWriter)
-
-	retNetworks := []entity.Network{}
-	err = json.Unmarshal(httpWriter.Body.Bytes(), &retNetworks)
-	suite.NoError(err)
-	suite.Equal(len(networks), len(retNetworks))
-	for i, v := range retNetworks {
-		suite.Equal(networks[i].Name, v.Name)
-		suite.Equal(networks[i].Type, v.Type)
-		suite.Equal(networks[i].NodeName, v.NodeName)
-	}
-
-	//list data by different page and page_size
-	httpRequest, err = http.NewRequest("GET", "http://localhost:7890/v1/networks?page=1&page_size=3", nil)
-	suite.NoError(err)
-
-	httpWriter = httptest.NewRecorder()
-	suite.wc.Dispatch(httpWriter, httpRequest)
-	assertResponseCode(suite.T(), http.StatusOK, httpWriter)
+	for _, tc := range testCases {
+		caseName := "page:pageSize" + tc.page + ":" + tc.pageSize
+		suite.T().Run(caseName, func(t *testing.T) {
+			url := "http://localhost:7890/v1/networks/"
+			if tc.page != "" || tc.pageSize != "" {
+				url = "http://localhost:7890/v1/networks?"
+				url += "page=" + tc.page + "%" + "page_size" + tc.pageSize
+			}
+			httpRequest, err := http.NewRequest("GET", url, nil)
 
-	retNetworks = []entity.Network{}
-	err = json.Unmarshal(httpWriter.Body.Bytes(), &retNetworks)
-	suite.NoError(err)
-	suite.Equal(len(networks), len(retNetworks))
-	for i, v := range retNetworks {
-		suite.Equal(networks[i].Name, v.Name)
-		suite.Equal(networks[i].Type, v.Type)
-		suite.Equal(networks[i].NodeName, v.NodeName)
-	}
+			suite.NoError(err)
 
-	//list data by different page and page_size
-	httpRequest, err = http.NewRequest("GET", "http://localhost:7890/v1/networks?page=1&page_size=1", nil)
-	suite.NoError(err)
+			httpWriter := httptest.NewRecorder()
+			suite.wc.Dispatch(httpWriter, httpRequest)
+			assertResponseCode(suite.T(), http.StatusOK, httpWriter)
 
-	httpWriter = httptest.NewRecorder()
-	suite.wc.Dispatch(httpWriter, httpRequest)
-	assertResponseCode(suite.T(), http.StatusOK, httpWriter)
+			retNetworks := []entity.Network{}
+			err = json.Unmarshal(httpWriter.Body.Bytes(), &retNetworks)
+			suite.NoError(err)
+			suite.Equal(tc.expectSize, len(retNetworks))
+			for i, v := range retNetworks {
+				suite.Equal(networks[i].Name, v.Name)
+				suite.Equal(networks[i].Type, v.Type)
+				suite.Equal(networks[i].NodeName, v.NodeName)
+			}
 
-	retNetworks = []entity.Network{}
-	err = json.Unmarshal(httpWriter.Body.Bytes(), &retNetworks)
-	suite.NoError(err)
-	suite.Equal(1, len(retNetworks))
-	for i, v := range retNetworks {
-		suite.Equal(networks[i].Name, v.Name)
-		suite.Equal(networks[i].Type, v.Type)
-		suite.Equal(networks[i].NodeName, v.NodeName)
+		})
 	}
 }