홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Sanjay S
Committed by Gerrit Code Review
eb5eebbb 1 parent fc3b48e0

Modified to accommodate code review comments. 

Change-Id: I391db0afe13d862706b5592e60e83b725d99e672
Inline Side-by-side
Showing 4 changed files with 56 additions and 94 deletions
...@@ -41,7 +41,7 @@ import com.tailf.jnc.SSHSession; ...@@ -41,7 +41,7 @@ import com.tailf.jnc.SSHSession;
41 * necessary information to connect and execute NETCONF operations. 41 * necessary information to connect and execute NETCONF operations.
42 */ 42 */
43 public class NetconfDevice { 43 public class NetconfDevice {
44 - private static final Logger log = getLogger(NetconfDevice.class); 44 + private final Logger log = getLogger(NetconfDevice.class);
45 45
46 /** 46 /**
47 * The Device State is used to determine whether the device is active or 47 * The Device State is used to determine whether the device is active or
...@@ -51,9 +51,9 @@ public class NetconfDevice { ...@@ -51,9 +51,9 @@ public class NetconfDevice {
51 public static enum DeviceState { 51 public static enum DeviceState {
52 /* Used to specify Active state of the device */ 52 /* Used to specify Active state of the device */
53 ACTIVE, 53 ACTIVE,
54 - /* Used to specify In Active state of the device */ 54 + /* Used to specify inactive state of the device */
55 INACTIVE, 55 INACTIVE,
56 - /* Used to specify In Valid state of the device */ 56 + /* Used to specify invalid state of the device */
57 INVALID 57 INVALID
58 } 58 }
59 59
...@@ -99,13 +99,11 @@ public class NetconfDevice { ...@@ -99,13 +99,11 @@ public class NetconfDevice {
99 } 99 }
100 // Send hello message to retrieve capabilities. 100 // Send hello message to retrieve capabilities.
101 } catch (IOException e) { 101 } catch (IOException e) {
102 - NetconfDevice.log 102 + log.error("Fatal Error while creating connection to the device: "
103 - .error("Fatal Error while creating connection to the device: "
104 + deviceInfo(), e); 103 + deviceInfo(), e);
105 throw e; 104 throw e;
106 } catch (JNCException e) { 105 } catch (JNCException e) {
107 - NetconfDevice.log.error("Failed to connect to the device: " 106 + log.error("Failed to connect to the device: " + deviceInfo(), e);
108 - + deviceInfo(), e);
109 throw e; 107 throw e;
110 } 108 }
111 109
...@@ -118,14 +116,10 @@ public class NetconfDevice { ...@@ -118,14 +116,10 @@ public class NetconfDevice {
118 ssh = new SSHSession(sshConnection); 116 ssh = new SSHSession(sshConnection);
119 String helloRequestXML = INPUT_HELLO_XML_MSG.trim(); 117 String helloRequestXML = INPUT_HELLO_XML_MSG.trim();
120 118
121 - if (NetconfDevice.log.isDebugEnabled()) { 119 + log.debug("++++++++++++++++++++++++++++++++++Sending Hello: "
122 - NetconfDevice.log 120 + + sshConnection.getGanymedConnection().getHostname()
123 - .debug("++++++++++++++++++++++++++++++++++Sending Hello: "
124 - + sshConnection.getGanymedConnection()
125 - .getHostname()
126 + "++++++++++++++++++++++++++++++++++"); 121 + "++++++++++++++++++++++++++++++++++");
127 printPrettyXML(helloRequestXML); 122 printPrettyXML(helloRequestXML);
128 - }
129 ssh.print(helloRequestXML); 123 ssh.print(helloRequestXML);
130 // ssh.print(endCharSeq); 124 // ssh.print(endCharSeq);
131 ssh.flush(); 125 ssh.flush();
...@@ -139,8 +133,7 @@ public class NetconfDevice { ...@@ -139,8 +133,7 @@ public class NetconfDevice {
139 if (ssh.ready()) { 133 if (ssh.ready()) {
140 StringBuffer readOne = ssh.readOne(); 134 StringBuffer readOne = ssh.readOne();
141 if (readOne == null) { 135 if (readOne == null) {
142 - NetconfDevice.log 136 + log.error("The Hello Contains No Capabilites");
143 - .error("The Hello Contains No Capabilites");
144 throw new JNCException( 137 throw new JNCException(
145 JNCException.SESSION_ERROR, 138 JNCException.SESSION_ERROR,
146 "server does not support NETCONF base capability: " 139 "server does not support NETCONF base capability: "
...@@ -148,39 +141,31 @@ public class NetconfDevice { ...@@ -148,39 +141,31 @@ public class NetconfDevice {
148 } else { 141 } else {
149 xmlResponse = readOne.toString().trim(); 142 xmlResponse = readOne.toString().trim();
150 143
151 - if (NetconfDevice.log.isDebugEnabled()) { 144 + log.debug("++++++++++++++++++++++++++++++++++Reading Capabilities: "
152 - NetconfDevice.log
153 - .debug("++++++++++++++++++++++++++++++++++Reading Capabilities: "
154 + sshConnection.getGanymedConnection() 145 + sshConnection.getGanymedConnection()
155 .getHostname() 146 .getHostname()
156 + "++++++++++++++++++++++++++++++++++"); 147 + "++++++++++++++++++++++++++++++++++");
157 148
158 printPrettyXML(xmlResponse); 149 printPrettyXML(xmlResponse);
159 - }
160 processCapabilities(xmlResponse); 150 processCapabilities(xmlResponse);
161 } 151 }
162 } 152 }
163 reachable = true; 153 reachable = true;
164 } catch (IOException e) { 154 } catch (IOException e) {
165 - NetconfDevice.log 155 + log.error("Fatal Error while sending Hello Message to the device: "
166 - .error("Fatal Error while sending Hello Message to the device: "
167 + deviceInfo(), e); 156 + deviceInfo(), e);
168 } catch (JNCException e) { 157 } catch (JNCException e) {
169 - NetconfDevice.log 158 + log.error("Fatal Error while sending Hello Message to the device: "
170 - .error("Fatal Error while sending Hello Message to the device: "
171 + deviceInfo(), e); 159 + deviceInfo(), e);
172 } finally { 160 } finally {
173 - if (NetconfDevice.log.isDebugEnabled()) { 161 + log.debug("Closing the session after successful execution");
174 - NetconfDevice.log
175 - .debug("Closing the session after successful execution");
176 - }
177 ssh.close(); 162 ssh.close();
178 } 163 }
179 } 164 }
180 165
181 private void processCapabilities(String xmlResponse) throws JNCException { 166 private void processCapabilities(String xmlResponse) throws JNCException {
182 if (xmlResponse.isEmpty()) { 167 if (xmlResponse.isEmpty()) {
183 - NetconfDevice.log.error("The capability response cannot be empty"); 168 + log.error("The capability response cannot be empty");
184 throw new JNCException( 169 throw new JNCException(
185 JNCException.SESSION_ERROR, 170 JNCException.SESSION_ERROR,
186 "server does not support NETCONF base capability: " 171 "server does not support NETCONF base capability: "
...@@ -192,8 +177,7 @@ public class NetconfDevice { ...@@ -192,8 +177,7 @@ public class NetconfDevice {
192 Element rootElement = doc.getRootElement(); 177 Element rootElement = doc.getRootElement();
193 processCapabilities(rootElement); 178 processCapabilities(rootElement);
194 } catch (Exception e) { 179 } catch (Exception e) {
195 - NetconfDevice.log.error("ERROR while parsing the XML " 180 + log.error("ERROR while parsing the XML " + xmlResponse);
196 - + xmlResponse);
197 } 181 }
198 } 182 }
199 183
...@@ -218,12 +202,9 @@ public class NetconfDevice { ...@@ -218,12 +202,9 @@ public class NetconfDevice {
218 Document doc = new SAXBuilder().build(new StringReader(xmlstring)); 202 Document doc = new SAXBuilder().build(new StringReader(xmlstring));
219 XMLOutputter xmOut = new XMLOutputter(Format.getPrettyFormat()); 203 XMLOutputter xmOut = new XMLOutputter(Format.getPrettyFormat());
220 String outputString = xmOut.outputString(doc); 204 String outputString = xmOut.outputString(doc);
221 - if (NetconfDevice.log.isDebugEnabled()) { 205 + log.debug(outputString);
222 - NetconfDevice.log.debug(outputString);
223 - }
224 } catch (Exception e) { 206 } catch (Exception e) {
225 - NetconfDevice.log.error("ERROR while parsing the XML " + xmlstring, 207 + log.error("ERROR while parsing the XML " + xmlstring, e);
226 - e);
227 208
228 } 209 }
229 } 210 }
......
...@@ -63,7 +63,7 @@ import org.slf4j.Logger; ...@@ -63,7 +63,7 @@ import org.slf4j.Logger;
63 public class NetconfDeviceProvider extends AbstractProvider 63 public class NetconfDeviceProvider extends AbstractProvider
64 implements DeviceProvider { 64 implements DeviceProvider {
65 65
66 - private static final Logger log = getLogger(NetconfDeviceProvider.class); 66 + private final Logger log = getLogger(NetconfDeviceProvider.class);
67 67
68 private Map<DeviceId, NetconfDevice> netconfDeviceMap = new ConcurrentHashMap<DeviceId, NetconfDevice>(); 68 private Map<DeviceId, NetconfDevice> netconfDeviceMap = new ConcurrentHashMap<DeviceId, NetconfDevice>();
69 69
...@@ -102,7 +102,7 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -102,7 +102,7 @@ public class NetconfDeviceProvider extends AbstractProvider
102 102
103 @Activate 103 @Activate
104 public void activate(ComponentContext context) { 104 public void activate(ComponentContext context) {
105 - NetconfDeviceProvider.log.info("Netconf Device Provider Started"); 105 + log.info("Netconf Device Provider Started");
106 providerService = providerRegistry.register(this); 106 providerService = providerRegistry.register(this);
107 modified(context); 107 modified(context);
108 } 108 }
...@@ -117,31 +117,29 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -117,31 +117,29 @@ public class NetconfDeviceProvider extends AbstractProvider
117 } 117 }
118 deviceBuilder.awaitTermination(1000, TimeUnit.MILLISECONDS); 118 deviceBuilder.awaitTermination(1000, TimeUnit.MILLISECONDS);
119 } catch (InterruptedException e) { 119 } catch (InterruptedException e) {
120 - NetconfDeviceProvider.log.error("Device builder did not terminate"); 120 + log.error("Device builder did not terminate");
121 } 121 }
122 deviceBuilder.shutdownNow(); 122 deviceBuilder.shutdownNow();
123 netconfDeviceMap.clear(); 123 netconfDeviceMap.clear();
124 providerRegistry.unregister(this); 124 providerRegistry.unregister(this);
125 providerService = null; 125 providerService = null;
126 - NetconfDeviceProvider.log.info("Stopped"); 126 + log.info("Stopped");
127 } 127 }
128 128
129 @Modified 129 @Modified
130 public void modified(ComponentContext context) { 130 public void modified(ComponentContext context) {
131 if (context == null) { 131 if (context == null) {
132 - NetconfDeviceProvider.log.info("No configuration file"); 132 + log.info("No configuration file");
133 return; 133 return;
134 } 134 }
135 Dictionary<?, ?> properties = context.getProperties(); 135 Dictionary<?, ?> properties = context.getProperties();
136 String deviceCfgValue = get(properties, "devConfigs"); 136 String deviceCfgValue = get(properties, "devConfigs");
137 - NetconfDeviceProvider.log 137 + log.info("Getting Device configuration from cfg file: "
138 - .info("Getting Device configuration from cfg file: "
139 + deviceCfgValue); 138 + deviceCfgValue);
140 if (!isNullOrEmpty(deviceCfgValue)) { 139 if (!isNullOrEmpty(deviceCfgValue)) {
141 addOrRemoveDevicesConfig(deviceCfgValue); 140 addOrRemoveDevicesConfig(deviceCfgValue);
142 } else { 141 } else {
143 - NetconfDeviceProvider.log 142 + log.info("Device Configuration value receiviced from the property 'devConfigs': "
144 - .info("Device Configuration value receiviced from the property 'devConfigs': "
145 + deviceCfgValue + ", is not valid"); 143 + deviceCfgValue + ", is not valid");
146 } 144 }
147 } 145 }
...@@ -150,10 +148,11 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -150,10 +148,11 @@ public class NetconfDeviceProvider extends AbstractProvider
150 for (String deviceEntry : deviceConfig.split(",")) { 148 for (String deviceEntry : deviceConfig.split(",")) {
151 NetconfDevice device = processDeviceEntry(deviceEntry); 149 NetconfDevice device = processDeviceEntry(deviceEntry);
152 if (device != null) { 150 if (device != null) {
153 - NetconfDeviceProvider.log.info("Device Detail: " + "username: " 151 + log.info("Device Detail: " + "username: "
154 + device.getUsername() + ", host: " 152 + device.getUsername() + ", host: "
155 + device.getSshHost() + ", port: " 153 + device.getSshHost() + ", port: "
156 - + device.getSshPort()); 154 + + device.getSshPort() + " device state: "
155 + + device.getDeviceState().name());
157 if (device.isActive()) { 156 if (device.isActive()) {
158 deviceBuilder.submit(new DeviceCreator(device, true)); 157 deviceBuilder.submit(new DeviceCreator(device, true));
159 } else { 158 } else {
...@@ -165,12 +164,10 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -165,12 +164,10 @@ public class NetconfDeviceProvider extends AbstractProvider
165 164
166 private NetconfDevice processDeviceEntry(String deviceEntry) { 165 private NetconfDevice processDeviceEntry(String deviceEntry) {
167 if (deviceEntry == null) { 166 if (deviceEntry == null) {
168 - NetconfDeviceProvider.log 167 + log.info("No content for Device Entry, so cannot proceed further.");
169 - .info("No content for Device Entry, so cannot proceed further.");
170 return null; 168 return null;
171 } 169 }
172 - NetconfDeviceProvider.log 170 + log.info("Trying to convert Device Entry String: " + deviceEntry
173 - .info("Trying to convert Device Entry String: " + deviceEntry
174 + " to a Netconf Device Object"); 171 + " to a Netconf Device Object");
175 NetconfDevice device = null; 172 NetconfDevice device = null;
176 try { 173 try {
...@@ -187,16 +184,14 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -187,16 +184,14 @@ public class NetconfDeviceProvider extends AbstractProvider
187 try { 184 try {
188 hostPort = Integer.parseInt(infoSplit[1]); 185 hostPort = Integer.parseInt(infoSplit[1]);
189 } catch (NumberFormatException nfe) { 186 } catch (NumberFormatException nfe) {
190 - NetconfDeviceProvider.log 187 + log.error("Bad Configuration Data: Failed to parse host port number string: "
191 - .error("Bad Configuration Data: Failed to parse host port number string: "
192 + infoSplit[1]); 188 + infoSplit[1]);
193 throw nfe; 189 throw nfe;
194 } 190 }
195 String deviceState = infoSplit[2]; 191 String deviceState = infoSplit[2];
196 if (isNullOrEmpty(username) || isNullOrEmpty(password) 192 if (isNullOrEmpty(username) || isNullOrEmpty(password)
197 || isNullOrEmpty(hostIp) || hostPort == 0) { 193 || isNullOrEmpty(hostIp) || hostPort == 0) {
198 - NetconfDeviceProvider.log 194 + log.warn("Bad Configuration Data: both user and device information parts of Configuration "
199 - .warn("Bad Configuration Data: both user and device information parts of Configuration "
200 + deviceEntry + " should be non-nullable"); 195 + deviceEntry + " should be non-nullable");
201 } else { 196 } else {
202 device = new NetconfDevice(hostIp, hostPort, username, password); 197 device = new NetconfDevice(hostIp, hostPort, username, password);
...@@ -206,27 +201,23 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -206,27 +201,23 @@ public class NetconfDeviceProvider extends AbstractProvider
206 device.setDeviceState(DeviceState.ACTIVE); 201 device.setDeviceState(DeviceState.ACTIVE);
207 } else if (deviceState.toUpperCase() 202 } else if (deviceState.toUpperCase()
208 .equals(DeviceState.INACTIVE.name())) { 203 .equals(DeviceState.INACTIVE.name())) {
209 - device.setDeviceState(DeviceState.ACTIVE); 204 + device.setDeviceState(DeviceState.INACTIVE);
210 } else { 205 } else {
211 - NetconfDeviceProvider.log 206 + log.warn("Device State Information can not be empty, so marking the state as INVALID");
212 - .warn("Device State Information can not be empty, so marking the state as INVALID");
213 device.setDeviceState(DeviceState.INVALID); 207 device.setDeviceState(DeviceState.INVALID);
214 } 208 }
215 } else { 209 } else {
216 - NetconfDeviceProvider.log 210 + log.warn("The device entry do not specify state information, so marking the state as INVALID");
217 - .warn("The device entry do not specify state information, so marking the state as INVALID");
218 device.setDeviceState(DeviceState.INVALID); 211 device.setDeviceState(DeviceState.INVALID);
219 } 212 }
220 } 213 }
221 } catch (ArrayIndexOutOfBoundsException aie) { 214 } catch (ArrayIndexOutOfBoundsException aie) {
222 - NetconfDeviceProvider.log 215 + log.error("Error while reading config infromation from the config file: "
223 - .error("Error while reading config infromation from the config file: "
224 + "The user, host and device state infomation should be " 216 + "The user, host and device state infomation should be "
225 + "in the order 'userInfo@hostInfo:deviceState'" 217 + "in the order 'userInfo@hostInfo:deviceState'"
226 + deviceEntry, aie); 218 + deviceEntry, aie);
227 } catch (Exception e) { 219 } catch (Exception e) {
228 - NetconfDeviceProvider.log 220 + log.error("Error while parsing config information for the device entry: "
229 - .error("Error while parsing config information for the device entry: "
230 + deviceEntry, e); 221 + deviceEntry, e);
231 } 222 }
232 return device; 223 return device;
...@@ -246,8 +237,7 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -246,8 +237,7 @@ public class NetconfDeviceProvider extends AbstractProvider
246 public boolean isReachable(DeviceId deviceId) { 237 public boolean isReachable(DeviceId deviceId) {
247 NetconfDevice netconfDevice = netconfDeviceMap.get(deviceId); 238 NetconfDevice netconfDevice = netconfDeviceMap.get(deviceId);
248 if (netconfDevice == null) { 239 if (netconfDevice == null) {
249 - NetconfDeviceProvider.log 240 + log.warn("BAD REQUEST: the requested device id: "
250 - .warn("BAD REQUEST: the requested device id: "
251 + deviceId.toString() 241 + deviceId.toString()
252 + " is not associated to any NETCONF Device"); 242 + " is not associated to any NETCONF Device");
253 return false; 243 return false;
...@@ -273,13 +263,11 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -273,13 +263,11 @@ public class NetconfDeviceProvider extends AbstractProvider
273 263
274 @Override 264 @Override
275 public void run() { 265 public void run() {
276 - if (createFlag && (device.getDeviceState() == DeviceState.ACTIVE)) { 266 + if (createFlag) {
277 - NetconfDeviceProvider.log 267 + log.info("Trying to create Device Info on ONOS core");
278 - .info("Trying to create Device Info on ONOS core");
279 advertiseDevices(); 268 advertiseDevices();
280 } else { 269 } else {
281 - NetconfDeviceProvider.log 270 + log.info("Trying to remove Device Info on ONOS core");
282 - .info("Trying to remove Device Info on ONOS core");
283 removeDevices(); 271 removeDevices();
284 } 272 }
285 } 273 }
...@@ -288,19 +276,24 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -288,19 +276,24 @@ public class NetconfDeviceProvider extends AbstractProvider
288 * For each Netconf Device, remove the entry from the device store. 276 * For each Netconf Device, remove the entry from the device store.
289 */ 277 */
290 private void removeDevices() { 278 private void removeDevices() {
291 - if (!device.isReachable()) { 279 + if (device == null) {
292 - log.error("BAD Request: 'Currently device is not discovered, so cannot remove/disconnect the device: " 280 + log.warn("The Request Netconf Device is null, cannot proceed further");
293 - + device.deviceInfo() + "'");
294 return; 281 return;
295 } 282 }
296 try { 283 try {
297 DeviceId did = getDeviceId(); 284 DeviceId did = getDeviceId();
285 + if (!netconfDeviceMap.containsKey(did)) {
286 + log.error("BAD Request: 'Currently device is not discovered, "
287 + + "so cannot remove/disconnect the device: "
288 + + device.deviceInfo() + "'");
289 + return;
290 + }
298 providerService.deviceDisconnected(did); 291 providerService.deviceDisconnected(did);
299 device.disconnect(); 292 device.disconnect();
293 + netconfDeviceMap.remove(did);
300 delay(EVENTINTERVAL); 294 delay(EVENTINTERVAL);
301 } catch (URISyntaxException uriSyntaxExcpetion) { 295 } catch (URISyntaxException uriSyntaxExcpetion) {
302 - NetconfDeviceProvider.log 296 + log.error("Syntax Error while creating URI for the device: "
303 - .error("Syntax Error while creating URI for the device: "
304 + device.deviceInfo() 297 + device.deviceInfo()
305 + " couldn't remove the device from the store", 298 + " couldn't remove the device from the store",
306 uriSyntaxExcpetion); 299 uriSyntaxExcpetion);
...@@ -314,8 +307,7 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -314,8 +307,7 @@ public class NetconfDeviceProvider extends AbstractProvider
314 private void advertiseDevices() { 307 private void advertiseDevices() {
315 try { 308 try {
316 if (device == null) { 309 if (device == null) {
317 - NetconfDeviceProvider.log 310 + log.warn("The Request Netconf Device is null, cannot proceed further");
318 - .warn("The Request Netconf Device is null, cannot proceed further");
319 return; 311 return;
320 } 312 }
321 device.init(); 313 device.init();
...@@ -327,30 +319,19 @@ public class NetconfDeviceProvider extends AbstractProvider ...@@ -327,30 +319,19 @@ public class NetconfDeviceProvider extends AbstractProvider
327 "", "", 319 "", "",
328 "", "", 320 "", "",
329 cid); 321 cid);
330 - if (NetconfDeviceProvider.log.isDebugEnabled()) { 322 + log.info("Persisting Device" + did.uri().toString());
331 - NetconfDeviceProvider.log.debug("Persisting Device"
332 - + did.uri().toString());
333 - }
334 323
335 netconfDeviceMap.put(did, device); 324 netconfDeviceMap.put(did, device);
336 providerService.deviceConnected(did, desc); 325 providerService.deviceConnected(did, desc);
337 - if (NetconfDeviceProvider.log.isDebugEnabled()) { 326 + log.info("Done with Device Info Creation on ONOS core. Device Info: "
338 - NetconfDeviceProvider.log 327 + + device.deviceInfo() + " " + did.uri().toString());
339 - .debug("Done with Device Info Creation on ONOS core. Device Info: "
340 - + device.deviceInfo()
341 - + " "
342 - + did.uri().toString());
343 - }
344 delay(EVENTINTERVAL); 328 delay(EVENTINTERVAL);
345 } catch (URISyntaxException e) { 329 } catch (URISyntaxException e) {
346 - NetconfDeviceProvider.log 330 + log.error("Syntax Error while creating URI for the device: "
347 - .error("Syntax Error while creating URI for the device: "
348 + device.deviceInfo() 331 + device.deviceInfo()
349 - + " couldn't persist the device onto the store", 332 + + " couldn't persist the device onto the store", e);
350 - e);
351 } catch (Exception e) { 333 } catch (Exception e) {
352 - NetconfDeviceProvider.log 334 + log.error("Error while initializing session for the device: "
353 - .error("Error while initializing session for the device: "
354 + device.deviceInfo(), e); 335 + device.deviceInfo(), e);
355 } 336 }
356 } 337 }
......