Skip to content

Commit 18b2f2f

Browse files
lucasirclucasirc
committed
Merge branch 'network_error_messages' into develop
2 parents 34a97db + 0a3600a commit 18b2f2f

8 files changed

Lines changed: 317 additions & 65 deletions

File tree

‎api/src/org/apache/cloudstack/api/command/user/network/DeleteNetworkCmd.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// under the License.
1717
package org.apache.cloudstack.api.command.user.network;
1818

19+
import com.cloud.utils.exception.CloudRuntimeException;
1920
import org.apache.log4j.Logger;
2021

2122
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
@@ -75,13 +76,19 @@ public String getCommandName() {
7576

7677
@Override
7778
public void execute() {
78-
CallContext.current().setEventDetails("Network Id: " + id);
79-
boolean result = _networkService.deleteNetwork(id, isForced());
80-
if (result) {
81-
SuccessResponse response = new SuccessResponse(getCommandName());
82-
setResponseObject(response);
83-
} else {
84-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete network");
79+
CallContext.current().setEventDetails("Network Id: " + getId());
80+
try {
81+
boolean result = _networkService.deleteNetwork(getId(), isForced());
82+
83+
if (result) {
84+
SuccessResponse response = new SuccessResponse(getCommandName());
85+
setResponseObject(response);
86+
} else {
87+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete network");
88+
}
89+
90+
}catch (CloudRuntimeException e ) {
91+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage(), e);
8592
}
8693
}
8794

‎api/test/org/apache/cloudstack/api/command/user/network/DeleteNetworkCmdTest.java‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package org.apache.cloudstack.api.command.user.network;
2+
3+
import com.cloud.network.NetworkService;
4+
import com.cloud.utils.exception.CloudRuntimeException;
5+
import junit.framework.TestCase;
6+
import org.apache.cloudstack.api.ServerApiException;
7+
import org.apache.cloudstack.api.response.SuccessResponse;
8+
9+
import org.apache.log4j.Logger;
10+
import org.junit.Before;
11+
import org.junit.Test;
12+
import org.mockito.Mockito;
13+
14+
public class DeleteNetworkCmdTest extends TestCase {
15+
16+
private static final Logger s_logger = Logger.getLogger(DeleteNetworkCmdTest.class);
17+
18+
private DeleteNetworkCmd cmd;
19+
20+
@Before
21+
public void setUp() {
22+
cmd = new DeleteNetworkCmd() {
23+
public Long getId() {
24+
return 1l;
25+
}
26+
public boolean isForced() {
27+
return false;
28+
}
29+
};
30+
}
31+
32+
@Test
33+
public void testExecute() {
34+
NetworkService netService = Mockito.mock(NetworkService.class);
35+
Mockito.when(netService.deleteNetwork(1l, false)).thenReturn(true);
36+
cmd._networkService = netService;
37+
38+
cmd.execute();
39+
40+
SuccessResponse response = (SuccessResponse) cmd.getResponseObject();
41+
assertNotNull(response);
42+
assertEquals("deletenetworkresponse", response.getResponseName());
43+
}
44+
45+
@Test
46+
public void testExecute_not_created() {
47+
NetworkService netService = Mockito.mock(NetworkService.class);
48+
Mockito.when(netService.deleteNetwork(1l, false)).thenReturn(false);
49+
cmd._networkService = netService;
50+
51+
try {
52+
cmd.execute();
53+
fail("expected exception ");
54+
}catch (ServerApiException e) {
55+
assertEquals("Failed to delete network", e.getMessage());
56+
}catch (Exception e) {
57+
fail("expected ServerApiException " + e.getClass() + " " + e.getMessage());
58+
} finally {
59+
Mockito.verify(netService, Mockito.times(1)).deleteNetwork(1l, false);
60+
}
61+
}
62+
63+
@Test
64+
public void testExecute_fail() {
65+
NetworkService netService = Mockito.mock(NetworkService.class);
66+
Mockito.when(netService.deleteNetwork(1l, false)).thenThrow(new CloudRuntimeException("not deleted"));
67+
cmd._networkService = netService;
68+
69+
try {
70+
cmd.execute();
71+
}catch (ServerApiException e) {
72+
assertEquals("not deleted", e.getMessage());
73+
}catch (Exception e) {
74+
fail("should be ServerApiException " + e.getClass() + " " + e.getMessage());
75+
}finally {
76+
Mockito.verify(netService, Mockito.times(1)).deleteNetwork(1l, false);
77+
}
78+
}
79+
}

‎engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2125,7 +2125,7 @@ public boolean shutdownNetworkElementsAndResources(ReservationContext context, b
21252125

21262126
@Override
21272127
@DB
2128-
public boolean destroyNetwork(long networkId, final ReservationContext context, boolean forced) {
2128+
public boolean destroyNetwork(long networkId, final ReservationContext context, boolean forced) throws CloudRuntimeException {
21292129
final Account callerAccount = context.getAccount();
21302130

21312131
NetworkVO network = _networksDao.findById(networkId);
@@ -2134,15 +2134,7 @@ public boolean destroyNetwork(long networkId, final ReservationContext context,
21342134
return false;
21352135
}
21362136

2137-
// Make sure that there are no user vms in the network that are not Expunged/Error
2138-
List<UserVmVO> userVms = _userVmDao.listByNetworkIdAndStates(networkId);
2139-
2140-
for (UserVmVO vm : userVms) {
2141-
if (!(vm.getState() == VirtualMachine.State.Expunging && vm.getRemoved() != null)) {
2142-
s_logger.warn("Can't delete the network, not all user vms are expunged. Vm " + vm + " is in " + vm.getState() + " state");
2143-
return false;
2144-
}
2145-
}
2137+
validateUserVMsInNetwork(networkId);
21462138

21472139
// Don't allow to delete network via api call when it has vms assigned to it
21482140
int nicCount = getActiveNicsInNetwork(networkId);
@@ -2266,6 +2258,24 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
22662258

22672259
return success;
22682260
}
2261+
// Make sure that there are no user vms in the network that are not Expunged/Error before delete network, else throws CloudRuntimeException
2262+
public void validateUserVMsInNetwork(Long networkId) throws CloudRuntimeException {
2263+
List<UserVmVO> userVms = _userVmDao.listByNetworkIdAndStates(networkId);
2264+
2265+
boolean error = false;
2266+
StringBuilder builder = new StringBuilder("");
2267+
for (UserVmVO vm : userVms) {
2268+
if (!(vm.getState() == VirtualMachine.State.Expunging && vm.getRemoved() != null)) {
2269+
error = true;
2270+
builder.append("Vm " + vm + " is in " + vm.getState() + " state. ");
2271+
}
2272+
}
2273+
if (error) {
2274+
String msg = "Can't delete the network, not all user vms are expunged. " + builder.toString();
2275+
s_logger.warn(msg);
2276+
throw new CloudRuntimeException(msg);
2277+
}
2278+
}
22692279

22702280
@Override
22712281
public boolean resourceCountNeedsUpdate(NetworkOffering ntwkOff, ACLType aclType) {

‎engine/orchestration/test/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java‎

Lines changed: 76 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,44 +16,48 @@
1616
// under the License.
1717
package org.apache.cloudstack.engine.orchestration;
1818

19-
import static org.mockito.Mockito.mock;
20-
import static org.mockito.Mockito.verify;
21-
import static org.mockito.Mockito.never;
22-
import static org.mockito.Mockito.times;
23-
import static org.mockito.Mockito.when;
24-
25-
import java.util.HashMap;
26-
import java.util.List;
27-
import java.util.ArrayList;
28-
import java.util.Map;
29-
30-
import junit.framework.TestCase;
31-
32-
import org.apache.log4j.Logger;
33-
import org.junit.Before;
34-
import org.junit.Test;
35-
import org.mockito.Matchers;
36-
3719
import com.cloud.network.Network;
38-
39-
import com.cloud.network.NetworkModel;
4020
import com.cloud.network.Network.GuestType;
4121
import com.cloud.network.Network.Service;
22+
import com.cloud.network.NetworkModel;
4223
import com.cloud.network.Networks.TrafficType;
4324
import com.cloud.network.dao.NetworkDao;
4425
import com.cloud.network.dao.NetworkServiceMapDao;
4526
import com.cloud.network.dao.NetworkVO;
4627
import com.cloud.network.element.DhcpServiceProvider;
4728
import com.cloud.network.guru.NetworkGuru;
48-
29+
import com.cloud.uservm.UserVm;
30+
import com.cloud.utils.exception.CloudRuntimeException;
4931
import com.cloud.vm.Nic;
5032
import com.cloud.vm.NicVO;
33+
import com.cloud.vm.UserVmVO;
5134
import com.cloud.vm.VirtualMachine;
52-
import com.cloud.vm.VirtualMachineProfile;
35+
import com.cloud.vm.VirtualMachine.State;
5336
import com.cloud.vm.VirtualMachine.Type;
37+
import com.cloud.vm.VirtualMachineProfile;
5438
import com.cloud.vm.dao.NicDao;
5539
import com.cloud.vm.dao.NicIpAliasDao;
5640
import com.cloud.vm.dao.NicSecondaryIpDao;
41+
import com.cloud.vm.dao.UserVmDaoImpl;
42+
import junit.framework.TestCase;
43+
import org.apache.log4j.Logger;
44+
import org.junit.Before;
45+
import org.junit.Test;
46+
import org.mockito.Matchers;
47+
48+
import java.util.ArrayList;
49+
import java.util.Date;
50+
import java.util.HashMap;
51+
import java.util.List;
52+
import java.util.Map;
53+
54+
import static org.mockito.Mockito.mock;
55+
import static org.mockito.Mockito.times;
56+
import static org.mockito.Mockito.verify;
57+
import static org.mockito.Mockito.when;
58+
import static org.mockito.Mockito.never;
59+
60+
5761

5862
/**
5963
* NetworkManagerImpl implements NetworkManager.
@@ -161,4 +165,54 @@ public void testDontRemoveDhcpServiceWhenNotProvided() {
161165
verify(testOrchastrator._ntwkSrvcDao, never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp);
162166
verify(testOrchastrator._networksDao, times(1)).findById(nic.getNetworkId());
163167
}
168+
169+
@Test
170+
public void testValidateUserVMsInNetwork() {
171+
List list = new ArrayList<UserVm>();
172+
list.add(newUserVm(1l, "vm 01", State.Expunging, new Date()));
173+
174+
UserVmDaoImpl dao = mock(UserVmDaoImpl.class);
175+
when(dao.listByNetworkIdAndStates(1l)).thenReturn(list);
176+
177+
testOrchastrator._userVmDao = dao;
178+
testOrchastrator.validateUserVMsInNetwork(1l);
179+
180+
}
181+
182+
@Test
183+
public void testValidateUserVMsInNetwork_fail() {
184+
List list = new ArrayList<UserVm>();
185+
list.add(newUserVm(123l, "vm-0123", State.Stopped, null));
186+
list.add(newUserVm(555l, "vm-555", State.Running, null));
187+
188+
try {
189+
UserVmDaoImpl dao = mock(UserVmDaoImpl.class);
190+
when(dao.listByNetworkIdAndStates(1l)).thenReturn(list);
191+
192+
testOrchastrator._userVmDao = dao;
193+
testOrchastrator.validateUserVMsInNetwork(1l);
194+
} catch (CloudRuntimeException e ) {
195+
assertTrue(e.getMessage().contains("vm-0123"));
196+
assertTrue(e.getMessage().contains("vm-555"));
197+
} catch (Exception e) {
198+
fail("should be CloudRuntimeException with vm name. " + e.getClass()+" "+ e.getMessage());
199+
}
200+
201+
}
202+
203+
204+
public UserVm newUserVm(Long id, String name, State state, final Date removedTemp) {
205+
UserVmVO vm = new UserVmVO(id, name, null, 0l, null,
206+
0l, false, false, 0l, 0l,
207+
0l, null, null, 0l) {
208+
public Date getRemoved() {
209+
return removedTemp;
210+
}
211+
};
212+
213+
vm.setState(state);
214+
215+
216+
return vm;
217+
}
164218
}

‎plugins/network-elements/globonetwork/src/com/globo/globonetwork/cloudstack/api/DeleteNetworkInGloboNetworkCmd.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import javax.inject.Inject;
2020

21+
import com.cloud.utils.exception.CloudRuntimeException;
2122
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
2223
import org.apache.cloudstack.api.ACL;
2324
import org.apache.cloudstack.api.APICommand;
@@ -79,13 +80,17 @@ public String getCommandName() {
7980

8081
@Override
8182
public void execute() {
82-
CallContext.current().setEventDetails("Network Id: " + id);
83-
boolean result = _glbNetService.destroyGloboNetwork(id, isForced());
84-
if (result) {
85-
SuccessResponse response = new SuccessResponse(getCommandName());
86-
setResponseObject(response);
87-
} else {
88-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete network");
83+
CallContext.current().setEventDetails("Network Id: " + getId());
84+
try {
85+
boolean result = _glbNetService.destroyGloboNetwork(getId(), isForced());
86+
if (result) {
87+
SuccessResponse response = new SuccessResponse(getCommandName());
88+
setResponseObject(response);
89+
} else {
90+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to delete network");
91+
}
92+
}catch (CloudRuntimeException e ) {
93+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage(), e);
8994
}
9095
}
9196

0 commit comments

Comments
 (0)