fix: Create host fails on 2.5.2

Fixes #207

app/helpers/proxmox_vm_config_helper.rb

@@ -25,10 +25,6 @@
 
 # Convert a foreman form server hash into a fog-proxmox server attributes hash
 module ProxmoxVmConfigHelper
-  KILO = 1024
-  MEGA = KILO * KILO
-  GIGA = KILO * MEGA
-
   def object_to_config_hash(vm, type)
     vm_h = ActiveSupport::HashWithIndifferentAccess.new
     main_a = ['vmid']
@@ -43,12 +39,6 @@ def object_to_config_hash(vm, type)
     vm_h
   end
 
-  def convert_memory_size(config_hash, key)
-    # default unit memory size is Mb
-    memory = (config_hash[key].to_i / MEGA).to_s == '0' ? config_hash[key] : (config_hash[key].to_i / MEGA).to_s
-    config_hash.store(key, memory)
-  end
-
   def general_a(type)
     general_a = ['node_id', 'type', 'config_attributes', 'volumes_attributes', 'interfaces_attributes']
     general_a += ['firmware_type', 'provision_method', 'container_volumes', 'server_volumes', 'start_after_create']
@@ -61,7 +51,7 @@ def config_typed_keys(type)
     main_a = ['name', 'type', 'node_id', 'vmid', 'interfaces', 'mount_points', 'disks']
     case type
     when 'lxc'
-      cpu_a = ['arch', 'cpulimit', 'cpuunits', 'cores', 'sockets']
+      cpu_a = ['arch', 'cpulimit', 'cpuunits']
       memory_a = ['memory', 'swap']
       ostemplate_a = ['ostemplate', 'ostemplate_storage', 'ostemplate_file']
       keys.store(:ostemplate, ostemplate_a)
@@ -78,10 +68,6 @@ def config_typed_keys(type)
     keys
   end
 
-  def convert_memory_sizes(args)
-    ['memory', 'balloon', 'shares', 'swap'].each { |key| convert_memory_size(args['config_attributes'], key) }
-  end
-
   def config_general_or_ostemplate_key?(key)
     config_typed_keys('lxc')[:general].include?(key) || config_typed_keys(type)[:ostemplate].include?(key)
   end
@@ -138,28 +124,23 @@ def parsed_typed_config(args, type)
   end
 
   def parse_typed_memory(args, type)
-    memory = {}
     ForemanFogProxmox::HashCollection.remove_empty_values(args)
-    config_typed_keys(type)[:memory].each do |key|
-      ForemanFogProxmox::HashCollection.add_and_format_element(memory, key.to_sym, args, key, :to_i)
-    end
-    memory
+    logger.debug("parse_typed_memory(#{type}): args=#{args}")
+    args
   end
 
   def parse_typed_cpu(args, type)
     cpu = {}
     ForemanFogProxmox::HashCollection.remove_empty_values(args)
-    if type == 'qemu'
+    case type
+    when 'qemu'
       logger.debug("parse_typed_cpu(#{type}): args=#{args}")
       cpu_flattened = Fog::Proxmox::CpuHelper.flatten(args)
       cpu_flattened = args[:cpu] if cpu_flattened.empty?
       logger.debug("parse_typed_cpu(#{type}): cpu_flattened=#{cpu_flattened}")
-      ForemanFogProxmox::HashCollection.remove_empty_values(args)
-      ForemanFogProxmox::HashCollection.remove_keys(args, config_typed_keys('qemu')[:cpu])
       args.each_value(&:to_i)
       cpu = { cpu: cpu_flattened }
-    end
-    if type == 'lxc'
+    when 'lxc'
       config_typed_keys('lxc')[:cpu].each do |key|
         ForemanFogProxmox::HashCollection.add_and_format_element(cpu, key.to_sym, args, key)
       end

app/helpers/proxmox_vm_volumes_helper.rb

@@ -26,9 +26,6 @@
 module ProxmoxVmVolumesHelper
   include ProxmoxVmCdromHelper
   include ProxmoxVmCloudinitHelper
-  KILO = 1024
-  MEGA = KILO * KILO
-  GIGA = KILO * MEGA
 
   def add_disk_options(disk, args)
     options = ForemanFogProxmox::HashCollection.new_hash_reject_keys(args,
@@ -52,12 +49,14 @@ def parsed_typed_volumes(args, type, parsed_vm)
   end
 
   def parse_hard_disk_volume(args)
+    logger.debug(format(_('parse_hard_disk_volume(): args=%<args>s'), args: args))
     disk = {}
     disk[:id] = args['id'] if args.key?('id')
     disk[:volid] = args['volid'] if args.key?('volid')
     disk[:storage] = args['storage'].to_s if args.key?('storage')
     disk[:size] = args['size'].to_i if args.key?('size')
-    add_disk_options(disk, args)
+    add_disk_options(disk, args) unless args.key?('options')
+    disk[:options] = args['options'] if args.key?('options')
     disk.key?(:storage) ? disk : {}
   end
 
@@ -93,22 +92,6 @@ def parse_typed_volumes(args, type)
     volumes
   end
 
-  def convert_volumes_size(args)
-    args['volumes_attributes'].each_value do |value|
-      value['size'] = (value['size'].to_i / GIGA).to_s unless ForemanFogProxmox::Value.empty?(value['size'])
-    end
-  end
-
-  def convert_sizes(args)
-    convert_memory_size(args['config_attributes'], 'memory')
-    convert_memory_size(args['config_attributes'], 'balloon')
-    convert_memory_size(args['config_attributes'], 'shares')
-    convert_memory_size(args['config_attributes'], 'swap')
-    args['volumes_attributes'].each_value do |value|
-      value['size'] = (value['size'].to_i / GIGA).to_s unless ForemanFogProxmox::Value.empty?(value['size'])
-    end
-  end
-
   def remove_volume_keys(args)
     if args.key?('volumes_attributes')
       args['volumes_attributes'].each_value do |volume_attributes|

app/models/concerns/fog_extensions/proxmox/server.rb

@@ -59,8 +59,8 @@ def state
       delegate :description, to: :config
 
       def vm_description
-        format(_('Type %<type>s, node %<node>s, %<cpus>s CPUs and %<memory>s MB memory'), type: type, node: node_id,
-cpus: config.cores || '0', memory: config.memory / (1024 * 1024) || '0')
+        format(_('Type %<type>s, node %<node>s, %<cpus>s CPUs and %<memory>s MB memory'),
+          type: type, node: node_id, cpus: config.cores || '0', memory: config.memory)
       end
 
       def select_nic(fog_nics, nic)

app/models/foreman_fog_proxmox/proxmox_vm_commands.rb

@@ -42,7 +42,6 @@ def create_vm(args = {})
       if image_id
         clone_from_image(image_id, args, vmid)
       else
-        convert_sizes(args)
         remove_volume_keys(args)
         logger.warn("create vm: args=#{args}")
         vm = node.send(vm_collection(type)).create(parse_typed_vm(args, type))
@@ -91,7 +90,6 @@ def save_vm(uuid, new_attributes)
       elsif vm.node_id != node_id
         vm.migrate(node_id)
       else
-        convert_memory_sizes(new_attributes)
         parsed_attr = parse_typed_vm(
           ForemanFogProxmox::HashCollection.new_hash_reject_keys(new_attributes,
             ['volumes_attributes']).merge(type: vm.type), vm.type

app/models/foreman_fog_proxmox/proxmox_vm_new.rb

@@ -36,7 +36,7 @@ def cloudinit_defaults
 
     def hard_disk_typed_defaults(vm_type)
       options = {}
-      volume_attributes_h = { storage: storages.first.identity.to_s, size: (8 * GIGA) }
+      volume_attributes_h = { storage: storages.first.identity.to_s, size: '8' }
       case vm_type
       when 'qemu'
         controller = 'virtio'
@@ -140,21 +140,21 @@ def config_attributes(type = 'qemu')
       case type
       when 'qemu'
         config_attributes = {
-          cores: 1,
-          sockets: 1,
-          kvm: 0,
+          cores: '1',
+          sockets: '1',
+          kvm: '0',
           vga: 'std',
-          memory: 512 * MEGA,
+          memory: '1024',
           ostype: 'l26',
           cpu: 'cputype=kvm64',
           scsihw: 'virtio-scsi-pci',
-          templated: 0,
+          templated: '0',
         }
         config_attributes = config_attributes
       when 'lxc'
         config_attributes = {
-          memory: 512 * MEGA,
-          templated: 0,
+          memory: '1024',
+          templated: '0',
         }
       end
       config_attributes
@@ -195,6 +195,8 @@ def new_typed_vm(new_attr, type)
       logger.debug("new_typed_vm(#{type}): options=#{options}")
       vm_h = parse_typed_vm(options, type).deep_symbolize_keys
       logger.debug("new_typed_vm(#{type}): vm_h=#{vm_h}")
+      vm_h = vm_h.merge(vm_typed_instance_defaults(type)) if vm_h.empty?
+      logger.debug(format(_('new_typed_vm(%<type>s) with vm_typed_instance_defaults: vm_h=%<vm_h>s'), type: type, vm_h: vm_h))
       node.send(vm_collection(type)).new(vm_h)
     end
   end

app/models/foreman_fog_proxmox/proxmox_vm_queries.rb

@@ -61,10 +61,9 @@ def find_vm_by_uuid(uuid)
       nodes.each do |node|
         vm = find_vm_in_servers_by_vmid(node.servers, vmid)
         vm ||= find_vm_in_servers_by_vmid(node.containers, vmid)
-        unless vm.nil?
-          logger.debug("found vm #{vmid} on node #{node.node}")
-          break
-        end
+        next if vm.nil?
+        logger.debug("found vm #{vmid} on node #{node.node}")
+        break
       end
       vm
     end

app/models/foreman_fog_proxmox/proxmox_volumes.rb

@@ -36,7 +36,7 @@ def delete_volume(vm, id, volume_attributes)
     def volume_options(vm, id, volume_attributes)
       options = {}
       options.store(:mp, volume_attributes['mp']) if vm.container? && id != 'rootfs'
-      options.store(:cache, volume_attributes['cache']) unless vm.container?
+      options.store(:cache, volume_attributes['cache']) unless vm.container? || volume_attributes['cache'].empty?
       options
     end
 
@@ -59,7 +59,7 @@ def update_cdrom(vm, disk, volume_attributes)
     end
 
     def extend_volume(vm, id, diff_size)
-      extension = '+' + (diff_size / GIGA).to_s + 'G'
+      extension = "+#{diff_size}G"
       logger.info("vm #{vm.identity} extend volume #{id} to #{extension}")
       vm.extend(id, extension)
     end
@@ -82,11 +82,13 @@ def update_volume(vm, disk, volume_attributes)
       if volume_type?(volume_attributes, 'cdrom')
         update_cdrom(vm, disk, volume_attributes)
       elsif volume_type?(volume_attributes, 'hard_disk')
-        diff_size = volume_attributes['size'].to_i - disk.size if volume_attributes['size'] && disk.size
+        diff_size = volume_attributes['size'].to_i - disk.size.to_i if volume_attributes['size'] && disk.size
         unless diff_size >= 0
           raise ::Foreman::Exception,
             format(_('Unable to shrink %<id>s size. Proxmox allows only increasing size.'), id: id)
         end
+        diff_size = volume_attributes['size'].to_i - disk.size.to_i if volume_attributes['size'] && disk.size
+        raise ::Foreman::Exception, format(_('Unable to shrink %<id>s size. Proxmox allows only increasing size.'), id: id) unless diff_size >= 0
 
         new_storage = volume_attributes['storage']
 
@@ -101,7 +103,13 @@ def update_volume(vm, disk, volume_attributes)
     end
 
     def volume_exists?(vm, volume_attributes)
-      vm.attributes.key?(volume_attributes['id'])
+      disk = vm.config.disks.get(volume_attributes['id'])
+      exists = false
+      return exists unless disk
+
+      exists = !volume_attributes['volid'].empty? if disk.hard_disk? || disk.cloud_init?
+      exists = !volume_attributes['cdrom'].empty? if disk.cdrom?
+      exists
     end
 
     def volume_to_delete?(volume_attributes)
@@ -124,7 +132,7 @@ def add_volume(vm, id, volume_attributes)
       if volume_type?(volume_attributes, 'hard_disk')
         options = volume_options(vm, id, volume_attributes)
         disk_attributes[:storage] = volume_attributes['storage']
-        disk_attributes[:size] = (volume_attributes['size'].to_i / GIGA).to_s
+        disk_attributes[:size] = volume_attributes['size']
       elsif volume_type?(volume_attributes, 'cdrom')
         disk_attributes[:volid] = volume_attributes[:iso]
       elsif volume_type?(volume_attributes, 'cloud_init')

app/views/compute_resources_vms/form/proxmox/container/_config.html.erb

@@ -28,8 +28,8 @@ along with ForemanFogProxmox. If not, see <http://www.gnu.org/licenses/>. %>
   <%= counter_f f, :cpuunits, :class => "input-mini", :label => _('CPU units'), :label_size => "col-md-2" %>
 <% end %>
 <%= field_set_tag _("Memory"), :id => "container_config_memory", :class => 'hide', :disabled => !container do %>
-  <%= byte_size_f f, :memory, :class => "input-mini", :label => _('Memory'), :label_size => "col-md-2" %>
-  <%= byte_size_f f, :swap, :class => "input-mini", :label => _('Swap'), :label_size => "col-md-2" %>
+  <%= text_f f, :memory, :class => "input-mini", :label => _('Memory (MB)'), :label_size => "col-md-2" %>
+  <%= text_f f, :swap, :class => "input-mini", :label => _('Swap (MB)'), :label_size => "col-md-2" %>
 <% end %>
 <%= field_set_tag _("DNS"), :id => "container_config_dns", :class => 'hide', :disabled => !container do %>
   <%= text_f f, :hostname, :label => _('Hostname'), :label_size => "col-md-2", :disabled => true %>

app/views/compute_resources_vms/form/proxmox/container/_volume_mp.html.erb

@@ -23,5 +23,5 @@ along with ForemanFogProxmox. If not, see <http://www.gnu.org/licenses/>. %>
     <%= select_f f, :storage, compute_resource.storages(node_id), :storage, :storage, { }, :label => _('Storage'), :label_size => "col-md-2" %>
     <%= text_f f, :mp, :label => _('Path'), :label_size => "col-md-2", :required => true, :help_inline => _("e.g. /path/to/") %>
     <%= text_f f, :device, :label => _('Device'), :label_size => "col-md-2", :class => ('hide' if f.object.rootfs?), :disabled => (!new_volume || f.object.rootfs?), :'data-soft-max' => 10 %>
-    <%= byte_size_f f, :size, :class => "input-mini", :label => _("Size"), :label_size => "col-md-2" %>
+    <%= text_f f, :size, :class => "input-mini", :label => _("Size (GB)"), :label_size => "col-md-2" %>
 <% end %>

app/views/compute_resources_vms/form/proxmox/container/_volume_rootfs.html.erb

@@ -20,5 +20,5 @@ along with ForemanFogProxmox. If not, see <http://www.gnu.org/licenses/>. %>
 <%= field_set_tag _("Rootfs"), :id => "container_volume_rootfs", :class => ('hide' unless container), :disabled => !container do %> 
     <%= f.hidden_field :volid if !new_vm %>
     <%= select_f f, :storage, compute_resource.storages(node_id), :storage, :storage, { }, :label => _('Storage'), :label_size => "col-md-2" %>
-    <%= byte_size_f f, :size, :class => "input-mini", :label => _("Size"), :label_size => "col-md-2" %>
+    <%= text_f f, :size, :class => "input-mini", :label => _("Size (GB)"), :label_size => "col-md-2" %>
 <% end %>

app/views/compute_resources_vms/form/proxmox/server/_config.html.erb

@@ -45,9 +45,9 @@ along with ForemanFogProxmox. If not, see <http://www.gnu.org/licenses/>. %>
   <% end %>
 <% end %>
 <%= field_set_tag _("Memory"), :id => "server_config_memory", :class => 'hide', :disabled => !server do %>
-  <%= byte_size_f f, :memory, :class => "input-mini", :label => _('Memory'), :label_size => "col-md-2" %>
-  <%= byte_size_f f, :balloon, :class => "input-mini", :label => _('Minimum memory'), :label_size => "col-md-2" %>
-  <%= counter_f f, :shares, :class => "input-mini", :label => _('Shares'), :label_size => "col-md-2" %>
+  <%= text_f f, :memory, :class => "input-mini", :label => _('Memory (MB)'), :label_size => "col-md-2" %>
+  <%= text_f f, :balloon, :class => "input-mini", :label => _('Minimum memory (MB)'), :label_size => "col-md-2" %>
+  <%= text_f f, :shares, :class => "input-mini", :label => _('Shares (MB)'), :label_size => "col-md-2" %>
 <% end %>
 <%= field_set_tag _("Operating System"), :id => "server_config_os", :class => 'hide', :disabled => !server do %>
   <%= select_f f, :ostype, proxmox_operating_systems_map, :id, :name, { :include_blank => true }, :label => _('OS type'), :label_size => "col-md-2" %>

app/views/compute_resources_vms/form/proxmox/server/_volume_hard_disk.html.erb

@@ -28,5 +28,5 @@ along with ForemanFogProxmox. If not, see <http://www.gnu.org/licenses/>. %>
     <%= select_f f, :controller, proxmox_controllers_map, :id, :name, { }, :label => _('Controller'), :label_size => "col-md-2", :disabled => !new_volume, :onchange => 'controllerSelected(this)' %>
     <%= text_f f, :device, :label => _('Device'), :label_size => "col-md-2", :disabled => !new_volume, :'data-min' => 0, :'data-soft-max' => proxmox_max_device(f.object.controller), :onchange => 'deviceSelected(this)' %>
     <%= select_f f, :cache, proxmox_caches_map, :id, :name, { include_blank: true }, :label => _('Cache'), :label_size => "col-md-2" %>
-    <%= byte_size_f f, :size, :class => "input-mini", :label => _("Size"), :label_size => "col-md-2" %>
+    <%= text_f f, :size, :class => "input-mini", :label => _("Size (GB)"), :label_size => "col-md-2", :disabled => !hard_disk %>
 <% end %>

foreman_fog_proxmox.gemspec

@@ -34,7 +34,7 @@ Gem::Specification.new do |s|
   s.test_files = Dir['test/**/*']
 
   s.add_dependency 'deface'
-  s.add_dependency 'fog-proxmox', '~> 0.14'
+  s.add_dependency 'fog-proxmox', '~> 0.15'
   s.add_development_dependency 'rdoc'
   s.add_development_dependency 'rubocop'
   s.add_development_dependency 'simplecov'