Committed by
Gerrit Code Review
[ONOS-3918] Handling of NETCONF <rpc-error> and no message-id
Change-Id: I8b9396a727fb54b5b84d02f258c14cfccad5bb99
Showing
3 changed files
with
43 additions
and
27 deletions
| ... | @@ -18,6 +18,8 @@ package org.onosproject.netconf; | ... | @@ -18,6 +18,8 @@ package org.onosproject.netconf; |
| 18 | 18 | ||
| 19 | import org.onosproject.event.AbstractEvent; | 19 | import org.onosproject.event.AbstractEvent; |
| 20 | 20 | ||
| 21 | +import java.util.Optional; | ||
| 22 | + | ||
| 21 | /** | 23 | /** |
| 22 | * Describes network configuration event. | 24 | * Describes network configuration event. |
| 23 | */ | 25 | */ |
| ... | @@ -25,7 +27,7 @@ public final class NetconfDeviceOutputEvent extends | ... | @@ -25,7 +27,7 @@ public final class NetconfDeviceOutputEvent extends |
| 25 | AbstractEvent<NetconfDeviceOutputEvent.Type, Object> { | 27 | AbstractEvent<NetconfDeviceOutputEvent.Type, Object> { |
| 26 | 28 | ||
| 27 | private final String messagePayload; | 29 | private final String messagePayload; |
| 28 | - private final int messageID; | 30 | + private final Optional<Integer> messageID; |
| 29 | private final NetconfDeviceInfo deviceInfo; | 31 | private final NetconfDeviceInfo deviceInfo; |
| 30 | 32 | ||
| 31 | /** | 33 | /** |
| ... | @@ -64,7 +66,8 @@ public final class NetconfDeviceOutputEvent extends | ... | @@ -64,7 +66,8 @@ public final class NetconfDeviceOutputEvent extends |
| 64 | * @param msgID id of the message related to the event | 66 | * @param msgID id of the message related to the event |
| 65 | * @param netconfDeviceInfo device of event | 67 | * @param netconfDeviceInfo device of event |
| 66 | */ | 68 | */ |
| 67 | - public NetconfDeviceOutputEvent(Type type, Object subject, String payload, int msgID, | 69 | + public NetconfDeviceOutputEvent(Type type, Object subject, String payload, |
| 70 | + Optional<Integer> msgID, | ||
| 68 | NetconfDeviceInfo netconfDeviceInfo) { | 71 | NetconfDeviceInfo netconfDeviceInfo) { |
| 69 | super(type, subject); | 72 | super(type, subject); |
| 70 | messagePayload = payload; | 73 | messagePayload = payload; |
| ... | @@ -83,8 +86,10 @@ public final class NetconfDeviceOutputEvent extends | ... | @@ -83,8 +86,10 @@ public final class NetconfDeviceOutputEvent extends |
| 83 | * @param msgID id of the message related to the event | 86 | * @param msgID id of the message related to the event |
| 84 | * @param time occurrence time | 87 | * @param time occurrence time |
| 85 | */ | 88 | */ |
| 86 | - public NetconfDeviceOutputEvent(Type type, Object subject, String payload, int msgID, | 89 | + public NetconfDeviceOutputEvent(Type type, Object subject, String payload, |
| 87 | - NetconfDeviceInfo netconfDeviceInfo, long time) { | 90 | + Optional<Integer> msgID, |
| 91 | + NetconfDeviceInfo netconfDeviceInfo, | ||
| 92 | + long time) { | ||
| 88 | super(type, subject, time); | 93 | super(type, subject, time); |
| 89 | messagePayload = payload; | 94 | messagePayload = payload; |
| 90 | deviceInfo = netconfDeviceInfo; | 95 | deviceInfo = netconfDeviceInfo; |
| ... | @@ -111,7 +116,7 @@ public final class NetconfDeviceOutputEvent extends | ... | @@ -111,7 +116,7 @@ public final class NetconfDeviceOutputEvent extends |
| 111 | * Reply messageId. | 116 | * Reply messageId. |
| 112 | * @return messageId | 117 | * @return messageId |
| 113 | */ | 118 | */ |
| 114 | - public Integer getMessageID() { | 119 | + public Optional<Integer> getMessageID() { |
| 115 | return messageID; | 120 | return messageID; |
| 116 | } | 121 | } |
| 117 | } | 122 | } | ... | ... |
| ... | @@ -28,10 +28,12 @@ import org.slf4j.Logger; | ... | @@ -28,10 +28,12 @@ import org.slf4j.Logger; |
| 28 | import org.slf4j.LoggerFactory; | 28 | import org.slf4j.LoggerFactory; |
| 29 | 29 | ||
| 30 | import java.io.IOException; | 30 | import java.io.IOException; |
| 31 | +import java.util.ArrayList; | ||
| 31 | import java.util.Collections; | 32 | import java.util.Collections; |
| 32 | import java.util.HashMap; | 33 | import java.util.HashMap; |
| 33 | import java.util.List; | 34 | import java.util.List; |
| 34 | import java.util.Map; | 35 | import java.util.Map; |
| 36 | +import java.util.Optional; | ||
| 35 | import java.util.concurrent.CompletableFuture; | 37 | import java.util.concurrent.CompletableFuture; |
| 36 | import java.util.concurrent.ExecutionException; | 38 | import java.util.concurrent.ExecutionException; |
| 37 | import java.util.concurrent.TimeUnit; | 39 | import java.util.concurrent.TimeUnit; |
| ... | @@ -71,12 +73,14 @@ public class NetconfSessionImpl implements NetconfSession { | ... | @@ -71,12 +73,14 @@ public class NetconfSessionImpl implements NetconfSession { |
| 71 | private String serverCapabilities; | 73 | private String serverCapabilities; |
| 72 | private NetconfStreamHandler t; | 74 | private NetconfStreamHandler t; |
| 73 | private Map<Integer, CompletableFuture<String>> replies; | 75 | private Map<Integer, CompletableFuture<String>> replies; |
| 76 | + private List<String> errorReplies; | ||
| 74 | 77 | ||
| 75 | 78 | ||
| 76 | public NetconfSessionImpl(NetconfDeviceInfo deviceInfo) throws NetconfException { | 79 | public NetconfSessionImpl(NetconfDeviceInfo deviceInfo) throws NetconfException { |
| 77 | this.deviceInfo = deviceInfo; | 80 | this.deviceInfo = deviceInfo; |
| 78 | connectionActive = false; | 81 | connectionActive = false; |
| 79 | replies = new HashMap<>(); | 82 | replies = new HashMap<>(); |
| 83 | + errorReplies = new ArrayList<>(); | ||
| 80 | startConnection(); | 84 | startConnection(); |
| 81 | } | 85 | } |
| 82 | 86 | ||
| ... | @@ -194,18 +198,13 @@ public class NetconfSessionImpl implements NetconfSession { | ... | @@ -194,18 +198,13 @@ public class NetconfSessionImpl implements NetconfSession { |
| 194 | request = formatRequestMessageId(request); | 198 | request = formatRequestMessageId(request); |
| 195 | request = formatXmlHeader(request); | 199 | request = formatXmlHeader(request); |
| 196 | CompletableFuture<String> futureReply = request(request); | 200 | CompletableFuture<String> futureReply = request(request); |
| 201 | + messageIdInteger.incrementAndGet(); | ||
| 197 | String rp; | 202 | String rp; |
| 198 | try { | 203 | try { |
| 199 | rp = futureReply.get(FUTURE_REPLY_TIMEOUT, TimeUnit.MILLISECONDS); | 204 | rp = futureReply.get(FUTURE_REPLY_TIMEOUT, TimeUnit.MILLISECONDS); |
| 200 | } catch (InterruptedException | ExecutionException | TimeoutException e) { | 205 | } catch (InterruptedException | ExecutionException | TimeoutException e) { |
| 201 | - //replies.remove(messageIdInteger.get()); | 206 | + throw new NetconfException("No matching reply for request " + request, e); |
| 202 | - throw new NetconfException("Can't get the reply for request" + request, e); | ||
| 203 | } | 207 | } |
| 204 | -// String rp = Tools.futureGetOrElse(futureReply, FUTURE_REPLY_TIMEOUT, TimeUnit.MILLISECONDS, | ||
| 205 | -// "Error in completing the request with message-id " + | ||
| 206 | -// messageIdInteger.get() + | ||
| 207 | -// ": future timed out."); | ||
| 208 | - messageIdInteger.incrementAndGet(); | ||
| 209 | log.debug("Result {} from request {} to device {}", rp, request, deviceInfo); | 208 | log.debug("Result {} from request {} to device {}", rp, request, deviceInfo); |
| 210 | return rp; | 209 | return rp; |
| 211 | } | 210 | } |
| ... | @@ -442,8 +441,16 @@ public class NetconfSessionImpl implements NetconfSession { | ... | @@ -442,8 +441,16 @@ public class NetconfSessionImpl implements NetconfSession { |
| 442 | public class NetconfSessionDelegateImpl implements NetconfSessionDelegate { | 441 | public class NetconfSessionDelegateImpl implements NetconfSessionDelegate { |
| 443 | 442 | ||
| 444 | @Override | 443 | @Override |
| 445 | - public void notify(NetconfDeviceOutputEvent event) { | 444 | + public void notify(NetconfDeviceOutputEvent event) { |
| 446 | - CompletableFuture<String> completedReply = replies.get(event.getMessageID()); | 445 | + Optional<Integer> messageId = event.getMessageID(); |
| 446 | + if (!messageId.isPresent()) { | ||
| 447 | + errorReplies.add(event.getMessagePayload()); | ||
| 448 | + log.error("Device " + event.getDeviceInfo() + | ||
| 449 | + " sent error reply " + event.getMessagePayload()); | ||
| 450 | + return; | ||
| 451 | + } | ||
| 452 | + CompletableFuture<String> completedReply = | ||
| 453 | + replies.get(messageId.get()); | ||
| 447 | completedReply.complete(event.getMessagePayload()); | 454 | completedReply.complete(event.getMessagePayload()); |
| 448 | } | 455 | } |
| 449 | } | 456 | } | ... | ... |
| ... | @@ -32,6 +32,7 @@ import java.io.InputStreamReader; | ... | @@ -32,6 +32,7 @@ import java.io.InputStreamReader; |
| 32 | import java.io.OutputStream; | 32 | import java.io.OutputStream; |
| 33 | import java.io.PrintWriter; | 33 | import java.io.PrintWriter; |
| 34 | import java.util.List; | 34 | import java.util.List; |
| 35 | +import java.util.Optional; | ||
| 35 | import java.util.concurrent.CompletableFuture; | 36 | import java.util.concurrent.CompletableFuture; |
| 36 | 37 | ||
| 37 | /** | 38 | /** |
| ... | @@ -48,6 +49,7 @@ public class NetconfStreamThread extends Thread implements NetconfStreamHandler | ... | @@ -48,6 +49,7 @@ public class NetconfStreamThread extends Thread implements NetconfStreamHandler |
| 48 | private static final String RPC_REPLY = "rpc-reply"; | 49 | private static final String RPC_REPLY = "rpc-reply"; |
| 49 | private static final String RPC_ERROR = "rpc-error"; | 50 | private static final String RPC_ERROR = "rpc-error"; |
| 50 | private static final String NOTIFICATION_LABEL = "<notification>"; | 51 | private static final String NOTIFICATION_LABEL = "<notification>"; |
| 52 | + private static final String MESSAGE_ID = "message-id="; | ||
| 51 | 53 | ||
| 52 | private PrintWriter outputStream; | 54 | private PrintWriter outputStream; |
| 53 | private final InputStream err; | 55 | private final InputStream err; |
| ... | @@ -163,7 +165,7 @@ public class NetconfStreamThread extends Thread implements NetconfStreamHandler | ... | @@ -163,7 +165,7 @@ public class NetconfStreamThread extends Thread implements NetconfStreamHandler |
| 163 | log.debug("char {} " + bufferReader.read()); | 165 | log.debug("char {} " + bufferReader.read()); |
| 164 | NetconfDeviceOutputEvent event = new NetconfDeviceOutputEvent( | 166 | NetconfDeviceOutputEvent event = new NetconfDeviceOutputEvent( |
| 165 | NetconfDeviceOutputEvent.Type.DEVICE_UNREGISTERED, | 167 | NetconfDeviceOutputEvent.Type.DEVICE_UNREGISTERED, |
| 166 | - null, null, -1, netconfDeviceInfo); | 168 | + null, null, Optional.of(-1), netconfDeviceInfo); |
| 167 | netconfDeviceEventListeners.forEach( | 169 | netconfDeviceEventListeners.forEach( |
| 168 | listener -> listener.event(event)); | 170 | listener -> listener.event(event)); |
| 169 | } | 171 | } |
| ... | @@ -175,7 +177,7 @@ public class NetconfStreamThread extends Thread implements NetconfStreamHandler | ... | @@ -175,7 +177,7 @@ public class NetconfStreamThread extends Thread implements NetconfStreamHandler |
| 175 | if (deviceReply.equals(END_PATTERN)) { | 177 | if (deviceReply.equals(END_PATTERN)) { |
| 176 | NetconfDeviceOutputEvent event = new NetconfDeviceOutputEvent( | 178 | NetconfDeviceOutputEvent event = new NetconfDeviceOutputEvent( |
| 177 | NetconfDeviceOutputEvent.Type.DEVICE_UNREGISTERED, | 179 | NetconfDeviceOutputEvent.Type.DEVICE_UNREGISTERED, |
| 178 | - null, null, -1, netconfDeviceInfo); | 180 | + null, null, Optional.of(-1), netconfDeviceInfo); |
| 179 | netconfDeviceEventListeners.forEach( | 181 | netconfDeviceEventListeners.forEach( |
| 180 | listener -> listener.event(event)); | 182 | listener -> listener.event(event)); |
| 181 | } else { | 183 | } else { |
| ... | @@ -211,18 +213,20 @@ public class NetconfStreamThread extends Thread implements NetconfStreamHandler | ... | @@ -211,18 +213,20 @@ public class NetconfStreamThread extends Thread implements NetconfStreamHandler |
| 211 | } | 213 | } |
| 212 | } | 214 | } |
| 213 | 215 | ||
| 214 | - private static int getMsgId(String reply) { | 216 | + private static Optional<Integer> getMsgId(String reply) { |
| 215 | - if (!reply.contains(HELLO)) { | 217 | + if (reply.contains(HELLO)) { |
| 216 | - String[] outer = reply.split("message-id="); | 218 | + return Optional.of(0); |
| 217 | - Preconditions.checkArgument(outer.length != 1, | ||
| 218 | - "Error in retrieving the message id"); | ||
| 219 | - String messageID = outer[1].substring(0, 3).replace("\"", ""); | ||
| 220 | - Preconditions.checkNotNull(Integer.parseInt(messageID), | ||
| 221 | - "Error in retrieving the message id"); | ||
| 222 | - return Integer.parseInt(messageID); | ||
| 223 | - } else { | ||
| 224 | - return 0; | ||
| 225 | } | 219 | } |
| 220 | + if (reply.contains(RPC_ERROR) && !reply.contains(MESSAGE_ID)) { | ||
| 221 | + return Optional.empty(); | ||
| 222 | + } | ||
| 223 | + String[] outer = reply.split(MESSAGE_ID); | ||
| 224 | + Preconditions.checkArgument(outer.length != 1, | ||
| 225 | + "Error in retrieving the message id"); | ||
| 226 | + String messageID = outer[1].substring(0, 3).replace("\"", ""); | ||
| 227 | + Preconditions.checkNotNull(Integer.parseInt(messageID), | ||
| 228 | + "Error in retrieving the message id"); | ||
| 229 | + return Optional.of(Integer.parseInt(messageID)); | ||
| 226 | } | 230 | } |
| 227 | 231 | ||
| 228 | public void addDeviceEventListener(NetconfDeviceOutputEventListener listener) { | 232 | public void addDeviceEventListener(NetconfDeviceOutputEventListener listener) { | ... | ... |
-
Please register or login to post a comment