Inventory Enterprise (Nuage) into CloudTenant (MIQ) #92
Conversation
miha-plesko
force-pushed
the
inventory-enterprises
branch
2 times, most recently
from
06b6a08 to
469a672
Compare
|
@miha-plesko interesting, I think NetworkGroup was originally designed to replace CloudNetwork model. We did a new model, because it was a bit different. Does CloudTenant fit this better? |
|
@Ladas I'm pretty sure, yes. See here how Enterprise object looks like: https://nuagenetworks.github.io/vsd-api-documentation/v5_0/enterprise.html - it's more like a "project" i.e. "tenant" i.e. "scope" than a network entity, because it has no networking properties like CIDR. We had a discussion with @gtanzillo , @Fryguy and @gberginc about this and I think CloudTenant seems the most reasonable choice here. Looking forward to your opinion though. |
| def network_group(ems_ref) | ||
| network_groups.find { |ng| ng['ID'] == ems_ref } | ||
| def cloud_tenant(ems_ref) | ||
| cloud_tenants.find { |tenant| tenant['ID'] == ems_ref } |
There was a problem hiding this comment.
we could cache it like in target_collection.rb, so we don't do repeated O(n) searches
There was a problem hiding this comment.
1 nit, otherwise it looks great 👍
@miha-plesko if it's more like a project, then yeah CloudTenant is better fit. (NetworkGroup will not be used anymore, so we can probably drop it)
With this commit we inventory Enterprise into CloudTenant model instead into NetworkGroup model. We capture its name and description and hook it to the network manager. RFE BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1574903 Signed-off-by: Miha Pleško <[email protected]>
miha-plesko
force-pushed
the
inventory-enterprises
branch
from
469a672 to
94390e6
Compare
|
Thanks for suggesting the caching improvement, I've updated the code. I can prepare a PR in core to remove NetworkGroup completely @Ladas if you're sure no other provider uses it. |
|
Checked commit miha-plesko@94390e6 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@miha-plesko I am 98% sure no other provider uses NetworkGroup :-) We've created it just for Nuage and the work for using it further was never finished. |
|
@Ladas OK I'll remove it then. Are you good with this PR now? We want to further expand inventory and need this one merged to continue. |
With this commit we inventory Enterprise into
CloudTenantmodel instead intoNetworkGroupmodel. We capture its name and description and hook it to the network manager.Followup PR on UI: ManageIQ/manageiq-ui-classic#3995
RFE BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1574903
@Ladas with this commit we stop using
NetworkGroupand useCloudTenantinstead. We promised such change long time ago and here we are. This commit seems large but what it does in fact is a find-and-replaceand nothing more. Most of the files are in fact specs...