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" <[email protected]> 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 <[email protected]> > --- > 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 > + @hp.storage.value = fail_value > @hp.should_not be_valid > end > end > > - 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 > [email protected] > https://fedorahosted.org/mailman/listinfo/deltacloud-devel _______________________________________________ deltacloud-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/deltacloud-devel
