From 40619b901798f1cea1f7baee820780b5b65d7bb8 Mon Sep 17 00:00:00 2001 From: Yevgeny Khodorkovsky Date: Wed, 4 Sep 2013 18:33:11 -0700 Subject: [PATCH] Bugfix: Exception when adding static host - Fix NumberFormatException when adding static host through REST API with no vlan configured. - Add IT to verify this - Clean up Host Tracker REST API Change-Id: Iefc672a0a978640ca47903f20962f150298ba11c Signed-off-by: Yevgeny Khodorkovsky --- .../controller/hosttracker/IfIptoHost.java | 2 +- .../hosttracker/internal/HostTracker.java | 61 +++---- .../hosttracker/internal/HostTrackerIT.java | 13 +- .../northbound/commons/RestMessages.java | 2 +- .../northbound/HostTrackerNorthbound.java | 168 ++++++------------ .../integrationtest/NorthboundIT.java | 10 +- 6 files changed, 99 insertions(+), 157 deletions(-) diff --git a/opendaylight/hosttracker/api/src/main/java/org/opendaylight/controller/hosttracker/IfIptoHost.java b/opendaylight/hosttracker/api/src/main/java/org/opendaylight/controller/hosttracker/IfIptoHost.java index 995ee57515..2451e196f2 100644 --- a/opendaylight/hosttracker/api/src/main/java/org/opendaylight/controller/hosttracker/IfIptoHost.java +++ b/opendaylight/hosttracker/api/src/main/java/org/opendaylight/controller/hosttracker/IfIptoHost.java @@ -120,7 +120,7 @@ public interface IfIptoHost { * @param nc * NodeConnector to which the host is attached * @param vlan - * VLAN the host belongs to + * VLAN the host belongs to (null or empty for no vlan) * @return The status object as described in {@code Status} indicating the * result of this action. */ diff --git a/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java b/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java index e4704d3048..9f0cd893b0 100644 --- a/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java +++ b/opendaylight/hosttracker/implementation/src/main/java/org/opendaylight/controller/hosttracker/internal/HostTracker.java @@ -307,7 +307,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw @Override public Future discoverHost(InetAddress networkAddress) { if (executor == null) { - logger.error("discoverHost: Null executor"); + logger.debug("discoverHost: Null executor"); return null; } Callable worker = new HostTrackerCallable(this, networkAddress); @@ -348,12 +348,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw @Override public Set getAllHosts() { - Set allHosts = new HashSet(); - for (Entry entry : hostsDB.entrySet()) { - HostNodeConnector host = entry.getValue(); - allHosts.add(host); - } - logger.debug("Exiting getAllHosts, Found {} Hosts", allHosts.size()); + Set allHosts = new HashSet(hostsDB.values()); return allHosts; } @@ -366,17 +361,12 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw list.add(host); } } - logger.debug("getActiveStaticHosts(): Found {} Hosts", list.size()); return list; } @Override public Set getInactiveStaticHosts() { - Set list = new HashSet(); - for (Entry entry : inactiveStaticHosts.entrySet()) { - list.add(entry.getValue()); - } - logger.debug("getInactiveStaticHosts(): Found {} Hosts", list.size()); + Set list = new HashSet(inactiveStaticHosts.values()); return list; } @@ -549,7 +539,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw ta.notifyHTClientHostRemoved(host); } } catch (Exception e) { - logger.error("Exception on callback", e); + logger.error("Exception on new host notification", e); } } } @@ -1034,7 +1024,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw * probe. However, continue the age out the hosts since * we don't know if the host is indeed out there or not. */ - logger.warn("ARPHandler is not avaialable, can't send the probe"); + logger.trace("ARPHandler is not avaialable, can't send the probe"); continue; } hostFinder.probe(host); @@ -1062,7 +1052,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw * indicating the result of this action. */ - public Status addStaticHostReq(InetAddress networkAddr, byte[] dataLayerAddress, NodeConnector nc, short vlan) { + protected Status addStaticHostReq(InetAddress networkAddr, byte[] dataLayerAddress, NodeConnector nc, short vlan) { if (dataLayerAddress.length != NetUtils.MACAddrLengthInBytes) { return new Status(StatusCode.BADREQUEST, "Invalid MAC address"); } @@ -1078,13 +1068,13 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw // northbound request HostNodeConnector transHost = hostsDB.get(networkAddr); transHost.setStaticHost(true); - return new Status(StatusCode.SUCCESS, null); + return new Status(StatusCode.SUCCESS); } if (hostsDB.get(networkAddr) != null) { // There is already a host with this IP address (but behind // a different (switch, port, vlan) tuple. Return an error - return new Status(StatusCode.CONFLICT, "Existing IP, Use PUT to update"); + return new Status(StatusCode.CONFLICT, "Host with this IP already exists."); } host.setStaticHost(true); /* @@ -1108,7 +1098,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw logger.debug("Switch or switchport is not up, adding host {} to inactive list", networkAddr.getHostName()); } - return new Status(StatusCode.SUCCESS, null); + return new Status(StatusCode.SUCCESS); } catch (ConstructionException e) { logger.error("", e); return new Status(StatusCode.INTERNALERROR, "Host could not be created"); @@ -1291,25 +1281,33 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw public Status addStaticHost(String networkAddress, String dataLayerAddress, NodeConnector nc, String vlan) { try { InetAddress ip = InetAddress.getByName(networkAddress); - if (nc == null) { - return new Status(StatusCode.BADREQUEST, "Invalid NodeId"); + short vl = 0; + if (vlan != null && !vlan.isEmpty()) { + vl = Short.decode(vlan); + if (vl < 1 || vl > 4095) { + return new Status(StatusCode.BADREQUEST, "Host vlan out of range [1 - 4095]"); + } } - return addStaticHostReq(ip, HexEncode.bytesFromHexString(dataLayerAddress), nc, Short.valueOf(vlan)); + + return addStaticHostReq(ip, HexEncode.bytesFromHexString(dataLayerAddress), nc, vl); + } catch (UnknownHostException e) { - logger.error("", e); - return new Status(StatusCode.BADREQUEST, "Invalid Address"); + logger.debug("Invalid host IP specified when adding static host", e); + return new Status(StatusCode.BADREQUEST, "Invalid Host IP Address"); + } catch (NumberFormatException nfe) { + logger.debug("Invalid host vlan or MAC specified when adding static host", nfe); + return new Status(StatusCode.BADREQUEST, "Invalid Host vLan/MAC"); } } @Override public Status removeStaticHost(String networkAddress) { - InetAddress address; try { - address = InetAddress.getByName(networkAddress); + InetAddress address = InetAddress.getByName(networkAddress); return removeStaticHostReq(address); } catch (UnknownHostException e) { - logger.error("", e); - return new Status(StatusCode.BADREQUEST, "Invalid Address"); + logger.debug("Invalid IP Address when trying to remove host", e); + return new Status(StatusCode.BADREQUEST, "Invalid IP Address when trying to remove host"); } } @@ -1317,11 +1315,11 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw ARPPending arphost; HostNodeConnector host = null; - logger.debug("handleNodeConnectorStatusUp {}", nodeConnector); + logger.trace("handleNodeConnectorStatusUp {}", nodeConnector); for (Entry entry : failedARPReqList.entrySet()) { arphost = entry.getValue(); - logger.debug("Sending the ARP from FailedARPReqList fors IP: {}", arphost.getHostIP().getHostAddress()); + logger.trace("Sending the ARP from FailedARPReqList fors IP: {}", arphost.getHostIP().getHostAddress()); if (hostFinder == null) { logger.warn("ARPHandler is not available at interface up"); logger.warn("Since this event is missed, host(s) connected to interface {} may not be discovered", @@ -1340,7 +1338,6 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw arphost.getHostIP(), nodeConnector); logger.error("", e); } - logger.debug("Done. handleNodeConnectorStatusUp {}", nodeConnector); } host = inactiveStaticHosts.get(nodeConnector); @@ -1353,7 +1350,7 @@ public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAw } private void handleNodeConnectorStatusDown(NodeConnector nodeConnector) { - logger.debug("handleNodeConnectorStatusDown {}", nodeConnector); + logger.trace("handleNodeConnectorStatusDown {}", nodeConnector); for (Entry entry : hostsDB.entrySet()) { HostNodeConnector host = entry.getValue(); diff --git a/opendaylight/hosttracker/integrationtest/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerIT.java b/opendaylight/hosttracker/integrationtest/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerIT.java index 3734a63e56..aa029a2d7b 100644 --- a/opendaylight/hosttracker/integrationtest/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerIT.java +++ b/opendaylight/hosttracker/integrationtest/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerIT.java @@ -174,9 +174,9 @@ public class HostTrackerIT { NodeConnector nc1_2 = NodeConnectorCreator.createOFNodeConnector((short) 2, node1); // test addStaticHost(), store into inactive host DB - Status st = this.hosttracker.addStaticHost("192.168.0.8", "11:22:33:44:55:66", nc1_1, "0"); + Status st = this.hosttracker.addStaticHost("192.168.0.8", "11:22:33:44:55:66", nc1_1, null); Assert.assertTrue(st.isSuccess()); - st = this.hosttracker.addStaticHost("192.168.0.13", "11:22:33:44:55:77", nc1_2, "0"); + st = this.hosttracker.addStaticHost("192.168.0.13", "11:22:33:44:55:77", nc1_2, ""); Assert.assertTrue(st.isSuccess()); // check inactive DB @@ -213,8 +213,11 @@ public class HostTrackerIT { NodeConnector nc1_2 = NodeConnectorCreator.createOFNodeConnector((short) 2, node1); // test addStaticHost(), put into inactive host DB if not verifiable - Status st = this.hosttracker.addStaticHost("192.168.0.8", "11:22:33:44:55:66", nc1_1, "0"); + Status st = this.hosttracker.addStaticHost("192.168.0.8", "11:22:33:44:55:66", nc1_1, null); + Assert.assertTrue(st.isSuccess()); st = this.hosttracker.addStaticHost("192.168.0.13", "11:22:33:44:55:77", nc1_2, "0"); + Assert.assertFalse(st.isSuccess()); + this.invtoryListener.notifyNodeConnector(nc1_1, UpdateType.ADDED, null); @@ -251,8 +254,8 @@ public class HostTrackerIT { NodeConnector nc1_2 = NodeConnectorCreator.createOFNodeConnector((short) 2, node1); // test addStaticHost(), put into inactive host DB if not verifiable - Status st = this.hosttracker.addStaticHost("192.168.0.8", "11:22:33:44:55:66", nc1_1, "0"); - st = this.hosttracker.addStaticHost("192.168.0.13", "11:22:33:44:55:77", nc1_2, "0"); + Status st = this.hosttracker.addStaticHost("192.168.0.8", "11:22:33:44:55:66", nc1_1, null); + st = this.hosttracker.addStaticHost("192.168.0.13", "11:22:33:44:55:77", nc1_2, ""); HostNodeConnector hnc_1 = this.hosttracker.hostFind(InetAddress.getByName("192.168.0.8")); assertNull(hnc_1); diff --git a/opendaylight/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/RestMessages.java b/opendaylight/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/RestMessages.java index 668efbc7ee..ba2476a137 100644 --- a/opendaylight/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/RestMessages.java +++ b/opendaylight/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/RestMessages.java @@ -15,7 +15,7 @@ public enum RestMessages { "Operation failed due to Resource Conflict"), NODEFAULT("Container default is not a custom container"), DEFAULTDISABLED( "Container(s) are configured. Container default is not operational"), NOTALLOWEDONDEFAULT( "Container default is a static resource, no modification allowed on it"), UNKNOWNACTION("Unknown action"), INVALIDJSON( - "JSON message is invalid"), INVALIDADDRESS("invalid InetAddress"), AVAILABLESOON( + "JSON message is invalid"), INVALIDADDRESS("Invalid InetAddress"), AVAILABLESOON( "Resource is not implemented yet"), INTERNALERROR("Internal Error"), SERVICEUNAVAILABLE( "Service is not available. Could be down for maintanence"), INVALIDDATA( "Data is invalid or conflicts with URI"); diff --git a/opendaylight/northbound/hosttracker/src/main/java/org/opendaylight/controller/hosttracker/northbound/HostTrackerNorthbound.java b/opendaylight/northbound/hosttracker/src/main/java/org/opendaylight/controller/hosttracker/northbound/HostTrackerNorthbound.java index 769461167c..91185bb405 100644 --- a/opendaylight/northbound/hosttracker/src/main/java/org/opendaylight/controller/hosttracker/northbound/HostTrackerNorthbound.java +++ b/opendaylight/northbound/hosttracker/src/main/java/org/opendaylight/controller/hosttracker/northbound/HostTrackerNorthbound.java @@ -25,6 +25,7 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.SecurityContext; +import javax.ws.rs.core.UriInfo; import javax.xml.bind.JAXBElement; import org.codehaus.enunciate.jaxrs.ResponseCode; @@ -34,12 +35,11 @@ import org.opendaylight.controller.containermanager.IContainerManager; import org.opendaylight.controller.hosttracker.IfIptoHost; import org.opendaylight.controller.hosttracker.hostAware.HostNodeConnector; import org.opendaylight.controller.northbound.commons.RestMessages; -import org.opendaylight.controller.northbound.commons.exception.InternalServerErrorException; +import org.opendaylight.controller.northbound.commons.exception.BadRequestException; import org.opendaylight.controller.northbound.commons.exception.ResourceConflictException; import org.opendaylight.controller.northbound.commons.exception.ResourceNotFoundException; import org.opendaylight.controller.northbound.commons.exception.ServiceUnavailableException; import org.opendaylight.controller.northbound.commons.exception.UnauthorizedException; -import org.opendaylight.controller.northbound.commons.exception.UnsupportedMediaTypeException; import org.opendaylight.controller.northbound.commons.utils.NorthboundUtils; import org.opendaylight.controller.sal.authorization.Privilege; import org.opendaylight.controller.sal.core.Node; @@ -47,7 +47,6 @@ import org.opendaylight.controller.sal.core.NodeConnector; import org.opendaylight.controller.sal.utils.GlobalConstants; import org.opendaylight.controller.sal.utils.ServiceHelper; import org.opendaylight.controller.sal.utils.Status; -import org.opendaylight.controller.sal.utils.StatusCode; import org.opendaylight.controller.switchmanager.ISwitchManager; /** @@ -86,11 +85,10 @@ public class HostTrackerNorthbound { } private IfIptoHost getIfIpToHostService(String containerName) { - IContainerManager containerManager = (IContainerManager) ServiceHelper - .getGlobalInstance(IContainerManager.class, this); + IContainerManager containerManager = (IContainerManager) ServiceHelper.getGlobalInstance( + IContainerManager.class, this); if (containerManager == null) { - throw new ServiceUnavailableException("Container " - + RestMessages.SERVICEUNAVAILABLE.toString()); + throw new ServiceUnavailableException("Container " + RestMessages.SERVICEUNAVAILABLE.toString()); } boolean found = false; @@ -98,20 +96,17 @@ public class HostTrackerNorthbound { for (String cName : containerNames) { if (cName.trim().equalsIgnoreCase(containerName.trim())) { found = true; + break; } } - if (found == false) { - throw new ResourceNotFoundException(containerName + " " - + RestMessages.NOCONTAINER.toString()); + if (!found) { + throw new ResourceNotFoundException(containerName + " " + RestMessages.NOCONTAINER.toString()); } - IfIptoHost hostTracker = (IfIptoHost) ServiceHelper.getInstance( - IfIptoHost.class, containerName, this); - + IfIptoHost hostTracker = (IfIptoHost) ServiceHelper.getInstance(IfIptoHost.class, containerName, this); if (hostTracker == null) { - throw new ServiceUnavailableException("Host Tracker " - + RestMessages.SERVICEUNAVAILABLE.toString()); + throw new ServiceUnavailableException("Host Tracker " + RestMessages.SERVICEUNAVAILABLE.toString()); } return hostTracker; @@ -207,17 +202,11 @@ public class HostTrackerNorthbound { @ResponseCode(code = 503, condition = "One or more of Controller Services are unavailable") }) public Hosts getActiveHosts(@PathParam("containerName") String containerName) { - if (!NorthboundUtils.isAuthorized( - getUserName(), containerName, Privilege.READ, this)) { - throw new UnauthorizedException( - "User is not authorized to perform this operation on container " - + containerName); + if (!NorthboundUtils.isAuthorized(getUserName(), containerName, Privilege.READ, this)) { + throw new UnauthorizedException("User is not authorized to perform this operation on container " + + containerName); } IfIptoHost hostTracker = getIfIpToHostService(containerName); - if (hostTracker == null) { - throw new ServiceUnavailableException("Host Tracker " - + RestMessages.SERVICEUNAVAILABLE.toString()); - } return convertHosts(hostTracker.getAllHosts()); } @@ -300,17 +289,11 @@ public class HostTrackerNorthbound { @ResponseCode(code = 503, condition = "One or more of Controller Services are unavailable") }) public Hosts getInactiveHosts( @PathParam("containerName") String containerName) { - if (!NorthboundUtils.isAuthorized( - getUserName(), containerName, Privilege.READ, this)) { - throw new UnauthorizedException( - "User is not authorized to perform this operation on container " - + containerName); + if (!NorthboundUtils.isAuthorized(getUserName(), containerName, Privilege.READ, this)) { + throw new UnauthorizedException("User is not authorized to perform this operation on container " + + containerName); } IfIptoHost hostTracker = getIfIpToHostService(containerName); - if (hostTracker == null) { - throw new ServiceUnavailableException("Host Tracker " - + RestMessages.SERVICEUNAVAILABLE.toString()); - } return convertHosts(hostTracker.getInactiveStaticHosts()); } @@ -364,30 +347,23 @@ public class HostTrackerNorthbound { @TypeHint(HostConfig.class) @StatusCodes({ @ResponseCode(code = 200, condition = "Operation successful"), + @ResponseCode(code = 400, condition = "Invalid IP specified in networkAddress parameter"), @ResponseCode(code = 404, condition = "The containerName is not found"), - @ResponseCode(code = 415, condition = "Invalid IP Address passed in networkAddress parameter"), @ResponseCode(code = 503, condition = "One or more of Controller Services are unavailable") }) public HostConfig getHostDetails( @PathParam("containerName") String containerName, @PathParam("networkAddress") String networkAddress) { - if (!NorthboundUtils.isAuthorized( - getUserName(), containerName, Privilege.READ, this)) { - throw new UnauthorizedException( - "User is not authorized to perform this operation on container " - + containerName); + if (!NorthboundUtils.isAuthorized(getUserName(), containerName, Privilege.READ, this)) { + throw new UnauthorizedException("User is not authorized to perform this operation on container " + + containerName); } IfIptoHost hostTracker = getIfIpToHostService(containerName); - if (hostTracker == null) { - throw new ServiceUnavailableException("Host Tracker " - + RestMessages.SERVICEUNAVAILABLE.toString()); - } InetAddress ip; try { ip = InetAddress.getByName(networkAddress); } catch (UnknownHostException e) { - throw new UnsupportedMediaTypeException(networkAddress + " " - + RestMessages.INVALIDADDRESS.toString()); + throw new BadRequestException(RestMessages.INVALIDADDRESS.toString() + " " + networkAddress); } for (HostNodeConnector host : hostTracker.getAllHosts()) { if (host.getNetworkAddress().equals(ip)) { @@ -450,63 +426,44 @@ public class HostTrackerNorthbound { @Consumes({ MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML }) @StatusCodes({ @ResponseCode(code = 201, condition = "Static host created successfully"), - @ResponseCode(code = 404, condition = "The Container Name or nodeId or configuration name is not found"), - @ResponseCode(code = 406, condition = "Cannot operate on Default Container when other Containers are active"), - @ResponseCode(code = 415, condition = "Invalid IP Address passed in networkAddress parameter"), - @ResponseCode(code = 500, condition = "Failed to create Static Host entry. Failure Reason included in HTTP Error response"), + @ResponseCode(code = 400, condition = "Invalid parameters specified, see response body for details"), + @ResponseCode(code = 404, condition = "The container or resource is not found"), + @ResponseCode(code = 409, condition = "Resource conflict, see response body for details"), @ResponseCode(code = 503, condition = "One or more of Controller services are unavailable") }) - public Response addHost(@PathParam("containerName") String containerName, + public Response addHost(@Context UriInfo uriInfo, @PathParam("containerName") String containerName, @PathParam("networkAddress") String networkAddress, @TypeHint(HostConfig.class) JAXBElement hostConfig) { - if (!NorthboundUtils.isAuthorized( - getUserName(), containerName, Privilege.WRITE, this)) { - throw new UnauthorizedException( - "User is not authorized to perform this operation on container " - + containerName); + if (!NorthboundUtils.isAuthorized(getUserName(), containerName, Privilege.WRITE, this)) { + return Response.status(Response.Status.UNAUTHORIZED) + .entity("User is not authorized to perform this operation on container " + containerName) + .build(); } handleDefaultDisabled(containerName); IfIptoHost hostTracker = getIfIpToHostService(containerName); - if (hostTracker == null) { - throw new ServiceUnavailableException("Host Tracker " - + RestMessages.SERVICEUNAVAILABLE.toString()); - } HostConfig hc = hostConfig.getValue(); - Node node = handleNodeAvailability(containerName, hc.getNodeType(), hc.getNodeId()); - if (node == null) { - throw new InternalServerErrorException( - RestMessages.NONODE.toString()); + if (!networkAddress.equals(hc.getNetworkAddress())) { + return Response.status(Response.Status.CONFLICT) + .entity("Resource name in config object doesn't match URI") + .build(); } - - try { - InetAddress.getByName(networkAddress); - } catch (UnknownHostException e) { - throw new UnsupportedMediaTypeException(networkAddress + " " - + RestMessages.INVALIDADDRESS.toString()); - } - if(!networkAddress.equals(hc.getNetworkAddress())) { - throw new UnsupportedMediaTypeException(networkAddress + " is not the same as " - + hc.getNetworkAddress()); - } - if(!hc.isStaticHost()) { - throw new UnsupportedMediaTypeException("StaticHost flag must be true"); + if (!hc.isStaticHost()) { + return Response.status(Response.Status.BAD_REQUEST) + .entity("Can only add static host.") + .build(); } + Node node = handleNodeAvailability(containerName, hc.getNodeType(), hc.getNodeId()); NodeConnector nc = NodeConnector.fromStringNoNode(hc.getNodeConnectorType(), hc.getNodeConnectorId(), node); - if (nc == null) { - throw new ResourceNotFoundException(hc.getNodeConnectorType() + "|" - + hc.getNodeConnectorId() + " : " + RestMessages.NONODE.toString()); - } - Status status = hostTracker.addStaticHost(networkAddress, - hc.getDataLayerAddress(), nc, hc.getVlan()); + + Status status = hostTracker.addStaticHost(networkAddress, hc.getDataLayerAddress(), nc, hc.getVlan()); if (status.isSuccess()) { NorthboundUtils.auditlog("Static Host", username, "added", networkAddress, containerName); - return Response.status(Response.Status.CREATED).build(); - } else if (status.getCode().equals(StatusCode.BADREQUEST)) { - throw new UnsupportedMediaTypeException(status.getDescription()); + return Response.created(uriInfo.getRequestUri()).build(); } - throw new InternalServerErrorException(status.getDescription()); + + return NorthboundUtils.getResponse(status); } /** @@ -524,42 +481,28 @@ public class HostTrackerNorthbound { @DELETE @Consumes({ MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML }) @StatusCodes({ - @ResponseCode(code = 200, condition = "Flow Config deleted successfully"), - @ResponseCode(code = 404, condition = "The Container Name or Node-id or Flow Name passed is not found"), + @ResponseCode(code = 204, condition = "Static host deleted successfully"), + @ResponseCode(code = 404, condition = "The container or a specified resource was not found"), @ResponseCode(code = 406, condition = "Cannot operate on Default Container when other Containers are active"), - @ResponseCode(code = 415, condition = "Invalid IP Address passed in networkAddress parameter"), - @ResponseCode(code = 500, condition = "Failed to delete Flow config. Failure Reason included in HTTP Error response"), @ResponseCode(code = 503, condition = "One or more of Controller service is unavailable") }) - public Response deleteFlow( + public Response deleteHost( @PathParam(value = "containerName") String containerName, @PathParam(value = "networkAddress") String networkAddress) { - if (!NorthboundUtils.isAuthorized( - getUserName(), containerName, Privilege.WRITE, this)) { - throw new UnauthorizedException( - "User is not authorized to perform this operation on container " - + containerName); + if (!NorthboundUtils.isAuthorized(getUserName(), containerName, Privilege.WRITE, this)) { + return Response.status(Response.Status.UNAUTHORIZED) + .entity("User is not authorized to perform this operation on container " + containerName) + .build(); } handleDefaultDisabled(containerName); IfIptoHost hostTracker = getIfIpToHostService(containerName); - if (hostTracker == null) { - throw new ServiceUnavailableException("Host Tracker " - + RestMessages.SERVICEUNAVAILABLE.toString()); - } - - try { - InetAddress.getByName(networkAddress); - } catch (UnknownHostException e) { - throw new UnsupportedMediaTypeException(networkAddress + " " - + RestMessages.INVALIDADDRESS.toString()); - } Status status = hostTracker.removeStaticHost(networkAddress); if (status.isSuccess()) { NorthboundUtils.auditlog("Static Host", username, "removed", networkAddress, containerName); - return Response.ok().build(); + return Response.noContent().build(); } - throw new InternalServerErrorException(status.getDescription()); + return NorthboundUtils.getResponse(status); } @@ -567,8 +510,8 @@ public class HostTrackerNorthbound { IContainerManager containerManager = (IContainerManager) ServiceHelper .getGlobalInstance(IContainerManager.class, this); if (containerManager == null) { - throw new InternalServerErrorException( - RestMessages.INTERNALERROR.toString()); + throw new ServiceUnavailableException( + RestMessages.SERVICEUNAVAILABLE.toString()); } if (containerName.equals(GlobalConstants.DEFAULT.toString()) && containerManager.hasNonDefaultContainer()) { @@ -577,8 +520,7 @@ public class HostTrackerNorthbound { } } - private Node handleNodeAvailability(String containerName, String nodeType, - String nodeId) { + private Node handleNodeAvailability(String containerName, String nodeType, String nodeId) { Node node = Node.fromString(nodeType, nodeId); if (node == null) { diff --git a/opendaylight/northbound/integrationtest/src/test/java/org/opendaylight/controller/northbound/integrationtest/NorthboundIT.java b/opendaylight/northbound/integrationtest/src/test/java/org/opendaylight/controller/northbound/integrationtest/NorthboundIT.java index f4adc71fa2..814731196c 100644 --- a/opendaylight/northbound/integrationtest/src/test/java/org/opendaylight/controller/northbound/integrationtest/NorthboundIT.java +++ b/opendaylight/northbound/integrationtest/src/test/java/org/opendaylight/controller/northbound/integrationtest/NorthboundIT.java @@ -903,7 +903,7 @@ public class NorthboundIT { Integer nodeId_1 = 3366; String nodeConnectorType_1 = "STUB"; Integer nodeConnectorId_1 = 12; - String vlan_1 = "4"; + String vlan_1 = ""; // 2nd host String networkAddress_2 = "10.1.1.1"; @@ -912,7 +912,7 @@ public class NorthboundIT { Integer nodeId_2 = 4477; String nodeConnectorType_2 = "STUB"; Integer nodeConnectorId_2 = 34; - String vlan_2 = "0"; + String vlan_2 = "123"; String baseURL = "http://127.0.0.1:8080/controller/nb/v2/host/default"; @@ -969,7 +969,7 @@ public class NorthboundIT { Assert.assertTrue(host_jo.getInt("nodeConnectorId") == nodeConnectorId_1); Assert.assertTrue(host_jo.getString("nodeType").equalsIgnoreCase(nodeType_1)); Assert.assertTrue(host_jo.getInt("nodeId") == nodeId_1); - Assert.assertTrue(host_jo.getString("vlan").equalsIgnoreCase(vlan_1)); + Assert.assertTrue(host_jo.getString("vlan").equals("0")); Assert.assertTrue(host_jo.getBoolean("staticHost")); } else if (networkAddress.equalsIgnoreCase(networkAddress_2)) { Assert.assertTrue(host_jo.getString("dataLayerAddress").equalsIgnoreCase(dataLayerAddress_2)); @@ -1030,13 +1030,13 @@ public class NorthboundIT { Assert.assertTrue(json.getInt("nodeConnectorId") == nodeConnectorId_1); Assert.assertTrue(json.getString("nodeType").equalsIgnoreCase(nodeType_1)); Assert.assertTrue(json.getInt("nodeId") == nodeId_1); - Assert.assertTrue(json.getString("vlan").equalsIgnoreCase(vlan_1)); + Assert.assertTrue(json.getString("vlan").equals("0")); Assert.assertTrue(json.getBoolean("staticHost")); // test DELETE method for deleteFlow() result = getJsonResult(baseURL + "/" + networkAddress_1, "DELETE"); - Assert.assertTrue(httpResponseCode == 200); + Assert.assertTrue(httpResponseCode == 204); // verify host_1 removed from active host DB // test GET method: getActiveHosts() - no host expected -- 2.36.6