openbaton / NFVO

Repository containing the source code of the NFVO
Apache License 2.0
61 stars 52 forks source link

Proposed Change to ComponentManager #228

Closed wittling closed 6 years ago

wittling commented 7 years ago

I had an issue with the NFVO last week with RabbitMQ, and it was not creating the user and queue for the Generic VNFM driver. For some reason, RabbitMQ was failing when those regular expression permissions were being added. The net effect of this, however, was that the Generic driver was being registered - and existed in ManagerCredentials - but there was no user or queue in RabbitMQ. After this, the NFVO would throw an exception every time I tried to re-register the generic endpoint (from the GUI) because it was trying to remove a RabbitMQ user that did not exist. I traced the issue down to ComponentManager.java - and put a separate try catch around removeRabbitMqUser call, and just printed a warning and let the code proceed, as opposed to throwing a full-on fatal exception for the situation where you cannot remove a user that does exist anyway.

You may decide to reject this change, but I will include it here lest you decide that this kind of code change makes sense. try { log.debug("Setting queue permissions for user: " + username); setRabbitMqUserPermissions( rabbitUsername, rabbitPassword, brokerIp, managementPort, username, vhost, configurePermissions, writePermissions, readPermissions); } catch (Exception e) { try { log.error("Error setting permissions for user: " + username); log.info("Attempting to remove rabbitMqUser: " + username); try { removeRabbitMqUser( rabbitUsername, rabbitPassword, brokerIp, managementPort, username); } catch (Exception e3) { log.warn("Error removing RabbitMqUser: " + username); e3.printStackTrace(); log.warn("RabbitMqUser may not exist. Continuing."); } } catch (Exception e2) { log.error("Clean up failed. Could not remove RabbitMQ user " + username); e2.printStackTrace(); } throw e; }

lorenzotomasini commented 7 years ago

Hello,

i find a NFVO without VNFMs nor VIM Drivers quite useless. For any kind of deployment you need at least a VNFM. And it makes sense from my point of view.

Thus, i think the NFVO should fail starting whenever is not able to create the rabbit user dedicated to the managers (vnfm and vim). I thought this was already the case, but apparently not. @ThomasBri can you have a look at this?

However, i also understand that there could be inconsistent situation in NFVO <--> RabbitMQ and then the removeRabbitMqUser should catch the error if the user does not exist.

Thanks for spotting, we will be back to this issue

wittling commented 7 years ago

I guess right now the way the orchestrator is architected and designed, it does require you to have vim drivers at nfvo startup. But it is designed to be able to allow you to start (add) / stop (remove) vnfms "on the fly" - which is kind of cool and powerful - if this kind of power and flexibility adds true value. For one thing, you can compile new code and launch the vnfm without an nfvo restart, which is kind of nice. i have never tested switching vnfm drivers while the nfvo is running, or given much thought to that use case.

But there are, and will be cases, undoubtedly, where a message queue administrator might go in and add or remove a user, or change their permissions while nfvo, vnfm and related processes are running, as dangerous a thing as this can be to do.

I certainly see the reason to error out if you are starting or creating and you have a dependency and need a resource that is not present or where you expect it to be. But if you are attempt to remove something that is not there, this is really a warning condition; not an error. Just like deleting a row in a database...if it is not there, consider it deleted and move on! But the code here is throwing an error not because it there was an error in the act of deleting, it is throwing the same exception because the item was not found. And I think those two conditions need to be treated differently (if you try to delete and it is not found, consider it a successful delete instead of an exception or at most bark out a warning about it, but continue processing).