Project

General

Profile

Bug #14179

Interface/NIC compute attributes in API host creation overwritten with compute profile attributes

Added by Dominic Cleal over 3 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Host creation
Target version:
Difficulty:
Triaged:
Bugzilla link:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

Since #6342, compute attributes specified inside a NIC/interface via the host create API are overwritten with compute attributes from associated compute profiles. This has the same symptoms as ticket #11124, which fixed it before #6342 reintroduced it.

POST to /api/v2/hosts with the following parameters:

{
    "host": {
        "name": "niccrattrs",
        "hostgroup_name": "libvirt",
        "compute_resource_name": "localhost",
        "compute_attributes": {
            "cpus": 3
        },
        "interfaces_attributes": {
            "0": {
                "primary": "true",
                "compute_attributes": {
                    "model": "e1000" 
                }
            }
        }
    }
}

... results in a VM with the NIC model set in the compute profile assigned to the host group, rather than e1000 as per the NIC compute attributes.

In HostsController#create, the host compute attrs after ComputeAttributeMerge contains the nic_attributes from the profile:

[1] pry(#<Api::V2::HostsController>)> @host.compute_attributes
=> {"cpus"=>3,
 "memory"=>"1073741824",
 "nics_attributes"=>{"0"=>{"type"=>"network", "network"=>"default", "model"=>"virtio"}},
 "volumes_attributes"=>{"0"=>{"pool_name"=>"default", "capacity"=>"10G", "allocation"=>"0G", "format_type"=>"qcow2"}}}

Prior to #6342, these compute attributes wouldn't have been filled in - it changed the API behaviour to merge compute profile attributes into host.compute_attributes.

But the NIC's compute_attributes does contain the merged compute attributes from the compute profile plus the user's parameters, due to InterfaceMerge and #11124:

[2] pry(#<Api::V2::HostsController>)> @host.interfaces
=> [#<Nic::Managed id: nil, mac: nil, ip: nil, type: "Nic::Managed", name: "test13982a", host_id: nil, subnet_id: nil, domain_id: nil, attrs: {}, created_at: nil, updated_at: nil, provider: nil, username: nil, password: nil, virtual: false, link: true, identifier: nil, tag: "", attached_to: "", managed: true, mode: "balance-rr", attached_devices: "", bond_options: "", primary: true, provision: true, compute_attributes: {"type"=>"network", "network"=>"default", "model"=>"e1000", "from_profile"=>"libvirt"}>]

Later in Orchestration::Compute#add_interfaces_to_compute_attrs, the per-NIC compute attributes should be merged back into the host compute attributes, but the step is skipped (the return unless in this method) as the host compute attributes already contains nics_attributes:

[1] pry(#<Host::Managed>)> compute_attributes[attrs_name]
=> {"0"=>{"type"=>"network", "network"=>"default", "model"=>"virtio"}}
[2] pry(#<Host::Managed>)> compute_attributes
=> {"cpus"=>3,
 "memory"=>"1073741824",
 "nics_attributes"=>{"0"=>{"type"=>"network", "network"=>"default", "model"=>"virtio"}},
 "volumes_attributes"=>{"0"=>{"pool_name"=>"default", "capacity"=>"10G", "allocation"=>"0G", "format_type"=>"qcow2"}}}

Related issues

Related to Foreman - Bug #6342: Compute profile should be used when empty volumes and interfaces passed in compute_attributesClosed2014-06-23
Related to Foreman - Bug #11124: Interface compute attributes not merged from API request over compute profileClosed2015-07-15

Associated revisions

Revision 860cbb0a (diff)
Added by Dominic Cleal over 3 years ago

fixes #14179 - don't merge NIC compute attrs in ComputeProfileMerge

When creating a host over the API and specifying interface compute
attributes, the interface attributes were overwritten from the compute
profile. Example request:

{"host":{"interfaces_attributes":{"0":{"primary":"true","compute_attributes":{"model":"e1000"}}}, ...}}

ComputeProfileMerge stores :interfaces_attributes (name varies based on
the CR type) from the profile in host.compute_attributes, while
InterfaceMerge stores attributes in each interface. When
Orchestration::Compute#add_interfaces_to_compute_attrs runs to merge
per-interface attributes into host.compute_attributes, the step was
skipped as host.compute_attributes[:interfaces_attributes] already
existed from ComputeProfileMerge.

This change excludes interface attributes from being merged by
ComputeProfileMerge, so InterfaceMerge is responsible for copying them
into the interface.

Revision ff222ae6 (diff)
Added by Dominic Cleal over 3 years ago

fixes #14179 - don't merge NIC compute attrs in ComputeProfileMerge

When creating a host over the API and specifying interface compute
attributes, the interface attributes were overwritten from the compute
profile. Example request:

{"host":{"interfaces_attributes":{"0":{"primary":"true","compute_attributes":{"model":"e1000"}}}, ...}}

ComputeProfileMerge stores :interfaces_attributes (name varies based on
the CR type) from the profile in host.compute_attributes, while
InterfaceMerge stores attributes in each interface. When
Orchestration::Compute#add_interfaces_to_compute_attrs runs to merge
per-interface attributes into host.compute_attributes, the step was
skipped as host.compute_attributes[:interfaces_attributes] already
existed from ComputeProfileMerge.

This change excludes interface attributes from being merged by
ComputeProfileMerge, so InterfaceMerge is responsible for copying them
into the interface.

(cherry picked from commit 860cbb0a7c86f7669aad75c7011dbb2030aaede0)

History

#1 Updated by Dominic Cleal over 3 years ago

  • Related to Bug #6342: Compute profile should be used when empty volumes and interfaces passed in compute_attributes added

#2 Updated by Dominic Cleal over 3 years ago

  • Related to Bug #11124: Interface compute attributes not merged from API request over compute profile added

#3 Updated by Dominic Cleal over 3 years ago

  • Status changed from New to Assigned
  • Assignee set to Dominic Cleal

#4 Updated by The Foreman Bot over 3 years ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/3316 added

#5 Updated by Dominic Cleal over 3 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF