Skip to content

Commit

Permalink
Fixed the issue osc-core issue #543 (#621)
Browse files Browse the repository at this point in the history 
* Fixed the issue osc-core issue #543

Modified and added test cases.

Added back to original intialization

Made the changes according to the review comments.

Did required changes for unit test case.

Did the changes as per review comments.

* Changes are incorporated as per the review comments.

* Changes have done as per review comments.

Change1:Reframed the Error Message.
Change2:Renamed the test methods.
Change3:Added the TODO comments for future references.
Change4:Reframed the throw message
  • Loading branch information
@balmukundblr @sudhirappaji
balmukundblr authored and sudhirappaji committed Jan 15, 2018
1 parent 05d9339 commit 2e8ac34
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 7 deletions.
8 changes: 6 additions & 2 deletions osc-server/src/main/java/org/osc/core/broker/service/persistence/VirtualSystemEntityMgr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,12 @@ public static synchronized Long generateUniqueTag(EntityManager em, VirtualSyste
return prevVal;
}

/**
* @param VirtualSystem
* @return true if ServiceFunctionChain(SFC) is bind with SecurityGroup(SG)
*/
public static boolean isProtectingWorkload(VirtualSystem vs) {
return !vs.getServiceFunctionChains().isEmpty() ||
CollectionUtils.emptyIfNull(vs.getDeploymentSpecs()).stream().anyMatch(ds -> DeploymentSpecEntityMgr.isProtectingWorkload(ds));
return CollectionUtils.emptyIfNull(vs.getDeploymentSpecs()).stream()
.anyMatch(ds -> DeploymentSpecEntityMgr.isProtectingWorkload(ds));
}
}
43 changes: 41 additions & 2 deletions ...ava/org/osc/core/broker/service/validator/DeleteDistributedApplianceRequestValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,24 @@
*******************************************************************************/
package org.osc.core.broker.service.validator;

import java.util.List;
import java.util.stream.Collectors;

import javax.persistence.EntityManager;

import org.osc.core.broker.model.entities.appliance.DistributedAppliance;
import org.osc.core.broker.model.entities.appliance.VirtualSystem;
import org.osc.core.broker.model.entities.virtualization.SecurityGroup;
import org.osc.core.broker.model.entities.virtualization.ServiceFunctionChain;
import org.osc.core.broker.service.exceptions.VmidcBrokerValidationException;
import org.osc.core.broker.service.persistence.DistributedApplianceEntityMgr;
import org.osc.core.broker.service.persistence.SecurityGroupEntityMgr;
import org.osc.core.broker.service.request.BaseDeleteRequest;

public class DeleteDistributedApplianceRequestValidator implements RequestValidator<BaseDeleteRequest,DistributedAppliance> {

private EntityManager em;


public DeleteDistributedApplianceRequestValidator(EntityManager em) {
this.em = em;
}
Expand All @@ -53,13 +59,46 @@ public DistributedAppliance validateAndLoad(BaseDeleteRequest request) throws Ex
da.getId()));
}

validateDAReferenceToSfcAndSecurityGroup(em, da);

if (!da.getMarkedForDeletion() && request.isForceDelete()) {
throw new VmidcBrokerValidationException(
"Distributed Appilance with ID "
+ request.getId()
+ " is not marked for deletion and force delete operation is applicable only for entries marked for deletion.");
}

return da;
}

private void validateDAReferenceToSfcAndSecurityGroup(EntityManager em, DistributedAppliance da) throws Exception {
for (VirtualSystem vs : da.getVirtualSystems()) {
try {
validateVSReferenceToSfcAndSecurityGroup(em, vs);
} catch (VmidcBrokerValidationException ex) {
throw new VmidcBrokerValidationException(String.format("Cannot delete the distributed appliance '%s' reason:",da.getName())+" "+ex.getMessage());
}
}
}

private void validateVSReferenceToSfcAndSecurityGroup(EntityManager em, VirtualSystem vs) throws Exception {
if (vs.getServiceFunctionChains() == null) {
return;
}
for (ServiceFunctionChain sfc : vs.getServiceFunctionChains()) {
validateSfcReferenceToSecurityGroup(em, sfc);
}
}

private void validateSfcReferenceToSecurityGroup(EntityManager em, ServiceFunctionChain sfc) throws Exception {
List<SecurityGroup> sgList = SecurityGroupEntityMgr.listSecurityGroupsBySfcId(em, sfc.getId());
String sgNames = sgList.stream().filter(sg -> !sg.getMarkedForDeletion()).map(sg -> sg.getName())
.collect(Collectors.joining(", "));
if (!sgNames.isEmpty()) {
throw new VmidcBrokerValidationException(String.format("Distributed appliance is referencing to Service Function Chain '%s' and binded to a Security Group(s) '%s'",
sfc.getName(), sgNames));
} else if (!sfc.getMarkedForDeletion() && sfc != null) {
throw new VmidcBrokerValidationException(String.format("Distributed appliance is referencing to Service Function Chain '%s'",
sfc.getName()));
}
}
}
75 changes: 72 additions & 3 deletions ...a/org/osc/core/broker/service/request/DeleteDistributedApplianceRequestValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
*******************************************************************************/
package org.osc.core.broker.service.request;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import javax.persistence.EntityManager;

Expand All @@ -25,16 +27,26 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.osc.core.broker.model.entities.appliance.DistributedAppliance;
import org.osc.core.broker.model.entities.appliance.TagEncapsulationType;
import org.osc.core.broker.model.entities.appliance.VirtualSystem;
import org.osc.core.broker.model.entities.virtualization.SecurityGroup;
import org.osc.core.broker.model.entities.virtualization.ServiceFunctionChain;
import org.osc.core.broker.service.exceptions.VmidcBrokerValidationException;
import org.osc.core.broker.service.persistence.DistributedApplianceEntityMgr;
import org.osc.core.broker.service.persistence.SecurityGroupEntityMgr;
import org.osc.core.broker.service.validator.DeleteDistributedApplianceRequestValidator;

import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
//TODO balmukund: Because the class under test now has a dependency on a complex DB query listReferencedVSBySecurityGroup it must be refactored to use an in mem db.
// Until then powermock is being used to mock static dependencies. This should be removed when this refactoring happens.
@RunWith(PowerMockRunner.class)
@PrepareForTest({SecurityGroupEntityMgr.class,DistributedApplianceEntityMgr.class})
public class DeleteDistributedApplianceRequestValidatorTest {

private static final Long VALID_ID = 1l;
Expand Down Expand Up @@ -126,8 +138,10 @@ public void testValidateAndLoad_WithNonForceDeleteRequestAndMarkedForDeletionDA_
Assert.assertEquals("The received ID in non force delete case is different than expected VALID_ID_FOR_DELETION.", VALID_ID_FOR_DELETION, da.getId());
}

//TODO balmukund: Because the class under test now has a dependency on a complex DB query listReferencedVSBySecurityGroup it must be refactored to use an in mem db.
// until then Powermock is being used to mock static dependencies. This should be removed when this refactoring happens.
@Test
public void testValidateAndLoad_WhenChainedToSfcDeleteRequest_ThrowsValidationException() throws Exception {
public void testValidateAndLoad_WhenChainedToSfc_ThrowsValidationException() throws Exception {
// Arrange
final Long VALID_ID_WITH_SFC = 4l;
DistributedAppliance da = createDA(VALID_ID_WITH_SFC, false);
Expand All @@ -138,7 +152,8 @@ public void testValidateAndLoad_WhenChainedToSfcDeleteRequest_ThrowsValidationEx
vs.setEncapsulationType(TagEncapsulationType.VLAN);
vs.setServiceFunctionChains(Arrays.asList(sfc));
da.addVirtualSystem(vs);

PowerMockito.mockStatic(DistributedApplianceEntityMgr.class);
PowerMockito.when(DistributedApplianceEntityMgr.isProtectingWorkload(da)).thenReturn(true);
Mockito.when(this.em.find(Mockito.eq(DistributedAppliance.class), Mockito.eq(VALID_ID_WITH_SFC)))
.thenReturn(da);
this.exception.expect(VmidcBrokerValidationException.class);
Expand All @@ -150,6 +165,60 @@ public void testValidateAndLoad_WhenChainedToSfcDeleteRequest_ThrowsValidationEx
this.validator.validateAndLoad(createRequest(VALID_ID_WITH_SFC, false));
}

@Test
public void testValidateAndLoad_WhenChainedToSfcAndSg_WhenSfcMode_ThrowsValidationException()
throws Exception {
// Arrange
final Long VALID_ID_WITH_SFC = 4l;
DistributedAppliance da = createDA(VALID_ID_WITH_SFC, false);
List<SecurityGroup> sgList=new ArrayList<>();
SecurityGroup sg=new SecurityGroup(null,null,null);
sg.setName("sg");
sgList.add(sg);
ServiceFunctionChain sfc = new ServiceFunctionChain("sfc-1", null);
sfc.setId(VALID_ID_WITH_SFC);
VirtualSystem vs = new VirtualSystem(null);
vs.setName("vs-1");
vs.setEncapsulationType(TagEncapsulationType.VLAN);
vs.setServiceFunctionChains(Arrays.asList(sfc));

da.addVirtualSystem(vs);
PowerMockito.mockStatic(SecurityGroupEntityMgr.class);
Mockito.when(SecurityGroupEntityMgr.listSecurityGroupsBySfcId(em,VALID_ID_WITH_SFC)).thenReturn(sgList);
Mockito.when(this.em.find(Mockito.eq(DistributedAppliance.class), Mockito.eq(VALID_ID_WITH_SFC)))
.thenReturn(da);
this.exception.expect(VmidcBrokerValidationException.class);
this.exception.expectMessage(String.format("Distributed appliance is referencing to Service Function Chain '%s' and binded to a Security Group(s) '%s'",
sfc.getName(), sg.getName()));

// Act
this.validator.validateAndLoad(createRequest(VALID_ID_WITH_SFC, false));
}

@Test
public void testValidateAndLoad_WhenChainedToSfc_WhenSfcMode_ThrowsValidationException()
throws Exception {
// Arrange
final Long VALID_ID_WITH_SFC = 4l;
DistributedAppliance da = createDA(VALID_ID_WITH_SFC, false);
ServiceFunctionChain sfc = new ServiceFunctionChain("sfc-1", null);
List<SecurityGroup> sgList=new ArrayList<>();
VirtualSystem vs = new VirtualSystem(null);
vs.setName("vs-1");
vs.setEncapsulationType(TagEncapsulationType.VLAN);
vs.setServiceFunctionChains(Arrays.asList(sfc));
da.addVirtualSystem(vs);
PowerMockito.mockStatic(SecurityGroupEntityMgr.class);
Mockito.when(SecurityGroupEntityMgr.listSecurityGroupsBySfcId(em,VALID_ID_WITH_SFC)).thenReturn(sgList);
Mockito.when(em.find(Mockito.eq(DistributedAppliance.class), Mockito.eq(VALID_ID_WITH_SFC)))
.thenReturn(da);
this.exception.expect(VmidcBrokerValidationException.class);
this.exception.expectMessage(String.format("Distributed appliance is referencing to Service Function Chain '%s'",sfc.getName()));

// Act
this.validator.validateAndLoad(createRequest(VALID_ID_WITH_SFC, false));
}

private static BaseDeleteRequest createRequest(Long id, boolean forceDelete) {
BaseDeleteRequest request = new BaseDeleteRequest(id);
request.setForceDelete(forceDelete);
Expand Down

0 comments on commit 2e8ac34

Please sign in to comment.