Skip to content

Commit

Permalink
Merge pull request #484 from DivyaVavili/epDelete
Browse files Browse the repository at this point in the history 
Restrict EPG delete with active EPs
  • Loading branch information
@shaleman
shaleman authored Aug 17, 2016
2 parents 5d69318 + 79bbfe4 commit 0ac7db7
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 30 deletions.
37 changes: 36 additions & 1 deletion netmaster/master/endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,27 @@ func CreateEndpoint(stateDriver core.StateDriver, nwCfg *mastercfg.CfgNetworkSta
epCfg.EndpointGroupKey = mastercfg.GetEndpointGroupKey(ep.ServiceName, nwCfg.Tenant)
epCfg.EndpointGroupID, err = mastercfg.GetEndpointGroupID(stateDriver, ep.ServiceName, nwCfg.Tenant)
if err != nil {
log.Errorf("Error getting endpoint group for %s.%s. Err: %v", ep.ServiceName, nwCfg.ID, err)
log.Errorf("Error getting endpoint group ID for %s.%s. Err: %v", ep.ServiceName, nwCfg.ID, err)
return nil, err
}

if epCfg.EndpointGroupKey != "" {
epgCfg := &mastercfg.EndpointGroupState{}
epgCfg.StateDriver = stateDriver
err = epgCfg.Read(epCfg.EndpointGroupKey)
if err != nil {
log.Errorf("Error reading Epg info for EP: %+v. Error: %v", ep, err)
return nil, err
}

epgCfg.EpCount++

err = epgCfg.Write()
if err != nil {
log.Errorf("Error saving epg state: %+v", epgCfg)
return nil, err
}
}
}

err = nwCfg.IncrEpCount()
Expand Down Expand Up @@ -195,6 +213,23 @@ func DeleteEndpointID(stateDriver core.StateDriver, epID string) (*mastercfg.Cfg
log.Errorf("Error releasing endpoint state for: %s. Err: %v", epCfg.IPAddress, err)
}

if epCfg.EndpointGroupKey != "" {
epgCfg := &mastercfg.EndpointGroupState{}
epgCfg.StateDriver = stateDriver
err = epgCfg.Read(epCfg.EndpointGroupKey)
if err != nil {
log.Errorf("Error reading EPG for endpoint: %+v", epCfg)
}

epgCfg.EpCount--

// write updated epg state
err = epgCfg.Write()
if err != nil {
log.Errorf("error writing epg config. Error: %s", err)
}
}

// decrement ep count
nwCfg.EpCount--

Expand Down
10 changes: 9 additions & 1 deletion netmaster/master/endpointGroup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"errors"
"strconv"

"github.com/contiv/netplugin/core"
"github.com/contiv/netplugin/netmaster/docknet"
"github.com/contiv/netplugin/netmaster/gstate"
"github.com/contiv/netplugin/netmaster/mastercfg"
Expand Down Expand Up @@ -149,6 +150,10 @@ func DeleteEndpointGroup(tenantName, groupName string) error {
return err
}

if epgCfg.EpCount != 0 {
return core.Errorf("Error: EPG %s has active endpoints", groupName)
}

// Delete the endpoint group state
gCfg := gstate.Cfg{}
gCfg.StateDriver = stateDriver
Expand Down Expand Up @@ -181,5 +186,8 @@ func DeleteEndpointGroup(tenantName, groupName string) error {
return err
}

return docknet.DeleteDockNet(epgCfg.TenantName, epgCfg.NetworkName, epgCfg.GroupName)
if GetClusterMode() == "docker" {
return docknet.DeleteDockNet(epgCfg.TenantName, epgCfg.NetworkName, epgCfg.GroupName)
}
return nil
}
1 change: 1 addition & 0 deletions netmaster/mastercfg/endpointGroupState.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type EndpointGroupState struct {
PktTagType string `json:"pktTagType"`
PktTag int `json:"pktTag"`
ExtPktTag int `json:"extPktTag"`
EpCount int `json:"epCount"`
}

// Write the state.
Expand Down
27 changes: 17 additions & 10 deletions netmaster/objApi/apiController.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,11 +429,12 @@ func (ac *APIController) EndpointGetOper(endpoint *contivModel.EndpointInspect)
}

// Cleans up state off endpointGroup and related objects.
func endpointGroupCleanup(endpointGroup *contivModel.EndpointGroup) {
func endpointGroupCleanup(endpointGroup *contivModel.EndpointGroup) error {
// delete the endpoint group state
err := master.DeleteEndpointGroup(endpointGroup.TenantName, endpointGroup.GroupName)
if err != nil {
log.Errorf("Error deleting endpoint group %+v. Err: %v", endpointGroup, err)
return err
}

// Detach the endpoint group from the Policies
Expand Down Expand Up @@ -477,6 +478,8 @@ func endpointGroupCleanup(endpointGroup *contivModel.EndpointGroup) {
modeldb.RemoveLinkSet(&tenant.LinkSets.EndpointGroups, endpointGroup)
tenant.Write()
}

return nil
}

// FIXME: hack to allocate unique endpoint group ids
Expand Down Expand Up @@ -509,7 +512,7 @@ func (ac *APIController) EndpointGroupCreate(endpointGroup *contivModel.Endpoint
// create the endpoint group state
err := master.CreateEndpointGroup(endpointGroup.TenantName, endpointGroup.NetworkName, endpointGroup.GroupName)
if err != nil {
log.Errorf("Error creating endpoing group %+v. Err: %v", endpointGroup, err)
log.Errorf("Error creating endpoint group %+v. Err: %v", endpointGroup, err)
return err
}

Expand Down Expand Up @@ -686,8 +689,12 @@ func (ac *APIController) EndpointGroupDelete(endpointGroup *contivModel.Endpoint
endpointGroup.GroupName, endpointGroup.Links.AppProfile.ObjKey)
}

endpointGroupCleanup(endpointGroup)
return nil
err := endpointGroupCleanup(endpointGroup)
if err != nil {
log.Errorf("EPG cleanup failed: %+v", err)
}

return err
}

// NetworkCreate creates network
Expand Down Expand Up @@ -856,12 +863,6 @@ func (ac *APIController) NetworkDelete(network *contivModel.Network) error {
// Remove link
modeldb.RemoveLinkSet(&tenant.LinkSets.Networks, network)

// Save the tenant too since we removed the links
err := tenant.Write()
if err != nil {
return err
}

// Get the state driver
stateDriver, err := utils.GetStateDriver()
if err != nil {
Expand All @@ -876,6 +877,12 @@ func (ac *APIController) NetworkDelete(network *contivModel.Network) error {
return err
}

// Save the tenant too since we removed the links
err = tenant.Write()
if err != nil {
return err
}

return nil
}

Expand Down
38 changes: 20 additions & 18 deletions netmaster/objApi/objapi_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func checkCreateNetwork(t *testing.T, expError bool, tenant, network, nwType, en
if err != nil && !expError {
t.Fatalf("Error creating network {%+v}. Err: %v", net, err)
} else if err == nil && expError {
t.Fatalf("Create network {%+v} succeded while expecing error", net)
t.Fatalf("Create network {%+v} succeeded while expecting error", net)
} else if err == nil {
// verify network is created
_, err := contivClient.NetworkGet(tenant, network)
Expand All @@ -160,7 +160,7 @@ func checkInspectNetwork(t *testing.T, expError bool, tenant, network, allocedIP
if err != nil && !expError {
t.Fatalf("Error inspecting network {%s.%s}. Err: %v", network, tenant, err)
} else if err == nil && expError {
t.Fatalf("Inspect network {%s.%s} succeded while expecing error", network, tenant)
t.Fatalf("Inspect network {%s.%s} succeeded while expecting error", network, tenant)
} else if err == nil {
if insp.Oper.AllocatedAddressesCount != addrCount || insp.Oper.PktTag != pktTag ||
insp.Oper.AllocatedIPAddresses != allocedIPs {
Expand Down Expand Up @@ -200,7 +200,7 @@ func checkDeleteNetwork(t *testing.T, expError bool, tenant, network string) {
if err != nil && !expError {
t.Fatalf("Error deleting network %s/%s. Err: %v", tenant, network, err)
} else if err == nil && expError {
t.Fatalf("Delete network %s/%s succeded while expecing error", tenant, network)
t.Fatalf("Delete network %s/%s succeeded while expecting error", tenant, network)
} else if err == nil {
// verify network is gone
_, err := contivClient.NetworkGet(tenant, network)
Expand All @@ -225,7 +225,7 @@ func checkInspectGlobal(t *testing.T, expError bool, allocedVlans, allocedVxlans
if err != nil && !expError {
t.Fatalf("Error inspecting global info. Err: %v", err)
} else if err == nil && expError {
t.Fatalf("Inspect global info succeded while expecing error")
t.Fatalf("Inspect global info succeeded while expecting error")
} else if err == nil {
if insp.Oper.VlansInUse != allocedVlans || insp.Oper.VxlansInUse != allocedVxlans {
t.Fatalf("Inspect network {%+v} failed with mismatching ", insp)
Expand All @@ -246,7 +246,7 @@ func checkGlobalSet(t *testing.T, expError bool, fabMode, vlans, vxlans, fwdMode
if err != nil && !expError {
t.Fatalf("Error setting global {%+v}. Err: %v", gl, err)
} else if err == nil && expError {
t.Fatalf("Set global {%+v} succeded while expecing error", gl)
t.Fatalf("Set global {%+v} succeeded while expecting error", gl)
} else if err == nil {
// verify global state
gotGl, err := contivClient.GlobalGet("global")
Expand Down Expand Up @@ -306,7 +306,7 @@ func checkCreatePolicy(t *testing.T, expError bool, tenant, policy string) {
if err != nil && !expError {
t.Fatalf("Error creating policy {%+v}. Err: %v", pol, err)
} else if err == nil && expError {
t.Fatalf("Create policy {%+v} succeded while expecing error", pol)
t.Fatalf("Create policy {%+v} succeeded while expecting error", pol)
} else if err == nil {
// verify policy is created
_, err := contivClient.PolicyGet(tenant, policy)
Expand All @@ -322,7 +322,7 @@ func checkDeletePolicy(t *testing.T, expError bool, tenant, policy string) {
if err != nil && !expError {
t.Fatalf("Error deleting policy %s/%s. Err: %v", tenant, policy, err)
} else if err == nil && expError {
t.Fatalf("Delete policy %s/%s succeded while expecing error", tenant, policy)
t.Fatalf("Delete policy %s/%s succeeded while expecting error", tenant, policy)
} else if err == nil {
// verify policy is gone
_, err := contivClient.PolicyGet(tenant, policy)
Expand Down Expand Up @@ -354,7 +354,7 @@ func checkCreateRule(t *testing.T, expError bool, tenant, policy, ruleID, dir, f
if err != nil && !expError {
t.Fatalf("Error creating rule {%+v}. Err: %v", pol, err)
} else if err == nil && expError {
t.Fatalf("Create rule {%+v} succeded while expecing error", pol)
t.Fatalf("Create rule {%+v} succeeded while expecting error", pol)
} else if err == nil {
// verify rule is created
_, err := contivClient.RuleGet(tenant, policy, ruleID)
Expand All @@ -370,7 +370,7 @@ func checkDeleteRule(t *testing.T, expError bool, tenant, policy, ruleID string)
if err != nil && !expError {
t.Fatalf("Error deleting rule %s/%s/%s. Err: %v", tenant, policy, ruleID, err)
} else if err == nil && expError {
t.Fatalf("Delete rule %s/%s/%s succeded while expecing error", tenant, policy, ruleID)
t.Fatalf("Delete rule %s/%s/%s succeeded while expecting error", tenant, policy, ruleID)
} else if err == nil {
// verify rule is gone
_, err := contivClient.RuleGet(tenant, policy, ruleID)
Expand All @@ -393,7 +393,7 @@ func checkCreateEpg(t *testing.T, expError bool, tenant, network, group string,
if err != nil && !expError {
t.Fatalf("Error creating epg {%+v}. Err: %v", epg, err)
} else if err == nil && expError {
t.Fatalf("Create epg {%+v} succeded while expecing error", epg)
t.Fatalf("Create epg {%+v} succeeded while expecting error", epg)
} else if err == nil {
// verify epg is created
_, err := contivClient.EndpointGroupGet(tenant, group)
Expand All @@ -416,7 +416,7 @@ func checkCreateExtContractsGrp(t *testing.T, expError bool, tenant, grpName, gr
if err != nil && !expError {
t.Fatalf("Error creating extContractsGrp {%+v}. Err: %v", extContractsGrp, err)
} else if err == nil && expError {
t.Fatalf("Create extContrctsGrp {%+v} succeded while expecing error", extContractsGrp)
t.Fatalf("Create extContrctsGrp {%+v} succeeded while expecting error", extContractsGrp)
} else if err == nil {
// verify extContractsGrp is created
_, err := contivClient.ExtContractsGroupGet(tenant, grpName)
Expand All @@ -432,7 +432,7 @@ func checkDeleteExtContractsGrp(t *testing.T, expError bool, tenant, grpName str
if err != nil && !expError {
t.Fatalf("Error deleting extContractsGrp %s/%s. Err: %v", tenant, grpName, err)
} else if err == nil && expError {
t.Fatalf("Delete extContractsGrp %s/%s succeded while expecing error", tenant, grpName)
t.Fatalf("Delete extContractsGrp %s/%s succeeded while expecting error", tenant, grpName)
} else if err == nil {
// verify epg is gone
_, err := contivClient.ExtContractsGroupGet(tenant, grpName)
Expand Down Expand Up @@ -483,7 +483,7 @@ func checkEpgPolicyDeleted(t *testing.T, tenant, network, group, policy string)
// See if it already exists
gp := mastercfg.FindEpgPolicy(epgpKey)
if gp != nil {
t.Fatalf("Found EPG policy %s while expecing it to be deleted", epgpKey)
t.Fatalf("Found EPG policy %s while expecting it to be deleted", epgpKey)
}

}
Expand All @@ -494,7 +494,7 @@ func checkDeleteEpg(t *testing.T, expError bool, tenant, network, group string)
if err != nil && !expError {
t.Fatalf("Error deleting epg %s/%s. Err: %v", tenant, group, err)
} else if err == nil && expError {
t.Fatalf("Delete epg %s/%s succeded while expecing error", tenant, group)
t.Fatalf("Delete epg %s/%s succeeded while expecting error", tenant, group)
} else if err == nil {
// verify epg is gone
_, err := contivClient.EndpointGroupGet(tenant, group)
Expand All @@ -515,7 +515,7 @@ func checkCreateAppProfile(t *testing.T, expError bool, tenant, profName string,
if err != nil && !expError {
t.Fatalf("Error creating AppProfile {%+v}. Err: %v", prof, err)
} else if err == nil && expError {
t.Fatalf("Create AppProfile {%+v} succeded while expecing error", prof)
t.Fatalf("Create AppProfile {%+v} succeeded while expecting error", prof)
} else if err == nil {
// verify AppProfile is created
_, err := contivClient.AppProfileGet(tenant, profName)
Expand Down Expand Up @@ -561,12 +561,14 @@ func checkDeleteAppProfile(t *testing.T, expError bool, tenant, prof string) {
if err != nil && !expError {
t.Fatalf("Error deleting AppProfile %s/%s. Err: %v", tenant, prof, err)
} else if err == nil && expError {
t.Fatalf("Delete AppProfile %s/%s succeded while expecing error", tenant, prof)
t.Fatalf("Delete AppProfile %s/%s succeeded while expecting error", tenant, prof)
} else if err == nil {
// verify AppProfile is gone
_, err := contivClient.AppProfileGet(tenant, prof)
if err == nil {
t.Fatalf("AppProfile %s/%s not deleted", tenant, prof)
} else {
t.Logf("AppProfile %s/%s successfully deleted", tenant, prof)
}
}
}
Expand All @@ -582,7 +584,7 @@ func checkCreateTenant(t *testing.T, expError bool, tenantName string) {
if err != nil && !expError {
t.Fatalf("Error creating tenant {%+v}. Err: %v", tenant, err)
} else if err == nil && expError {
t.Fatalf("Create tenant {%+v} succeded while expecing error", tenant)
t.Fatalf("Create tenant {%+v} succeeded while expecting error", tenant)
} else if err == nil {
// verify tenant is created
_, err := contivClient.TenantGet(tenantName)
Expand All @@ -598,7 +600,7 @@ func checkDeleteTenant(t *testing.T, expError bool, tenant string) {
if err != nil && !expError {
t.Fatalf("Error deleting tenant %s. Err: %v", tenant, err)
} else if err == nil && expError {
t.Fatalf("Delete tenant %s succeded while expecing error", tenant)
t.Fatalf("Delete tenant %s succeeded while expecting error", tenant)
} else if err == nil {
// verify network is gone
_, err := contivClient.TenantGet(tenant)
Expand Down
46 changes: 46 additions & 0 deletions test/integration/endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,49 @@ func (its *integTestSuite) TestEndpointCreateDeleteParallel(c *C) {

assertNoErr(its.client.NetworkDelete("default", "test"), c, "deleting network")
}

// TestEndpointGroupCreateDelete tests EPG create delete ops
func (its *integTestSuite) TestEndpointGroupCreateDelete(c *C) {
// Create a network
err := its.client.NetworkPost(&client.Network{
TenantName: "default",
NetworkName: "test",
Subnet: "10.1.1.0/24",
Encap: its.encap,
})
assertNoErr(err, c, "creating network")

for i := 0; i < its.iterations; i++ {
err := its.client.EndpointGroupPost(&client.EndpointGroup{
TenantName: "default",
NetworkName: "test",
GroupName: "epg1",
Policies: []string{},
ExtContractsGrps: []string{},
})
addr, err := its.allocAddress("", "test.default", "")
assertNoErr(err, c, "allocating address")
c.Assert(addr, Equals, "10.1.1.1")

// create an endpoint in the network
epCfg1, err := its.createEndpoint("default", "test", "epg1", addr, "")
assertNoErr(err, c, "creating endpoint")

// delete epg with active endpoints - should FAIL
err = its.client.EndpointGroupDelete("default", "epg1")
assertErr(err, c, "deleting epg")

// delete the endpoints
err = its.deleteEndpoint("default", "test", "", epCfg1)
assertNoErr(err, c, "deleting endpoint")

// delete epg
err = its.client.EndpointGroupDelete("default", "epg1")
assertNoErr(err, c, "deleting epg")

// verify flows are also gone
its.verifyEndpointFlowRemoved(epCfg1, c)
}

assertNoErr(its.client.NetworkDelete("default", "test"), c, "deleting network")
}
9 changes: 9 additions & 0 deletions test/integration/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ func assertNoErr(err error, c *C, msg string) {
}
}

// assertErr utility function to assert no error
func assertErr(err error, c *C, msg string) {
if err == nil {
log.Errorf("Expected Error %s.", err)
debug.PrintStack()
c.Fatalf("Expected Error %s.", err)
}
}

// allocAddress gets an address from the master
func (its *integTestSuite) allocAddress(addrPool, networkID, prefAddress string) (string, error) {
// Build an alloc request to be sent to master
Expand Down

0 comments on commit 0ac7db7

Please sign in to comment.