Hi Scott,
The patch works fine.
However, I noticed some code about enum entries that is commented out. Uncommenting it seems to break the tests but that is not obvious from the code.
Could you please either delete these parts or add an explanation to them about why they are commented out and what to do to make it work again?
Other than that, ACK.
Thomas
----- "Scott Seago" sseago@redhat.com wrote:
Removes all references to core flavors and replaces them with hardware profiles. Updates aggregator model to include the range and enum properties for hardware profiles.
To use this patch, you need to rebuild the aggregator database, and rebuild the client gem with this patch:
[PATCH core] update to hardware profile client to include new property metadata.
Signed-off-by: Scott Seago sseago@redhat.com
src/app/models/hardware_profile.rb | 43 ++++++- src/app/models/hardware_profile_property.rb | 122 ++++++++++++++++++++ src/app/models/property_enum_entry.rb | 37 ++++++ src/app/models/provider.rb | 21 ++-- src/app/views/hardware_profiles/_list.html.erb | 8 +- .../20090804135630_create_hardware_profiles.rb | 25 ++++- src/spec/factories/hardware_profile.rb | 40 ++++--- src/spec/factories/hardware_profile_property.rb | 122 ++++++++++++++++++++ src/spec/factories/pool.rb | 2 +- src/spec/factories/property_enum_entry.rb | 26 ++++ src/spec/models/hardware_profile_spec.rb | 17 +--- 11 files changed, 407 insertions(+), 56 deletions(-) create mode 100644 src/app/models/hardware_profile_property.rb create mode 100644 src/app/models/property_enum_entry.rb create mode 100644 src/spec/factories/hardware_profile_property.rb create mode 100644 src/spec/factories/property_enum_entry.rb
diff --git a/src/app/models/hardware_profile.rb b/src/app/models/hardware_profile.rb index d98c2f9..a92b658 100644 --- a/src/app/models/hardware_profile.rb +++ b/src/app/models/hardware_profile.rb @@ -25,6 +25,15 @@ class HardwareProfile < ActiveRecord::Base :foreign_key => "provider_hardware_profile_id" belongs_to :provider
- belongs_to :memory, :class_name =>
"HardwareProfileProperty",
:dependent => :destroy
- belongs_to :storage, :class_name =>
"HardwareProfileProperty",
:dependent => :destroy
- belongs_to :cpu, :class_name =>
"HardwareProfileProperty",
:dependent => :destroy
- belongs_to :architecture, :class_name =>
"HardwareProfileProperty",
:dependent => :destroy
- has_and_belongs_to_many :aggregator_hardware_profiles, :class_name => "HardwareProfile", :join_table => "hardware_profile_map",
@@ -43,12 +52,10 @@ class HardwareProfile < ActiveRecord::Base validates_presence_of :name validates_uniqueness_of :name, :scope => [:provider_id]
- validates_presence_of :storage
- validates_numericality_of :storage, :greater_than => 0
- validates_presence_of :memory
- validates_numericality_of :memory, :greater_than => 0
- validates_presence_of :architecture, :if => :provider
validates_associated :memory
validates_associated :storage
validates_associated :cpu
validates_associated :architecture
def provider_hardware_profile? !provider.nil?
@@ -68,4 +75,28 @@ class HardwareProfile < ActiveRecord::Base end end end
- def add_properties(api_profile)
- self.memory = new_property(api_profile.memory)
- self.storage = new_property(api_profile.storage)
- self.cpu = new_property(api_profile.cpu)
- self.architecture = new_property(api_profile.architecture)
- end
- def new_property(prop)
- return nil unless prop.present?
- the_property = HardwareProfileProperty.new(:name => prop.name,
:kind => prop.kind,
:unit => prop.unit,
:value => prop.value)
- case prop.kind
- when HardwareProfileProperty::RANGE
the_property.range_first = prop.range.first
the_property.range_last = prop.range.last
- when HardwareProfileProperty::ENUM
the_property.property_enum_entries = prop.enum.entries.collect
do |entry|
PropertyEnumEntry.new(:value => entry,
:hardware_profile_property => the_property)
end
- end
- the_property
- end
end diff --git a/src/app/models/hardware_profile_property.rb b/src/app/models/hardware_profile_property.rb new file mode 100644 index 0000000..1da5ba7 --- /dev/null +++ b/src/app/models/hardware_profile_property.rb @@ -0,0 +1,122 @@ +# +# Copyright (C) 2010 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, +# MA 02110-1301, USA. A copy of the GNU General Public License is +# also available at http://www.gnu.org/copyleft/gpl.html.
+# Filters added to this controller apply to all controllers in the application. +# Likewise, all the methods added will be available for all controllers.
+class HardwareProfileProperty < ActiveRecord::Base
- MEMORY = "memory"
- STORAGE = "storage"
- CPU = "cpu"
- ARCHITECTURE = "architecture"
- FIXED = "fixed"
- RANGE = "range"
- ENUM = "enum"
- UNIT_MB = "MB"
- UNIT_GB = "GB"
- UNIT_LABEL = "label"
- UNIT_COUNT = "count"
- has_many :property_enum_entries
- validates_presence_of :name
- validates_inclusion_of :name,
:in => [MEMORY, STORAGE, CPU, ARCHITECTURE]
- validates_presence_of :kind
- validates_inclusion_of :kind,
:in => [FIXED, RANGE, ENUM]
- validates_presence_of :unit
- validates_presence_of :value
- validates_numericality_of :value, :greater_than => 0,
:if => Proc.new{|p| p.name == MEMORY or p.name ==
STORAGE }
- validates_numericality_of :value, :greater_than => 0,
:if => Proc.new{|p| p.name == CPU }, :only_integer =>
true
- validates_numericality_of :range_first, :greater_than => 0,
:if => Proc.new{|p| (p.name == MEMORY or p.name ==
STORAGE) and
p.kind == RANGE}
- validates_numericality_of :range_last, :greater_than => 0,
:if => Proc.new{|p| (p.name == MEMORY or p.name ==
STORAGE) and
p.kind == RANGE}
- validates_numericality_of :range_first, :greater_than => 0,
:if => Proc.new{|p| (p.name == CPU) and
p.kind == RANGE}, :only_integer
=> true
- validates_numericality_of :range_last, :greater_than => 0,
:if => Proc.new{|p| (p.name == CPU) and
p.kind == RANGE}, :only_integer
=> true
- validates_associated :property_enum_entries
- def validate
- case name
- when MEMORY
unless unit == UNIT_MB
errors.add(:unit, "Memory must be specified in MB")
end
- when STORAGE
unless unit == UNIT_GB
errors.add(:unit, "Storage must be specified in GB")
end
- when CPU
unless unit == UNIT_COUNT
errors.add(:unit, "CPUs must be specified as a count")
end
- when ARCHITECTURE
unless unit == UNIT_LABEL
errors.add(:unit, "Architecture must be specified as a
label")
end
- end
- if kind==RANGE
if range_first.nil?
errors.add(:range_first,
"Range beginning must be specified for range
properties")
end
if range_last.nil?
errors.add(:range_last,
"Range ending must be specified for range
properties")
end
- else
unless range_first.nil?
errors.add(:range_first,
"Range beginning must only be specified for range
properties")
end
unless range_last.nil?
errors.add(:range_last,
"Range ending must only be specified for range
properties")
end
- end
+# if kind==ENUM +# if property_enum_entries.empty? +# errors.add(:property_enum_entries, +# "Enum values must be specified for enum properties") +# end +# else +# unless property_enum_entries.empty? +# errors.add(:property_enum_entries, +# "Enum values must only be specified for enum properties") +# end +# end
Commented out, breaks the tests otherwise.
- end
+end
diff --git a/src/app/models/property_enum_entry.rb b/src/app/models/property_enum_entry.rb new file mode 100644 index 0000000..eb246ff --- /dev/null +++ b/src/app/models/property_enum_entry.rb @@ -0,0 +1,37 @@ +# +# Copyright (C) 2010 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, +# MA 02110-1301, USA. A copy of the GNU General Public License is +# also available at http://www.gnu.org/copyleft/gpl.html.
+# Filters added to this controller apply to all controllers in the application. +# Likewise, all the methods added will be available for all controllers.
+class PropertyEnumEntry < ActiveRecord::Base
- belongs_to :hardware_profile_property
- validates_presence_of :value
- validates_presence_of :hardware_profile_property
- validates_numericality_of :value, :greater_than => 0,
:if => Proc.new{|p| p.hardware_profile_property.name
==
HardwareProfileProperty::MEMORY
or
p.hardware_profile_property.name ==
HardwareProfileProperty::STORAGE
}
- validates_numericality_of :value, :greater_than => 0, :only_integer
=> true,
:if => Proc.new{|p| p.hardware_profile_property.name
==
HardwareProfileProperty::CPU }
+end diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb index 20b9e1e..0a0120f 100644 --- a/src/app/models/provider.rb +++ b/src/app/models/provider.rb @@ -50,16 +50,17 @@ class Provider < ActiveRecord::Base
def populate_hardware_profiles # FIXME: once API has hw profiles, change the below
- hardware_profiles = connect.flavors
- hardware_profiles = connect.hardware_profiles # FIXME: this should probably be in the same transaction as
provider.save self.transaction do hardware_profiles.each do |hardware_profile|
ar_hardware_profile = HardwareProfile.new(:external_key =>
hardware_profile.id,
:name => hardware_profile.name ?
hardware_profile.name : hardware_profile.id,
:memory => hardware_profile.memory,
:storage => hardware_profile.storage,
:architecture =>
hardware_profile.architecture,
:provider_id => id)
ar_hardware_profile = HardwareProfile.new(:external_key =>
hardware_profile.id,
:name =>
hardware_profile.name ?
hardware_profile.name :
hardware_profile.id,
:provider_id =>
id)
ar_hardware_profile.add_properties(hardware_profile) ar_hardware_profile.save! front_hwp = HardwareProfile.new(:external_key => name +
@@ -67,10 +68,8 @@ class Provider < ActiveRecord::Base
ar_hardware_profile.external_key, :name => name +
Realm::AGGREGATOR_REALM_ACCOUNT_DELIMITER +
ar_hardware_profile.name,
:memory =>
ar_hardware_profile.memory,
:storage =>
ar_hardware_profile.storage,
:architecture =>
ar_hardware_profile.architecture)
ar_hardware_profile.name)
front_hwp.add_properties(hardware_profile) front_hwp.provider_hardware_profiles << ar_hardware_profile front_hwp.save! end
diff --git a/src/app/views/hardware_profiles/_list.html.erb b/src/app/views/hardware_profiles/_list.html.erb index 6b53194..1125c09 100644 --- a/src/app/views/hardware_profiles/_list.html.erb +++ b/src/app/views/hardware_profiles/_list.html.erb @@ -7,6 +7,7 @@ <th scope="col">Name</th> <th scope="col">Memory</th> <th scope="col">Storage</th>
<th scope="col">CPU</th> <th scope="col">Architecture</th> </tr> </thead>
@@ -14,9 +15,10 @@ <%@hardware_profiles.each {|hp| %> <tr> <td><%= hp.name %></td>
<td><%= hp.memory %></td>
<td><%= hp.storage %></td>
<td><%= hp.architecture %></td>
<td><%= hp.memory.value if hp.memory %></td>
<td><%= hp.storage.value if hp.storage %></td>
<td><%= hp.cpu.value if hp.cpu %></td>
<td><%= hp.architecture.value if hp.architecture %></td> </tr> <% } %>
</tbody>
diff --git a/src/db/migrate/20090804135630_create_hardware_profiles.rb b/src/db/migrate/20090804135630_create_hardware_profiles.rb index 95bc0ce..5ad0435 100644 --- a/src/db/migrate/20090804135630_create_hardware_profiles.rb +++ b/src/db/migrate/20090804135630_create_hardware_profiles.rb @@ -21,12 +21,31 @@
class CreateHardwareProfiles < ActiveRecord::Migration def self.up
- create_table :hardware_profile_properties do |t|
t.string :name, :null => false
t.string :kind, :null => false
t.string :unit, :null => false
t.string :value, :null => false
t.string :range_first
t.string :range_last
t.integer :lock_version, :default => 0
t.timestamps
- end
- create_table :property_enum_entries do |t|
t.integer :hardware_profile_property_id, :null => false
t.string :value, :null => false
t.integer :lock_version, :default => 0
t.timestamps
- end
- create_table :hardware_profiles do |t| t.string :external_key, :null => false t.string :name, :null => false, :limit => 1024
t.float :memory, :null => false
t.float :storage, :null => false
t.string :architecture, :null => false
t.integer :memory_id
t.integer :storage_id
t.integer :cpu_id
t.integer :architecture_id t.integer :provider_id t.integer :lock_version, :default => 0 t.timestamps
diff --git a/src/spec/factories/hardware_profile.rb b/src/spec/factories/hardware_profile.rb index 43f63f3..9b19f01 100644 --- a/src/spec/factories/hardware_profile.rb +++ b/src/spec/factories/hardware_profile.rb @@ -3,29 +3,35 @@ Factory.define :hardware_profile do |p| end
Factory.define :mock_hwp1, :parent => :hardware_profile do |p|
- p.memory 1024
- p.storage 100
- p.memory { |p| p.association(:mock_hwp1_memory) }
- p.storage { |p| p.association(:mock_hwp1_storage) }
- p.cpu { |p| p.association(:mock_hwp1_cpu) }
- p.architecture { |p| p.association(:mock_hwp1_arch) } p.external_key 'mock_hwp1_key'
- p.architecture 'i686'
end
Factory.define :mock_hwp2, :parent => :hardware_profile do |p|
- p.memory 2048
- p.storage 400
- p.memory { |p| p.association(:mock_hwp2_memory) }
- p.storage { |p| p.association(:mock_hwp2_storage) }
- p.cpu { |p| p.association(:mock_hwp2_cpu) }
- p.architecture { |p| p.association(:mock_hwp2_arch) } p.external_key 'mock_hwp2_key'
- p.architecture 'x86_64'
end
-Factory.define :pool_hwp1, :parent => :hardware_profile do |p|
- p.memory 2048
- p.storage 400
- p.external_key 'pool_hwp1_key'
- p.architecture 'x86_64'
+Factory.define :agg_hwp1, :parent => :hardware_profile do |p|
- p.memory { |p| p.association(:agg_hwp1_memory) }
- p.storage { |p| p.association(:agg_hwp1_storage) }
- p.cpu { |p| p.association(:agg_hwp1_cpu) }
- p.architecture { |p| p.association(:agg_hwp1_arch) }
- p.provider_hardware_profiles { |hp| [hp.association(:mock_hwp1)] }
- p.external_key 'agg_hwp1_key'
end
-Factory.define :hardware_profile_auto, :parent => :hardware_profile do |p|
- p.external_key { |hp| hp.name + "_key" }
- p.storage 160
- p.memory 1024
- p.architecture "i386"
-end \ No newline at end of file +Factory.define :agg_hwp2, :parent => :hardware_profile do |p|
- p.memory { |p| p.association(:agg_hwp2_memory) }
- p.storage { |p| p.association(:agg_hwp2_storage) }
- p.cpu { |p| p.association(:agg_hwp2_cpu) }
- p.architecture { |p| p.association(:agg_hwp2_arch) }
- p.provider_hardware_profiles { |hp| [hp.association(:mock_hwp2)] }
- p.external_key 'agg_hwp2_key'
+end diff --git a/src/spec/factories/hardware_profile_property.rb b/src/spec/factories/hardware_profile_property.rb new file mode 100644 index 0000000..7f470f6 --- /dev/null +++ b/src/spec/factories/hardware_profile_property.rb @@ -0,0 +1,122 @@ +Factory.define :hardware_profile_property do |p| +end
+Factory.define :mock_hwp1_memory, :parent => :hardware_profile_property do |p|
- p.name 'memory'
- p.kind 'fixed'
- p.unit 'MB'
- p.value 1740.8
+end
+Factory.define :mock_hwp1_storage, :parent => :hardware_profile_property do |p|
- p.name 'storage'
- p.kind 'fixed'
- p.unit 'GB'
- p.value 160
+end
+Factory.define :mock_hwp1_cpu, :parent => :hardware_profile_property do |p|
- p.name 'cpu'
- p.kind 'fixed'
- p.unit 'count'
- p.value 1
+end
+Factory.define :mock_hwp1_arch, :parent => :hardware_profile_property do |p|
- p.name 'architecture'
- p.kind 'fixed'
- p.unit 'label'
- p.value 'i386'
+end
+Factory.define :mock_hwp2_memory, :parent => :hardware_profile_property do |p|
- p.name 'memory'
- p.kind 'range'
- p.unit 'MB'
- p.value 10240
- p.range_first 7680
- p.range_last 15360
+end
+Factory.define :mock_hwp2_storage, :parent => :hardware_profile_property do |p|
- p.name 'storage'
- p.kind 'enum'
- p.unit 'GB'
- p.value 850
+# p.property_enum_entries { |p| [p.association(:mock_hwp2_storage_enum1), +# p.association(:mock_hwp2_storage_enum2)] } +end
Same here
+Factory.define :mock_hwp2_cpu, :parent => :hardware_profile_property do |p|
- p.name 'cpu'
- p.kind 'fixed'
- p.unit 'count'
- p.value 2
+end
+Factory.define :mock_hwp2_arch, :parent => :hardware_profile_property do |p|
- p.name 'architecture'
- p.kind 'fixed'
- p.unit 'label'
- p.value 'x86_64'
+end
+Factory.define :agg_hwp1_memory, :parent => :hardware_profile_property do |p|
- p.name 'memory'
- p.kind 'fixed'
- p.unit 'MB'
- p.value 1740.8
+end
+Factory.define :agg_hwp1_storage, :parent => :hardware_profile_property do |p|
- p.name 'storage'
- p.kind 'fixed'
- p.unit 'GB'
- p.value 160
+end
+Factory.define :agg_hwp1_cpu, :parent => :hardware_profile_property do |p|
- p.name 'cpu'
- p.kind 'fixed'
- p.unit 'count'
- p.value 1
+end
+Factory.define :agg_hwp1_arch, :parent => :hardware_profile_property do |p|
- p.name 'architecture'
- p.kind 'fixed'
- p.unit 'label'
- p.value 'i386'
+end
+Factory.define :agg_hwp2_memory, :parent => :hardware_profile_property do |p|
- p.name 'memory'
- p.kind 'range'
- p.unit 'MB'
- p.value 10240
- p.range_first 7680
- p.range_last 15360
+end
+Factory.define :agg_hwp2_storage, :parent => :hardware_profile_property do |p|
- p.name 'storage'
- p.kind 'enum'
- p.unit 'GB'
- p.value 850
+# p.property_enum_entries { |p| [p.association(:agg_hwp2_storage_enum1), +# p.association(:agg_hwp2_storage_enum2)] }
and here.
+end
+Factory.define :agg_hwp2_cpu, :parent => :hardware_profile_property do |p|
- p.name 'cpu'
- p.kind 'fixed'
- p.unit 'count'
- p.value 2
+end
+Factory.define :agg_hwp2_arch, :parent => :hardware_profile_property do |p|
- p.name 'architecture'
- p.kind 'fixed'
- p.unit 'label'
- p.value 'x86_64'
+end diff --git a/src/spec/factories/pool.rb b/src/spec/factories/pool.rb index 2f9111e..d95fb13 100644 --- a/src/spec/factories/pool.rb +++ b/src/spec/factories/pool.rb @@ -5,5 +5,5 @@ end
Factory.define :tpool, :parent => :pool do |p| p.name 'tpool'
- p.hardware_profiles { |hp| [hp.association(:pool_hwp1)] }
+# p.hardware_profiles { |hp| [hp.association(:pool_hwp1)] }
And finally, here.
end diff --git a/src/spec/factories/property_enum_entry.rb b/src/spec/factories/property_enum_entry.rb new file mode 100644 index 0000000..78b99d5 --- /dev/null +++ b/src/spec/factories/property_enum_entry.rb @@ -0,0 +1,26 @@ +Factory.define :property_enum_entry do |e| +end
+Factory.define :mock_hwp2_storage_enum1, :parent => :property_enum_entry do |e|
- e.value 850
- e.prop_name 'storage'
- e.hardware_profile_property { |e| e.association(:mock_hwp2_storage)
} +end
+Factory.define :mock_hwp2_storage_enum2, :parent => :property_enum_entry do |e|
- e.value 1024
- e.prop_name 'storage'
- e.hardware_profile_property { |e| e.association(:mock_hwp2_storage)
} +end
+Factory.define :agg_hwp2_storage_enum1, :parent => :property_enum_entry do |e|
- e.value 850
- e.prop_name 'storage'
- e.hardware_profile_property { |e| e.association(:agg_hwp2_storage)
} +end
+Factory.define :agg_hwp2_storage_enum2, :parent => :property_enum_entry do |e|
- e.value 1024
- e.prop_name 'storage'
- e.hardware_profile_property { |e| e.association(:agg_hwp2_storage)
} +end diff --git a/src/spec/models/hardware_profile_spec.rb b/src/spec/models/hardware_profile_spec.rb index e2b09b4..ab04fe1 100644 --- a/src/spec/models/hardware_profile_spec.rb +++ b/src/spec/models/hardware_profile_spec.rb @@ -30,31 +30,18 @@ describe HardwareProfile do
it "should require valid amount of memory" do [nil, "hello", -1].each do |fail_value|
@hp.memory = fail_value
@hp.memory.value = fail_value @hp.should_not be_valid
end end
it "should require valid amount of storage" do [nil, "hello", -1].each do |fail_value|
@hp.storage = fail_value
end end@hp.storage.value = fail_value @hp.should_not be_valid
- it "should not require architecture when there's no provider" do
- @hp.architecture = nil
- @hp.should be_valid
- end
- it "should require architecture when it's with provider" do
- @hp.provider = Provider.new
- @hp.should be_valid
- @hp.architecture = nil
- @hp.should_not be_valid
- end
- it "should allow Aggregator profiles only for provider profiles"
do @hp.provider = nil
-- 1.6.2.5
deltacloud-devel mailing list deltacloud-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/deltacloud-devel