diff --git a/app/models/foreman_fog_proxmox/proxmox_vm_commands.rb b/app/models/foreman_fog_proxmox/proxmox_vm_commands.rb index ef9ab09e1..edda4eba2 100644 --- a/app/models/foreman_fog_proxmox/proxmox_vm_commands.rb +++ b/app/models/foreman_fog_proxmox/proxmox_vm_commands.rb @@ -56,7 +56,24 @@ def create_vm(args = {}) end def assign_vmid(vmid, node) - (vmid < 1) ? node.servers.next_id : vmid + vmid = node.servers.next_id if vmid < 1 + max_retries = 5 + retries = 0 + while vmid_exists?(vmid) && retries < max_retries + logger.warn("assign_vmid: VMID #{vmid} already exists, requesting new one (attempt #{retries + 1})") + vmid = node.servers.next_id + retries += 1 + end + raise ::Foreman::Exception, N_('Unable to find a free VMID after %s attempts') % max_retries if vmid_exists?(vmid) + vmid + end + + def vmid_exists?(vmid) + nodes.any? do |search_node| + search_node.servers.get(vmid) || search_node.containers.get(vmid) + rescue Fog::Errors::NotFound + false + end end def compute_clone_attributes(args, container, type) diff --git a/test/factories/foreman_fog_proxmox/proxmox_node_mock_factory.rb b/test/factories/foreman_fog_proxmox/proxmox_node_mock_factory.rb index a5f1a4623..ec08830b7 100644 --- a/test/factories/foreman_fog_proxmox/proxmox_node_mock_factory.rb +++ b/test/factories/foreman_fog_proxmox/proxmox_node_mock_factory.rb @@ -19,11 +19,18 @@ module ForemanFogProxmox module ProxmoxNodeMockFactory + def mock_empty_collection + col = mock('collection') + col.stubs(:get).returns(nil) + col + end + def mock_node_servers(cr, servers) node = mock('node') nodes = mock('nodes') node.stubs(:node).returns('proxmox') node.stubs(:servers).returns(servers) + node.stubs(:containers).returns(mock_empty_collection) nodes.stubs(:get).returns(node) nodes.stubs(:all).returns([node]) client = mock('client') @@ -37,6 +44,7 @@ def mock_node_containers(cr, containers) nodes = mock('nodes') node.stubs(:node).returns('proxmox') node.stubs(:containers).returns(containers) + node.stubs(:servers).returns(mock_empty_collection) nodes.stubs(:get).returns(node) nodes.stubs(:all).returns([node]) client = mock('client') diff --git a/test/unit/foreman_fog_proxmox/proxmox_vm_commands_container_test.rb b/test/unit/foreman_fog_proxmox/proxmox_vm_commands_container_test.rb index 6e0869b9f..58b9039d8 100644 --- a/test/unit/foreman_fog_proxmox/proxmox_vm_commands_container_test.rb +++ b/test/unit/foreman_fog_proxmox/proxmox_vm_commands_container_test.rb @@ -256,7 +256,9 @@ class ProxmoxVMCommandsContainerTest < ActiveSupport::TestCase args = { vmid: '100', type: 'lxc', node_id: 'proxmox', start_after_create: '0' } servers = mock('servers') servers.stubs(:id_valid?).returns(true) + servers.stubs(:get).returns(nil) containers = mock('containers') + containers.stubs(:get).returns(nil) containers.stubs(:create).with(args) cr = mock_node_servers_containers(ForemanFogProxmox::Proxmox.new, servers, containers) cr.stubs(:parse_typed_vm).with(args, 'lxc').returns(args) @@ -269,7 +271,9 @@ class ProxmoxVMCommandsContainerTest < ActiveSupport::TestCase args = { vmid: '100', type: 'lxc', node_id: 'proxmox', start_after_create: '1' } servers = mock('servers') servers.stubs(:id_valid?).returns(true) + servers.stubs(:get).returns(nil) containers = mock('containers') + containers.stubs(:get).returns(nil) vm = mock('vm') containers.stubs(:create).with(args).returns(vm) cr = mock_node_servers_containers(ForemanFogProxmox::Proxmox.new, servers, containers) @@ -283,7 +287,9 @@ class ProxmoxVMCommandsContainerTest < ActiveSupport::TestCase args = { vmid: '100', type: 'lxc', node_id: 'proxmox', start_after_create: '0', pool: 'pool1' } servers = mock('servers') servers.stubs(:id_valid?).returns(true) + servers.stubs(:get).returns(nil) containers = mock('containers') + containers.stubs(:get).returns(nil) vm = mock('vm') containers.stubs(:create).with(args).returns(vm) cr = mock_node_servers_containers(ForemanFogProxmox::Proxmox.new, servers, containers) @@ -296,7 +302,9 @@ class ProxmoxVMCommandsContainerTest < ActiveSupport::TestCase args = { vmid: '100', type: 'lxc', image_id: '999', name: 'name' } servers = mock('servers') servers.stubs(:id_valid?).returns(true) + servers.stubs(:get).returns(nil) containers = mock('containers') + containers.stubs(:get).returns(nil) cr = mock_node_servers_containers(ForemanFogProxmox::Proxmox.new, servers, containers) vm = mock('vm') cr.expects(:clone_from_image).with('999', 100).returns(vm) diff --git a/test/unit/foreman_fog_proxmox/proxmox_vm_commands_server_create_test.rb b/test/unit/foreman_fog_proxmox/proxmox_vm_commands_server_create_test.rb index 5fe1de222..05a605527 100644 --- a/test/unit/foreman_fog_proxmox/proxmox_vm_commands_server_create_test.rb +++ b/test/unit/foreman_fog_proxmox/proxmox_vm_commands_server_create_test.rb @@ -35,6 +35,7 @@ class ProxmoxVMCommandsServerCreateTest < ActiveSupport::TestCase args = { vmid: '100' } servers = mock('servers') servers.stubs(:id_valid?).returns(false) + servers.stubs(:get).returns(nil) cr = mock_node_servers(ForemanFogProxmox::Proxmox.new, servers) err = assert_raises Foreman::Exception do cr.create_vm(args) @@ -47,6 +48,7 @@ class ProxmoxVMCommandsServerCreateTest < ActiveSupport::TestCase servers = mock('servers') servers.stubs(:id_valid?).returns(true) servers.stubs(:next_id).returns('101') + servers.stubs(:get).returns(nil) cr = mock_node_servers(ForemanFogProxmox::Proxmox.new, servers) cr.stubs(:parse_typed_vm).with(args, 'qemu').returns(args) servers.stubs(:create).with(args) @@ -59,6 +61,7 @@ class ProxmoxVMCommandsServerCreateTest < ActiveSupport::TestCase args = { vmid: '100', type: 'qemu', node_id: 'proxmox', start_after_create: '0' } servers = mock('servers') servers.stubs(:id_valid?).returns(true) + servers.stubs(:get).returns(nil) cr = mock_node_servers(ForemanFogProxmox::Proxmox.new, servers) cr.stubs(:parse_typed_vm).with(args, 'qemu').returns(args) servers.stubs(:create).with(args) @@ -71,6 +74,7 @@ class ProxmoxVMCommandsServerCreateTest < ActiveSupport::TestCase args = { vmid: '100', type: 'qemu', node_id: 'proxmox', start_after_create: '1' } servers = mock('servers') servers.stubs(:id_valid?).returns(true) + servers.stubs(:get).returns(nil) cr = mock_node_servers(ForemanFogProxmox::Proxmox.new, servers) cr.stubs(:parse_typed_vm).with(args, 'qemu').returns(args) vm = mock('vm') @@ -84,6 +88,7 @@ class ProxmoxVMCommandsServerCreateTest < ActiveSupport::TestCase args = { vmid: '100', type: 'qemu', node_id: 'proxmox', start_after_create: '0', pool: 'pool1' } servers = mock('servers') servers.stubs(:id_valid?).returns(true) + servers.stubs(:get).returns(nil) cr = mock_node_servers(ForemanFogProxmox::Proxmox.new, servers) cr.stubs(:parse_typed_vm).with(args, 'qemu').returns(args) vm = mock('vm') @@ -98,6 +103,8 @@ class ProxmoxVMCommandsServerCreateTest < ActiveSupport::TestCase servers = mock('servers') containers = mock('containers') servers.stubs(:id_valid?).returns(true) + servers.stubs(:get).returns(nil) + containers.stubs(:get).returns(nil) cr = mock_node_servers_containers(ForemanFogProxmox::Proxmox.new, servers, containers) vm = mock('vm') cr.expects(:clone_from_image).with('999', 100).returns(vm)