-
Notifications
You must be signed in to change notification settings - Fork 982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #18088 - Add compute resources to subnets #4193
Fixes #18088 - Add compute resources to subnets #4193
Conversation
@bagasse, thanks for your PR! By analyzing the history of the files in this pull request, we identified @timogoebel, @domcleal and @dLobatog to be potential reviewers. |
wouldnt it be better to link the compure resource to an interface object (nic) instead? /cc @ares ? |
I think it makes more sense to link CR to subnet as suggested in the patch. For virtual interfaces (vlans, aliases, bonds, bridges) the association would not make sense since they don't exist in the hypervisor. Or is there some use case I'm missing? From quick scan of the PR, we should make sure that it works with bare metals well too, I haven't tested but I think it might fail in host form. |
4bccc59
to
88a9e1c
Compare
The fog part was released yesterday (fog 1.40), so all third party deps are now released.
|
[test foreman] |
class CreateSubnetComputeRessources < ActiveRecord::Migration | ||
def up | ||
create_table :subnet_compute_resources do |t| | ||
t.references :compute_resource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a way to default to the taxonomy subnets instead of introducing a new db model?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ohadlevy thanks for taking time to look at this PR. I implemented it like it was already done for domains. All the code i've seen related to taxonomy was for rights management, orgs and locations, so i didn't take a look at it as a way to implement this. I didn't saw many documentation (developer handbook, blog, or videos/deepdive) about taxonomies. If you have some resources to share, i will be happy to take a look at it.
88a9e1c
to
5977b69
Compare
@bagasse : First of all, thanks for the PR. I briefly checked it out and I think, this isn't a complete solution. I'd suggest that instead of linking subnets to compute resources it would be better to link subnets to compute resource networks instead. Allow me elaborate: |
@timogoebel, thank you for your comment. The original purpose of this PR was to automatically link foreman subnets to VLAN based Compute resources virtual networks based on VLAN ID in the foreman subnet and in the virtual network. So the mapping to compute resource is mostly sufficient for these case because of this common id. It keep the mapping configuration and the mapping UI simple and similar to the subnet-domain mapping. We don't use any other compute resource than ovirt ATM, so I didn't take a look on how other common foreman compute resources work on the network part. For the UI part, do you have any pointer ? |
Yeah. That would have been my preferred way to do that as well. Unfortunately, reality looks different. On VMWare you actually don't have an easy way to get the VLAN-ID of a VM. I think my previous comment didn't make this clear. I believe, it's similar with OpenStack. AWS doesn't have the concept of a VLAN, just VPCs. That's why I'd suggest to use a different mapping.
Unfortunately I don't have oVirt available to fully test this PR.
How about using
Does that make sense to you? If you are willing to change your implementation to what I suggested, I'd be happy to assist if time permits. |
I am with @timogoebel here, we need better flexibility here. Therefore associating subnet with CR network is better, but much more challenging of course as networks are not stored in our database. I will be more than happy to test on libvirt and also oVirt 3.6 (will need to install 4.0 I don't have one yet). Additional note, when you look in the Foreman Survey 2017 you will find that libvirt is in wide use. This needs to work on there as well, luckily libvirt is very similar to oVirt (as oVirt uses it for backend). I don't understand how exactly this should work with bare-metal. What is the expectations and workflow in this case? |
Any progress here? |
Hi @xprazak2, No this was abandoned. I think foreman is missing some piece on network management as stated in this comment to integrate compute resource correctly at network/subnet level. I will make a post on this to get some comments on this. |
Depends on [1]. No change needed on fog.
Tested with oVirt 4.0.5 with APIv3 and [1] applied to rbovirt. Any advises to add automated test for this PR are welcome.
[1] abenari/rbovirt#116