search

theforeman / foreman_fog_proxmox

Foreman plugin to add Proxmox compute resource based on fog-proxmox gem
GNU General Public License v3.0
105 stars 30 forks source link

Duplicate vmid on different cluster causes trouble #186

Closed selund closed 3 years ago

selund commented 3 years ago

Describe the bug The foreman-proxmox plugin does not handle that a vmid might be the same in two different clusters

To Reproduce Steps to reproduce the behavior: Create a vm on proxmox cluster 1 with vmid 100 Create a vm on proxmox cluster 2 with same vmid, and you will get into trouble.

Expected behavior vmid should be handled on a pr cluster level.

tristanrobert commented 3 years ago

The plugin only checks that the vmid is unique in a cluster according to this API request: https://pve.proxmox.com/pve-docs/api-viewer/#/cluster/nextid`. You can't have more than one cluster by compute resource. Maybe you mean different nodes in the same cluster attached to your compute resource. That's why you can't create two VMs in two nodes of the same cluster with the same vmid. If you want to handle two different clusters (not nodes in a cluster), you should create two compute resources attached to each cluster.

selund commented 3 years ago

No, we have two different compute resource defined. I'll try to create some examples of the problem when I'm back to work on Monday. Strictly speaking I think one of the resources only have one pve node and isn't defined as a cluster at this point in time.

Philambdo commented 3 years ago

We have that problem too. We have 3 Clusters in our Foreman Environment where we deploy VMS according to their usage. If i have a VMID on ClusterA that is already used on ClusterB (and that VM is managed by foreman too), than you can't create the vm in foreman. You have to pick the next free VMID based on all Clusters that Foreman knows off.

It looks like the vmid is used in the database (hosts table) as uuid for the corresponding server. It would be better if we can calculate the uuid in the database from a combination of proxmox cluster AND vmid so there can't be any mixup.

Here from the production.log some samples:

2021-03-09T10:07:11 [E|app|259fad52] Task 'Set up compute instance test-tst-01' *rollbacked*
2021-03-09T10:07:11 [E|app|259fad52] Task 'Query instance details for test-tst-01' *failed*
2021-03-09T10:07:11 [E|app|259fad52] Failed to save: uuid 103 is already used by ipsec-test
2021-03-09T10:07:12 [I|app|259fad52]   Rendered nic/_provider_specific_form.html.erb (Duration: 846.4ms | Allocations: 37829)
2021-03-09T10:07:12 [I|app|259fad52]   Rendered nic/manageds/_managed.html.erb (Duration: 925.6ms | Allocations: 57972)
2021-03-09T10:07:12 [I|app|259fad52]   Rendered hosts/_interfaces.html.erb (Duration: 930.1ms | Allocations: 58587)
2021-03-09T10:07:12 [I|app|259fad52]   Rendered hosts/_form.html.erb (Duration: 1170.9ms | Allocations: 160279)
2021-03-09T10:07:12 [I|app|259fad52]   Rendered hosts/new.html.erb within layouts/application (Duration: 1174.4ms | Allocations: 160483)
2021-03-09T10:07:12 [W|app|259fad52] undefined method `type' for nil:NilClass
2021-03-09T10:07:12 [I|app|259fad52] Backtrace for 'undefined method `type' for nil:NilClass' error (ActionView::Template::Error): undefined method `type' for nil:NilClass
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/foreman_fog_proxmox-0.11.1/app/helpers/proxmox_vm_helper.rb:90:in `vm_type'
 259fad52 | /usr/share/foreman/app/views/nic/_provider_specific_form.html.erb:31:in `_06543d6838078fafbe88c832f2dd7b75'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/base.rb:274:in `_run'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/template.rb:185:in `block in render'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/activesupport-6.0.3.5/lib/active_support/notifications.rb:182:in `instrument'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/template.rb:385:in `instrument_render_template'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/template.rb:183:in `render'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/deface-1.6.2/lib/deface/action_view_extensions.rb:46:in `render'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/renderer/partial_renderer.rb:357:in `block in render_partial'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/renderer/abstract_renderer.rb:88:in `block in instrument'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/activesupport-6.0.3.5/lib/active_support/notifications.rb:180:in `block in instrument'
...
tristanrobert commented 3 years ago

I am testing it.

tristanrobert commented 3 years ago

We have that problem too. We have 3 Clusters in our Foreman Environment where we deploy VMS according to their usage. If i have a VMID on ClusterA that is already used on ClusterB (and that VM is managed by foreman too), than you can't create the vm in foreman. You have to pick the next free VMID based on all Clusters that Foreman knows off.

It looks like the vmid is used in the database (hosts table) as uuid for the corresponding server. It would be better if we can calculate the uuid in the database from a combination of proxmox cluster AND vmid so there can't be any mixup.

Here from the production.log some samples:

2021-03-09T10:07:11 [E|app|259fad52] Task 'Set up compute instance test-tst-01' *rollbacked*
2021-03-09T10:07:11 [E|app|259fad52] Task 'Query instance details for test-tst-01' *failed*
2021-03-09T10:07:11 [E|app|259fad52] Failed to save: uuid 103 is already used by ipsec-test
2021-03-09T10:07:12 [I|app|259fad52]   Rendered nic/_provider_specific_form.html.erb (Duration: 846.4ms | Allocations: 37829)
2021-03-09T10:07:12 [I|app|259fad52]   Rendered nic/manageds/_managed.html.erb (Duration: 925.6ms | Allocations: 57972)
2021-03-09T10:07:12 [I|app|259fad52]   Rendered hosts/_interfaces.html.erb (Duration: 930.1ms | Allocations: 58587)
2021-03-09T10:07:12 [I|app|259fad52]   Rendered hosts/_form.html.erb (Duration: 1170.9ms | Allocations: 160279)
2021-03-09T10:07:12 [I|app|259fad52]   Rendered hosts/new.html.erb within layouts/application (Duration: 1174.4ms | Allocations: 160483)
2021-03-09T10:07:12 [W|app|259fad52] undefined method `type' for nil:NilClass
2021-03-09T10:07:12 [I|app|259fad52] Backtrace for 'undefined method `type' for nil:NilClass' error (ActionView::Template::Error): undefined method `type' for nil:NilClass
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/foreman_fog_proxmox-0.11.1/app/helpers/proxmox_vm_helper.rb:90:in `vm_type'
 259fad52 | /usr/share/foreman/app/views/nic/_provider_specific_form.html.erb:31:in `_06543d6838078fafbe88c832f2dd7b75'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/base.rb:274:in `_run'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/template.rb:185:in `block in render'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/activesupport-6.0.3.5/lib/active_support/notifications.rb:182:in `instrument'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/template.rb:385:in `instrument_render_template'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/template.rb:183:in `render'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/deface-1.6.2/lib/deface/action_view_extensions.rb:46:in `render'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/renderer/partial_renderer.rb:357:in `block in render_partial'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/actionview-6.0.3.5/lib/action_view/renderer/abstract_renderer.rb:88:in `block in instrument'
 259fad52 | /usr/share/foreman/vendor/ruby/2.5.0/gems/activesupport-6.0.3.5/lib/active_support/notifications.rb:180:in `block in instrument'
...

I see another (old) issue in your logs: undefined method type for nil:NilClass. You use an old release: 0.11.1. You should try with the newest.

tristanrobert commented 3 years ago

I have reproduced the issue. UUID's host must be unique in foreman database. foreman_fog_proxmox uses vmid as uuid: uuid=vmid so it is the issue. A solution could be something like that: uuid=cluster_id + '_' + vmid and the function find_vm_by_uuid must be changed too. I am working on it.