Skip to content
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 #27652 - Fix interfaces when creating a host #439

Merged
merged 1 commit into from Aug 27, 2019

Conversation

@shiramax
Copy link
Contributor

shiramax commented Aug 19, 2019

Creating a host with interfaces didn't work properly.
the reason for that is that in the compute profile PR, I removed the "compute_" string from each attribute in the interfaces, that worked for the compute profile but didn't work for the host creation.
the reason for that is this functiom:

 def interfaces_attributes
        return [] if option_interface_list.empty?

        # move each attribute starting with "compute_" to compute_attributes
        option_interface_list.collect do |nic|
          compute_attributes = {}
          nic.keys.each do |key|
            if key.start_with? 'compute_'
              compute_attributes[key.gsub('compute_', '')] = nic.delete(key)
            end
          end
          subnet_name = nic.delete('subnet')
          nic['subnet_id'] ||= subnet_id(subnet_name) if subnet_name
          domain_name = nic.delete('domain')
          nic['domain_id'] ||= domain_id(domain_name) if domain_name
          nic['compute_attributes'] = compute_attributes unless compute_attributes.empty?
          nic
        end
      end

that puts the interfaces attributes (that are spesfic to a compute resource) inside "compute_attributes"
the function checks if the attribute starts with "compute" and then puts it under "compute_attributes"

so I needed to restore the "compute_" string and add a function in compute_attribute that removes it.

@mbacovsky

This comment has been minimized.

Copy link
Member

mbacovsky commented Aug 20, 2019

@shiramax, the code looks reasonable I need to do some testing though.
My only concern so far is interface_attributes in VmWare. They were prefixed before and with this patch the 'compute_' will be stripped down, correct? Will it still work?

@shiramax

This comment has been minimized.

Copy link
Contributor Author

shiramax commented Aug 20, 2019

@mbacovsky I think that VMWare wasn't tested before, But see :
/foreman/app/models/compute_resources/foreman/model/vmware.rb

    def vm_compute_attributes(vm)
      vm_attrs = super
      dc_networks = networks
      interfaces = vm.interfaces || []
      vm_attrs[:interfaces_attributes] = interfaces.each_with_index.each_with_object({}) do |(interface, index), hsh|
        network = dc_networks.detect { |n| [n.id, n.name].include?(interface.network) }
        raise Foreman::Exception.new(N_("Could not find network %s on VMWare compute resource"), interface.network) unless network
        interface_attrs = {}
        interface_attrs[:compute_attributes] = {}
        interface_attrs[:mac] = interface.mac
        interface_attrs[:compute_attributes][:network] = network.name
        interface_attrs[:compute_attributes][:type] = interface.type.to_s.split('::').last
        hsh[index.to_s] = interface_attrs
      end
      vm_attrs[:scsi_controllers] = vm.scsi_controllers.map do |controller|
        controller.attributes
      end
      vm_attrs
    end
@mbacovsky

This comment has been minimized.

Copy link
Member

mbacovsky commented Aug 21, 2019

👍 @shiramax it worked for me for libvirt. I was able to create interfaces in compute profile as well as in host.

@ofedoren don't you have env to test this for vmware?

@ofedoren

This comment has been minimized.

Copy link
Member

ofedoren commented Aug 21, 2019

@mbacovsky, unfortunatelly I don't have it currenlty. But I hope @ezr-ondrej can help with this one.

Copy link
Member

ezr-ondrej left a comment

Works fine for Vmware :)

Copy link
Member

mbacovsky left a comment

👍 LGTM, Thanks @shiramax!

@mbacovsky mbacovsky merged commit 3450905 into theforeman:master Aug 27, 2019
2 checks passed
2 checks passed
Redmine issues Valid issues
Details
hammer Build finished. 5337 tests run, 0 skipped, 0 failed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.