[email protected] wrote:
> From: Jan Provaznik <[email protected]>
>
> This is second shoot in image/template models refactoring, lotof TODOs
> - suppose image uuid is generated by aggregator for now when image is created
> in CE
> - when image factory builds image, image's status should be updated
> - image_providers table represents images uploaded to providers, suppose user
> can create instances only from uploaded images
>
I'm thinking we rename this replicated_images (class ReplicatedImage),
as there may be more than one per provider. The issue that prompted this
change is that we had come to believe that ec2 AMIs are valid for only
one realm, so if you want to make an image available in multiple realms,
you'll have to upload/register multiple times. In this case we'll have
one image but several ReplicatedImages.
Note that this impression of AMI-realm connection turned out to be
false, so we may not need to have multiple ReplicatedImages for a single
provider, but I'm not sure we want to constrain the model that way, as
other providers might do this.
For the rest of my comments I'll refer to replicated_image instead of
image_provider. I think functionally, this is really just a name change.
> - instance is now connected with image_provider instead of image
>
Actually image_id should change to template_id -- the user picks a
template. Condor will later derive the replicated_image when it chooses
a provider/account. We may eventually add a replicated_image_id field
that condor would fill in at launch, which would be used for more
details on the instance page, but we can skip that for now
> - when image is uploaded to provider, image_providers mapping should be
> updated
>
> ---
> diff --git a/src/app/models/image.rb b/src/app/models/image.rb
> index f3304cf..954668b 100644
> --- a/src/app/models/image.rb
> +++ b/src/app/models/image.rb
> @@ -25,46 +25,35 @@ class Image < ActiveRecord::Base
> cattr_reader :per_page
> @@per_page = 15
>
> + belongs_to :template
> has_many :instances
>
The direct instances association goes away since that's on template now.
> - belongs_to :provider
> -
> - has_and_belongs_to_many :aggregator_images,
> - :class_name => "Image",
> - :join_table => "image_map",
> - :foreign_key => "provider_image_id",
> - :association_foreign_key => "aggregator_image_id"
> -
> - has_and_belongs_to_many :provider_images,
> - :class_name => "Image",
> - :join_table => "image_map",
> - :foreign_key => "aggregator_image_id",
> - :association_foreign_key => "provider_image_id"
> -
> - validates_presence_of :external_key
> - validates_uniqueness_of :external_key, :scope => [:provider_id]
> + has_many :image_providers, :dependent => :destroy
>
or replicated_images here
> + has_many :providers, :through => :image_providers
>
replicated_images
>
> + validates_presence_of :uuid
> validates_presence_of :name
> validates_length_of :name, :maximum => 1024
> + validates_presence_of :status
> + validates_presence_of :target
> + validates_presence_of :template_id
>
> - validates_presence_of :architecture, :if => :provider
> + SEARCHABLE_COLUMNS = %w(name)
>
> - SEARCHABLE_COLUMNS = %w(name architecture)
> + STATE_QUEUED = 'queued'
> + STATE_WAITING = 'waiting'
> + STATE_BUILDING = 'building'
> + STATE_COMPLETE = 'complete'
> + STATE_CANCELED = 'canceled'
>
> - def provider_image?
> - !provider.nil?
> - end
> + ACTIVE_STATES = [ STATE_WAITING, STATE_BUILDING ]
>
> - def validate
> - if provider.nil?
> - if !aggregator_images.empty?
> - errors.add(:aggregator_images,
> - "Aggregator image only allowed for provider images")
> - end
> - else
> - if !provider_images.empty?
> - errors.add(:provider_images,
> - "Provider images only allowed for aggregator images")
> - end
> + def self.new_if_not_exists(data)
> + unless find_by_uuid(data[:uuid])
> + Image.new(data).save!
> end
> end
> +
> + def self.available_targets
> + return
> YAML.load_file("#{RAILS_ROOT}/config/image_descriptor_targets.yml")
> + end
> end
> diff --git a/src/app/models/image_provider.rb
> b/src/app/models/image_provider.rb
>
change this name to replicated_image
> new file mode 100644
> index 0000000..873f9a1
> --- /dev/null
> +++ b/src/app/models/image_provider.rb
> @@ -0,0 +1,4 @@
> +class ImageProvider < ActiveRecord::Base
> + belongs_to :provider
> + belongs_to :image
> +end
> diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
> index 0212e24..7439297 100644
> --- a/src/app/models/instance.rb
> +++ b/src/app/models/instance.rb
> @@ -30,12 +30,12 @@ class Instance < ActiveRecord::Base
> belongs_to :cloud_account
>
> belongs_to :hardware_profile
> - belongs_to :image
> + belongs_to :image_provider
>
belongs_to :template
> belongs_to :realm
>
> validates_presence_of :pool_id
> validates_presence_of :hardware_profile_id
> - validates_presence_of :image_id
> + validates_presence_of :image_provider_id
>
template_id
>
> #validates_presence_of :external_key
> # TODO: can we do uniqueness validation on indirect association
> @@ -46,9 +46,6 @@ class Instance < ActiveRecord::Base
> validates_uniqueness_of :name, :scope => :pool_id
> validates_length_of :name, :maximum => 1024
>
> - validates_presence_of :hardware_profile_id
> - validates_presence_of :image_id
> -
> STATE_NEW = "new"
> STATE_PENDING = "pending"
> STATE_RUNNING = "running"
> diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb
> index ed8fc70..ca87f05 100644
> --- a/src/app/models/provider.rb
> +++ b/src/app/models/provider.rb
> @@ -23,10 +23,11 @@ class Provider < ActiveRecord::Base
> require 'util/deltacloud'
> include PermissionedObject
>
> - has_many :cloud_accounts, :dependent => :destroy
> - has_many :hardware_profiles, :dependent => :destroy
> - has_many :images, :dependent => :destroy
> - has_many :realms, :dependent => :destroy
> + has_many :cloud_accounts, :dependent => :destroy
> + has_many :hardware_profiles, :dependent => :destroy
> + has_many :image_providers, :dependent => :destroy
>
replicated_images
> + has_many :images, :through => :image_providers
>
we don't need this anymore
> + has_many :realms, :dependent => :destroy
>
> validates_presence_of :name
> validates_uniqueness_of :name
> diff --git a/src/app/models/template.rb b/src/app/models/template.rb
> new file mode 100644
> index 0000000..04f3a37
> --- /dev/null
> +++ b/src/app/models/template.rb
> @@ -0,0 +1,48 @@
> +require 'util/image_descriptor_xml'
> +require 'typhoeus'
> +
> +class Template < ActiveRecord::Base
> + has_many :images, :dependent => :destroy
>
we probably need to add some way of pulling :providers, but this will
require 2 levels of :through, so this might need a custom method (unless
there's a way to do it w/ association syntax, I don't remember if AR
supports this or not)
> + before_save :update_attrs_from_xml
> +
> + WAREHOUSE_CONFIG =
> YAML.load_file("#{RAILS_ROOT}/config/image_warehouse.yml")
> +
> + #TODO: validations
> + #validates_uniqueness_of :name
> +
> + def update_xml_attributes!(opts = {})
> + doc = xml
> + doc.name = opts[:name] if opts[:name]
> + doc.platform = opts[:platform] if opts[:platform]
> + doc.description = opts[:description] if opts[:description]
> + doc.services = (opts[:services] || []) if opts[:services] or
> opts[:set_services]
> + doc.packages = (opts[:packages] || []) if opts[:packages] or
> opts[:set_packages]
> + save_xml!
> + end
> +
> + def save_xml!
> + self[:xml] = xml.to_xml
> + @xml = nil
> + save!
> + end
> +
> + def xml
> + @xml ||= ImageDescriptorXML.new(self[:xml].to_s)
> + end
> +
> + def upload_template
> + self.uri = File.join(WAREHOUSE_CONFIG['baseurl'], "template_#{id}")
> + response = Typhoeus::Request.put(self.uri, :body => xml.to_xml)
> + if response.code == 200
> + save!
> + else
> + raise "failed to upload template (return code #{response.code}):
> #{response.body}"
> + end
> + return true
> + end
> +
> + def update_attrs_from_xml
> + self.name = xml.name
> + self.summary = xml.description
> + end
> +end
> diff --git a/src/db/migrate/20090804140143_create_images.rb
> b/src/db/migrate/20090804140143_create_images.rb
> index 8c9c962..f963a6a 100644
> --- a/src/db/migrate/20090804140143_create_images.rb
> +++ b/src/db/migrate/20090804140143_create_images.rb
> @@ -22,21 +22,27 @@
> class CreateImages < ActiveRecord::Migration
> def self.up
> create_table :images do |t|
> - t.string :external_key, :null => false
> - t.string :name, :null => false, :limit => 1024
> - t.string :architecture, :null => false
> - t.integer :provider_id
> - t.integer :lock_version, :default => 0
> + t.string :uuid, :null => false
> + t.string :name, :null => false
> + t.string :build_id
> + t.string :uri
> + t.string :status
> + t.string :target
> + t.integer :template_id
> t.timestamps
> end
> - create_table "image_map", :force => true, :id => false do |t|
> - t.column "aggregator_image_id", :integer
> - t.column "provider_image_id", :integer
> +
> + create_table :image_providers do |t|
>
replicated_images
> + t.integer :image_id
> + t.integer :provider_id
> + t.string :provider_image_key
>
> + t.boolean :uploaded, :default => false
> + t.boolean :registered, :default => false
> end
> end
>
> def self.down
> - drop_table :image_map
> + drop_table :image_providers
>
replicated_images
> drop_table :images
> end
> end
> diff --git a/src/db/migrate/20090804142049_create_instances.rb
> b/src/db/migrate/20090804142049_create_instances.rb
> index 42706e1..640b194 100644
> --- a/src/db/migrate/20090804142049_create_instances.rb
> +++ b/src/db/migrate/20090804142049_create_instances.rb
> @@ -25,7 +25,7 @@ class CreateInstances < ActiveRecord::Migration
> t.string :external_key
> t.string :name, :null => false, :limit => 1024
> t.integer :hardware_profile_id, :null => false
> - t.integer :image_id, :null => false
> + t.integer :image_provider_id, :null => false
>
template_id -- we may eventually add image_provider_id too but for now
template_id is what we need
> t.integer :realm_id
> t.integer :pool_id, :null => false
> t.integer :cloud_account_id
> diff --git a/src/db/migrate/20100830150014_create_templates.rb
> b/src/db/migrate/20100830150014_create_templates.rb
> new file mode 100644
> index 0000000..0204d27
> --- /dev/null
> +++ b/src/db/migrate/20100830150014_create_templates.rb
> @@ -0,0 +1,16 @@
> +class CreateTemplates < ActiveRecord::Migration
> + def self.up
> + create_table :templates do |t|
>
we also need uuid here
> + t.binary :xml, :null => false
> + t.string :uri
> + t.string :name
> + t.text :summary
> + t.boolean :complete, :default => false
> + t.timestamps
> + end
> + end
> +
> + def self.down
> + drop_table :templates
> + end
> +end
>
Scott
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel
