This removes the pool-account model association and related view/controller logic.
You will need to rebuild your database after applying this patch. In order to create instances, you will need the follow-on patch that handles the required changes for import/mapping of images and hardware profiles, since these will no longer be pool-specific.
Signed-off-by: Scott Seago sseago@redhat.com --- src/app/controllers/cloud_accounts_controller.rb | 27 --------------- src/app/controllers/pool_controller.rb | 29 ---------------- src/app/models/cloud_account.rb | 3 +- src/app/models/pool.rb | 19 ++++++++-- src/app/models/pool_account.rb | 29 ---------------- src/app/models/privilege.rb | 16 ++------- src/app/models/provider.rb | 2 +- src/app/models/quota.rb | 2 +- src/app/services/registration_service.rb | 2 +- src/app/views/pool/accounts.html.erb | 30 ---------------- src/app/views/pool/accounts_for_pool.html.erb | 36 -------------------- src/app/views/pool/show.html.erb | 1 - .../20090801045212_create_cloud_accounts.rb | 1 + src/db/migrate/20090803141507_create_pools.rb | 1 - src/db/migrate/20091008153046_create_privileges.rb | 2 +- src/db/migrate/20091008153058_create_roles.rb | 22 +----------- .../migrate/20091215182906_create_pool_accounts.rb | 36 -------------------- src/spec/controllers/pool_controller_spec.rb | 9 ----- src/spec/controllers/users_controller_spec.rb | 2 +- src/spec/fixtures/roles.yml | 16 ++------ src/test/fixtures/roles.yml | 10 ++---- 21 files changed, 33 insertions(+), 262 deletions(-) delete mode 100644 src/app/models/pool_account.rb delete mode 100644 src/app/views/pool/accounts.html.erb delete mode 100644 src/app/views/pool/accounts_for_pool.html.erb delete mode 100644 src/db/migrate/20091215182906_create_pool_accounts.rb
diff --git a/src/app/controllers/cloud_accounts_controller.rb b/src/app/controllers/cloud_accounts_controller.rb index 05e024e..ea0a34a 100644 --- a/src/app/controllers/cloud_accounts_controller.rb +++ b/src/app/controllers/cloud_accounts_controller.rb @@ -31,14 +31,6 @@ class CloudAccountsController < ApplicationController } end
- def new_from_pool - @pool = Pool.find(params[:pool_id]) - require_privilege(Privilege::ACCOUNT_ADD,@pool) - @cloud_account = CloudAccount.new - @providers = Provider.all - end - - def create @cloud_account = CloudAccount.new(params[:cloud_account]) @provider = Provider.find(params[:provider][:id]) @@ -47,25 +39,6 @@ class CloudAccountsController < ApplicationController @cloud_account.save! end
- def create_from_pool - @pool = Pool.find(params[:pool][:id]) - require_privilege(Privilege::ACCOUNT_ADD,@pool) - Pool.transaction do - @cloud_account = CloudAccount.new(params[:cloud_account]) - @provider = Provider.find(params[:provider][:id]) - @cloud_account.provider = @provider - @cloud_account.save! - @pool.cloud_accounts << @cloud_account unless @pool.cloud_accounts.map{|x| x.id}.include?(@cloud_account.id) - @pool.save! - @pool.populate_realms_and_images([@cloud_account]) - perm = Permission.new(:user => @current_user, - :role => Role.find_by_name("Account Administrator"), - :permission_object => @cloud_account) - perm.save! - end - redirect_to :controller => "pool", :action => 'show', :id => @pool.id - end - def edit @cloud_account = CloudAccount.find(params[:id]) @provider = @cloud_account.provider diff --git a/src/app/controllers/pool_controller.rb b/src/app/controllers/pool_controller.rb index cf751e0..2ae0988 100644 --- a/src/app/controllers/pool_controller.rb +++ b/src/app/controllers/pool_controller.rb @@ -44,11 +44,6 @@ class PoolController < ApplicationController require_privilege(Privilege::POOL_VIEW, @pool) end
- def accounts - @pool = Pool.find(params[:id]) - require_privilege(Privilege::ACCOUNT_VIEW,@pool) - end - def realms @pool = Pool.find(params[:id]) @realm_names = @pool.realms @@ -58,7 +53,6 @@ class PoolController < ApplicationController def new require_privilege(Privilege::POOL_MODIFY) @pool = Pool.new - @account = CloudAccount.new end
def create @@ -89,27 +83,4 @@ class PoolController < ApplicationController require_privilege(Privilege::POOL_VIEW, @pool) end
- def accounts_for_pool - @pool = Pool.find(params[:pool_id]) - require_privilege(Privilege::ACCOUNT_VIEW,@pool) - @cloud_accounts = [] - all_accounts = CloudAccount.list_for_user(@current_user, Privilege::ACCOUNT_ADD) - all_accounts.each {|account| - @cloud_accounts << account unless @pool.cloud_accounts.map{|x| x.id}.include?(account.id) - } - end - - def add_account - @pool = Pool.find(params[:pool]) - @cloud_account = CloudAccount.find(params[:cloud_account]) - require_privilege(Privilege::ACCOUNT_ADD,@pool) - require_privilege(Privilege::ACCOUNT_ADD,@cloud_account) - Pool.transaction do - @pool.cloud_accounts << @cloud_account unless @pool.cloud_accounts.map{|x| x.id}.include?(@cloud_account.id) - @pool.save! - @pool.populate_realms_and_images([@cloud_account]) - end - redirect_to :action => 'show', :id => @pool.id - end - end diff --git a/src/app/models/cloud_account.rb b/src/app/models/cloud_account.rb index 1d6b3e9..f3dc317 100644 --- a/src/app/models/cloud_account.rb +++ b/src/app/models/cloud_account.rb @@ -22,8 +22,7 @@ class CloudAccount < ActiveRecord::Base include PermissionedObject belongs_to :provider - has_many :pool_accounts, :dependent => :destroy - has_many :pools, :through => :pool_accounts + belongs_to :quota has_many :instances
# what form does the account quota take? diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb index c97e03f..5b89a6b 100644 --- a/src/app/models/pool.rb +++ b/src/app/models/pool.rb @@ -21,10 +21,9 @@
class Pool < ActiveRecord::Base include PermissionedObject - has_many :pool_accounts, :dependent => :destroy - has_many :cloud_accounts, :through => :pool_accounts has_many :instances, :dependent => :destroy belongs_to :owner, :class_name => "User", :foreign_key => "owner_id" + belongs_to :quota
has_many :images, :dependent => :destroy has_many :hardware_profiles, :dependent => :destroy @@ -40,9 +39,20 @@ class Pool < ActiveRecord::Base :include => [:role], :order => "permissions.id ASC"
+ def cloud_accounts + accounts = [] + instances.each do |instance| + if instance.cloud_account and !accounts.include?(instance.cloud_account) + accounts << instance.cloud_account + end + end + end + + #FIXME: do we still allow explicit cloud/account choice via realm selection? + #FIXME: How is account list for realm defined without explicit pool-account relationship? def realms realm_list = [] - cloud_accounts.each do |cloud_account| + CloudAccount.all.each do |cloud_account| prefix = cloud_account.account_prefix_for_realm realm_list << prefix cloud_account.provider.realms.each do |realm| @@ -54,7 +64,8 @@ class Pool < ActiveRecord::Base end
# FIXME: for already-mapped accounts, update rather than add new - def populate_realms_and_images(accounts=cloud_accounts) + # FIXME: this needs to be revised to handle the removal of the account-pool association + def populate_realms_and_images(accounts=CloudAccount.all) accounts.each do |cloud_account| client = cloud_account.connect realms = client.realms diff --git a/src/app/models/pool_account.rb b/src/app/models/pool_account.rb deleted file mode 100644 index 1bb5429..0000000 --- a/src/app/models/pool_account.rb +++ /dev/null @@ -1,29 +0,0 @@ -# -# 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 PoolAccount < ActiveRecord::Base - belongs_to :cloud_account - belongs_to :pool - belongs_to :quota - - validates_presence_of :cloud_account_id - validates_presence_of :pool_id - -end diff --git a/src/app/models/privilege.rb b/src/app/models/privilege.rb index 9e085c4..0117c5c 100644 --- a/src/app/models/privilege.rb +++ b/src/app/models/privilege.rb @@ -43,26 +43,16 @@ class Privilege < ActiveRecord::Base STATS_VIEW = "stats_view" # can view monitoring data for # instances
- # to create(i.e. import) an account on a provider (but not added to - # a pool) needs ACCOUNT_MODIFY on the provider. - # to add a new provider account (i.e. import) to a pool needs - # ACCOUNT_ADD on the pool - # to add an existing provider account to a pool needs ACCOUNT_ADD - # on the pool _and_ ACCOUNT_ADD on the account. + # to create(i.e. import) an account on a provider needs ACCOUNT_MODIFY on the + # provider. ACCOUNT_MODIFY = "account_modify" # can create or modify cloud accounts ACCOUNT_VIEW = "account_view" # can view cloud accounts - ACCOUNT_ADD = "account_add" # can add an account to a pool
- # pool privileges normally checked at the provider level - # (and at the account level for choosing which accounts are visible on the - # new pool form), although - # pool-specific overrides could be a future enhancement. POOL_MODIFY = "pool_modify" # can create or modify a pool POOL_VIEW = "pool_view" # can view a pool
# quota privileges normally checked at the pool or account level, # depending on which quota level we're dealing with - # (account level for cloud-imposed quota, pool level for aggregator quota) QUOTA_MODIFY = "quota_modify" # can create or modify a quota QUOTA_VIEW = "quota_view" # can view a quota
@@ -81,7 +71,7 @@ class Privilege < ActiveRecord::Base FULL_PRIVILEGE_LIST = [PERM_SET, PERM_VIEW, INSTANCE_MODIFY, INSTANCE_CONTROL, INSTANCE_VIEW, STATS_VIEW, - ACCOUNT_MODIFY, ACCOUNT_ADD, ACCOUNT_VIEW, + ACCOUNT_MODIFY, ACCOUNT_VIEW, POOL_MODIFY, POOL_VIEW, QUOTA_MODIFY, QUOTA_VIEW, PROVIDER_MODIFY, PROVIDER_VIEW, diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb index e45b8cc..4e4e3b7 100644 --- a/src/app/models/provider.rb +++ b/src/app/models/provider.rb @@ -66,7 +66,7 @@ class Provider < ActiveRecord::Base end
def pools - cloud_accounts.collect {|account| account.pools}.flatten + cloud_accounts.collect {|account| account.pools}.flatten.uniq end
# TODO: implement or remove - this is meant to contain a hash of diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb index 21205b5..23f9aee 100644 --- a/src/app/models/quota.rb +++ b/src/app/models/quota.rb @@ -21,5 +21,5 @@
class Quota < ActiveRecord::Base has_one :pool - has_one :pool_account + has_one :cloud_account end diff --git a/src/app/services/registration_service.rb b/src/app/services/registration_service.rb index e414c6a..b2de097 100644 --- a/src/app/services/registration_service.rb +++ b/src/app/services/registration_service.rb @@ -11,7 +11,7 @@ class RegistrationService @user.save! @pool = Pool.create!({ :name => @user.login, :owner => @user}) Permission.create!({:user => @user, - :role => Role.find_by_name("Self-service Pool User"), + :role => Role.find_by_name("Instance Creator and User"), :permission_object => @pool}) end rescue diff --git a/src/app/views/pool/accounts.html.erb b/src/app/views/pool/accounts.html.erb deleted file mode 100644 index 96c4e2d..0000000 --- a/src/app/views/pool/accounts.html.erb +++ /dev/null @@ -1,30 +0,0 @@ -<% if @pool.cloud_accounts.size == 0 %> -<h1>There are no accounts to display</h1> -<% else %> - <table> - <thead> - <tr> - <th scope="col">Provider</th> - <th scope="col">Username</th> - <th scope="col">Actions</th> - </tr> - </thead> - <tbody> - <%@pool.cloud_accounts.each {|account| %> - <tr> - <td><%= account.provider.name %></td> - <td><%= account.username %></td> - <td> - <%= link_to "User access", - {:controller => "permissions", - :action => "list", - :cloud_account_id => account.id}, - :class=>"actionlink" if has_view_perms?(account) %> - </td> - </tr> - <% } %> - </tbody> - </table> -<% end %> -<%= link_to "View/Add Existing Cloud Accounts", {:controller => "pool",:action => "accounts_for_pool", :pool_id => @pool}, :class=>"actionlink" %> -<%= link_to "Add a New Cloud Account", {:controller => "cloud_accounts",:action => "new_from_pool", :pool_id => @pool}, :class=>"actionlink" %><br/> diff --git a/src/app/views/pool/accounts_for_pool.html.erb b/src/app/views/pool/accounts_for_pool.html.erb deleted file mode 100644 index fc4e5cb..0000000 --- a/src/app/views/pool/accounts_for_pool.html.erb +++ /dev/null @@ -1,36 +0,0 @@ -<% if @pool.cloud_accounts.size > 0 %> -<h1>These Cloud Accounts are already attached to this pool</h1> -<table> -<thead> -<tr> -<th scope="col">Provider Name</th> -<th scope="col">Cloud Account User Name</th> -</tr> -<% @pool.cloud_accounts.each {|a| %> -<tr> -<td><%= a.provider.name %></td> -<td><%= a.username %></td> -</tr> -<% } %> -</table> -<% end %> - -<% if @cloud_accounts.size == 0 %> -<h1>There are no existing Cloud Accounts available to add</h1> -<% else %> -<h1>These Cloud Accounts are available to add</h1> -<table> -<thead> -<tr> -<th scope="col">Provider Name</th> -<th scope="col">Cloud Account User Name</th> -</tr> -<% @cloud_accounts.each {|a| %> -<tr> -<td><%= a.provider.name %></td> -<td><%= a.username %> <%= link_to "Add this account", {:controller=> "pool", - :action => "add_account", :pool => @pool, :cloud_account => a}, :class => "actionlink" %></td> -</tr> -<% } %> -</table> -<% end %> diff --git a/src/app/views/pool/show.html.erb b/src/app/views/pool/show.html.erb index 015f13b..ef637f1 100644 --- a/src/app/views/pool/show.html.erb +++ b/src/app/views/pool/show.html.erb @@ -36,7 +36,6 @@ </table> <% end %> <%= link_to "Add a new instance", {:controller => "instance", :action => "new", :id => @pool}, :class=>"actionlink"%> -<%= link_to "Back end Accounts", {:action => "accounts", :id => @pool.id}, :class=>"actionlink"%> <%= link_to "User access", {:controller => "permissions", :action => "list", :pool_id => @pool.id}, :class=>"actionlink" if has_view_perms? %> <%= link_to "Hardware Profiles", {:action => "hardware_profiles", :id => @pool.id}, :class=>"actionlink"%> <%=link_to "View Images", {:controller => "pool", :action => "images", :id => @pool}, :class => "actionlink" %> diff --git a/src/db/migrate/20090801045212_create_cloud_accounts.rb b/src/db/migrate/20090801045212_create_cloud_accounts.rb index 933ef0e..316de87 100644 --- a/src/db/migrate/20090801045212_create_cloud_accounts.rb +++ b/src/db/migrate/20090801045212_create_cloud_accounts.rb @@ -25,6 +25,7 @@ class CreateCloudAccounts < ActiveRecord::Migration t.string :username, :null => false t.string :password, :null => false t.integer :provider_id, :null => false + t.integer :quota_id t.integer :lock_version, :default => 0 t.timestamps end diff --git a/src/db/migrate/20090803141507_create_pools.rb b/src/db/migrate/20090803141507_create_pools.rb index 5552869..f35021a 100644 --- a/src/db/migrate/20090803141507_create_pools.rb +++ b/src/db/migrate/20090803141507_create_pools.rb @@ -33,7 +33,6 @@ class CreatePools < ActiveRecord::Migration end
def self.down - drop_table :cloud_accounts_pools drop_table :pools end end diff --git a/src/db/migrate/20091008153046_create_privileges.rb b/src/db/migrate/20091008153046_create_privileges.rb index 12d94c6..74932b0 100644 --- a/src/db/migrate/20091008153046_create_privileges.rb +++ b/src/db/migrate/20091008153046_create_privileges.rb @@ -30,7 +30,7 @@ class CreatePrivileges < ActiveRecord::Migration privileges = ["set_perms", "view_perms", "instance_modify", "instance_control", "instance_view", "stats_view", - "account_modify", "account_add", "account_view", + "account_modify", "account_view", "pool_modify", "pool_view", "quota_modify", "quota_view", "provider_modify", "provider_view", diff --git a/src/db/migrate/20091008153058_create_roles.rb b/src/db/migrate/20091008153058_create_roles.rb index 5234c90..74cf519 100644 --- a/src/db/migrate/20091008153058_create_roles.rb +++ b/src/db/migrate/20091008153058_create_roles.rb @@ -53,18 +53,6 @@ class CreateRoles < ActiveRecord::Migration "quota_view", "set_perms", "view_perms"]}, - "Self-service Pool User" => - {:role_scope => "Pool", - :privileges => ["instance_control", - "instance_view", - "pool_view", - "stats_view", - "instance_modify", - "quota_view", - "set_perms", - "view_perms", - "account_view", - "account_add"]}, "Pool Creator" => {:role_scope => "Provider", :privileges => ["provider_view", @@ -78,9 +66,6 @@ class CreateRoles < ActiveRecord::Migration "pool_view", "quota_view", "quota_modify", - "account_view", - "account_add", - "account_modify", "set_perms", "view_perms"]}, "Provider Administrator" => @@ -93,13 +78,9 @@ class CreateRoles < ActiveRecord::Migration {:role_scope => "CloudAccount", :privileges => ["set_perms", "view_perms", + "stats_view", "account_view", - "account_add", "account_modify"]}, - "Account User" => - {:role_scope => "CloudAccount", - :privileges => ["account_view", - "account_add"]}, "Account Viewer" => {:role_scope => "CloudAccount", :privileges => ["account_view"]}, @@ -112,7 +93,6 @@ class CreateRoles < ActiveRecord::Migration :privileges => ["provider_modify", "provider_view", "account_modify", - "account_add", "account_view", "user_modify", "user_view", diff --git a/src/db/migrate/20091215182906_create_pool_accounts.rb b/src/db/migrate/20091215182906_create_pool_accounts.rb deleted file mode 100644 index aa71c09..0000000 --- a/src/db/migrate/20091215182906_create_pool_accounts.rb +++ /dev/null @@ -1,36 +0,0 @@ -# -# Copyright (C) 2009 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 CreatePoolAccounts < ActiveRecord::Migration - def self.up - create_table :pool_accounts do |t| - t.integer :cloud_account_id, :null => false - t.integer :pool_id, :null => false - t.integer :quota_id - t.integer :lock_version, :default => 0 - t.timestamps - end - end - - def self.down - drop_table :pool_accounts - end -end diff --git a/src/spec/controllers/pool_controller_spec.rb b/src/spec/controllers/pool_controller_spec.rb index 7917ed3..3daed93 100644 --- a/src/spec/controllers/pool_controller_spec.rb +++ b/src/spec/controllers/pool_controller_spec.rb @@ -39,15 +39,6 @@ describe PoolController do response.should render_template("hardware_profiles") end
- it "should get cloud accounts" do - @pool = Factory :tpool - UserSession.create(@admin) - get :accounts, :id => @pool.id - response.should be_success - response.should render_template("accounts") - @pool.should_not == nil - end - it "should provide ui to view realms" do UserSession.create(@admin) pool = Factory :tpool diff --git a/src/spec/controllers/users_controller_spec.rb b/src/spec/controllers/users_controller_spec.rb index 1189294..231a533 100644 --- a/src/spec/controllers/users_controller_spec.rb +++ b/src/spec/controllers/users_controller_spec.rb @@ -35,7 +35,7 @@ describe UsersController do p.name.should == "tuser2" p.permissions.size.should == 1 p.permissions.any? { - |perm| perm.role.name.eql?('Self-service Pool User') + |perm| perm.role.name.eql?('Instance Creator and User') }.should be_true id = User.find(:first, :conditions => ['login = ?', "tuser2"]).id response.should redirect_to("http://test.host/users/show/#%7Bid%7D") diff --git a/src/spec/fixtures/roles.yml b/src/spec/fixtures/roles.yml index 5c72efb..9a4c550 100644 --- a/src/spec/fixtures/roles.yml +++ b/src/spec/fixtures/roles.yml @@ -17,10 +17,6 @@ instance_creator_and_user: name: Instance Creator and User scope: Pool privileges: instance_control, instance_view, pool_view, stats_view, instance_modify, quota_view, set_perms, view_perms -self_service_pool_user: - name: Self-service Pool User - scope: Pool - privileges: instance_control, instance_view, pool_view, stats_view, instance_modify, quota_view, set_perms, view_perms, account_add pool_creator: name: Pool Creator scope: Provider @@ -28,19 +24,15 @@ pool_creator: pool_administrator: name: Pool Administrator scope: Provider - privileges: provider_view, pool_modify, pool_view, quota_modify, quota_view, account_modify, account_add, account_view, stats_view, set_perms, view_perms + privileges: provider_view, pool_modify, pool_view, quota_modify, quota_view, stats_view, set_perms, view_perms provider_administrator: name: Provider Administrator scope: Provider - privileges: provider_modify, provider_view, account_modify, account_view + privileges: provider_modify, provider_view, account_modify, account_view, stats_view account_administrator: name: Account Administrator scope: CloudAccount - privileges: account_modify, account_view, stats_view, set_perms, view_perms, account_add -account_user: - name: Account User - scope: CloudAccount - privileges: account_view, account_add + privileges: account_modify, account_view, stats_view, set_perms, view_perms account_viewer: name: Account Viewer scope: CloudAccount @@ -52,4 +44,4 @@ provider_creator: administrator: name: Administrator scope: BasePermissionObject - privileges: provider_modify, provider_view, account_modify, account_add, account_view, user_modify, user_view, set_perms, view_perms, pool_modify, pool_view, quota_modify, quota_view, stats_view, instance_modify, instance_control, instance_view + privileges: provider_modify, provider_view, account_modify, account_view, user_modify, user_view, set_perms, view_perms, pool_modify, pool_view, quota_modify, quota_view, stats_view, instance_modify, instance_control, instance_view diff --git a/src/test/fixtures/roles.yml b/src/test/fixtures/roles.yml index 5ec515b..99df4b7 100644 --- a/src/test/fixtures/roles.yml +++ b/src/test/fixtures/roles.yml @@ -17,10 +17,6 @@ instance_creator_and_user: name: Instance Creator and User scope: Pool privileges: instance_control, instance_view, pool_view, stats_view, instance_modify, quota_view, set_perms, view_perms -self_service_pool_user: - name: Self-service Pool User - scope: Pool - privileges: instance_control, instance_view, pool_view, stats_view, instance_modify, quota_view, set_perms, view_perms, account_modify pool_creator: name: Pool Creator scope: Provider @@ -28,19 +24,19 @@ pool_creator: pool_administrator: name: Pool Administrator scope: Provider - privileges: provider_view, pool_modify, pool_view, quota_modify, quota_view, account_modify, account_view, stats_view, set_perms, view_perms + privileges: provider_view, pool_modify, pool_view, quota_modify, quota_view, stats_view, set_perms, view_perms account_administrator: name: Account Administrator scope: CloudAccount privileges: account_modify, account_view, stats_view, set_perms, view_perms -account_user: +account_viewer: name: Account User scope: CloudAccount privileges: account_view provider_administrator: name: Provider Administrator scope: BasePermissionObject - privileges: provider_modify, provider_view, stats_view + privileges: provider_modify, provider_view, account_modify, account_view, stats_view site_administrator: name: Site Administrator scope: BasePermissionObject
fixed image/HWP associations and some additional fixes to spec tests and controllers/forms.
Signed-off-by: Scott Seago sseago@redhat.com --- src/app/controllers/cloud_accounts_controller.rb | 19 +++--- src/app/controllers/provider_controller.rb | 13 ---- src/app/models/cloud_account.rb | 61 ++++++++++++++++ src/app/models/hardware_profile.rb | 30 +++----- src/app/models/image.rb | 18 ++--- src/app/models/pool.rb | 73 ++----------------- src/app/models/provider.rb | 12 +++ src/app/util/taskomatic.rb | 8 ++ src/app/views/cloud_accounts/_form.erb | 13 ---- src/app/views/cloud_accounts/_form.html.erb | 2 +- src/app/views/cloud_accounts/new.html.erb | 10 +-- .../views/cloud_accounts/new_from_pool.html.erb | 11 --- src/app/views/provider/accounts.html.erb | 2 +- src/app/views/provider/new_account.html.erb | 11 --- src/app/views/provider/show.html.erb | 21 ------ .../20090804135630_create_hardware_profiles.rb | 1 - src/db/migrate/20090804140143_create_images.rb | 1 - src/features/pool.feature | 16 ---- .../controllers/cloud_accounts_controller_spec.rb | 12 +++ src/spec/controllers/provider_controller_spec.rb | 10 --- src/spec/factories/image.rb | 3 +- src/spec/models/hardware_profile_spec.rb | 55 ++------------- src/spec/models/image_spec.rb | 27 +------- 23 files changed, 144 insertions(+), 285 deletions(-) delete mode 100644 src/app/views/cloud_accounts/_form.erb delete mode 100644 src/app/views/cloud_accounts/new_from_pool.html.erb delete mode 100644 src/app/views/provider/new_account.html.erb
diff --git a/src/app/controllers/cloud_accounts_controller.rb b/src/app/controllers/cloud_accounts_controller.rb index ea0a34a..29be676 100644 --- a/src/app/controllers/cloud_accounts_controller.rb +++ b/src/app/controllers/cloud_accounts_controller.rb @@ -23,20 +23,21 @@ class CloudAccountsController < ApplicationController before_filter :require_user
def new + @provider = Provider.find(params[:provider_id]) @cloud_account = CloudAccount.new - @providers = [] - all_providers = Provider.all - all_providers.each {|provider| - @providers << provider if authorized?(Privilege::PROVIDER_VIEW,provider) - } + require_privilege(Privilege::ACCOUNT_MODIFY, @provider) end
def create - @cloud_account = CloudAccount.new(params[:cloud_account]) - @provider = Provider.find(params[:provider][:id]) + @provider = Provider.find(params[:cloud_account][:provider_id]) require_privilege(Privilege::ACCOUNT_MODIFY,@provider) - @cloud_account.provider = @provider - @cloud_account.save! + @cloud_account = CloudAccount.new(params[:cloud_account]) + if request.post? && @cloud_account.save && @cloud_account.populate_realms_and_images + flash[:notice] = "Provider account added." + redirect_to :controller => "provider", :action => "accounts", :id => @provider + else + render :action => "new" + end end
def edit diff --git a/src/app/controllers/provider_controller.rb b/src/app/controllers/provider_controller.rb index 23b5ba0..7a4cefc 100644 --- a/src/app/controllers/provider_controller.rb +++ b/src/app/controllers/provider_controller.rb @@ -67,17 +67,4 @@ class ProviderController < ApplicationController require_privilege(Privilege::PROVIDER_VIEW, @provider) end
- def new_account - @provider = Provider.find(params[:id]) - require_privilege(Privilege::ACCOUNT_MODIFY, @provider) - end - - def create_account - require_privilege(Privilege::ACCOUNT_MODIFY) - @acct = CloudAccount.find_or_create(params[:cloud_account]) - @provider = Provider.find(params[:cloud_account][:provider_id]) - @provider.cloud_accounts << @acct - redirect_to :action => 'accounts', :id => @provider.id - end - end diff --git a/src/app/models/cloud_account.rb b/src/app/models/cloud_account.rb index f3dc317..9b8458d 100644 --- a/src/app/models/cloud_account.rb +++ b/src/app/models/cloud_account.rb @@ -63,7 +63,68 @@ class CloudAccount < ActiveRecord::Base provider.name + Realm::AGGREGATOR_REALM_PROVIDER_DELIMITER + username end
+ def pools + pools = [] + instances.each do |instance| + pools << instance.pool + end + end + def name username end + + # FIXME: for already-mapped accounts, update rather than add new + def populate_realms_and_images + client = connect + realms = client.realms + # FIXME: the "self" filtering has to go as soon as we have a decent image selection UI + if client.driver_name == "ec2" + images = client.images(:owner_id=>:self) + else + images = client.images + end + # FIXME: this should probably be in the same transaction as cloud_account.save + self.transaction do + realms.each do |realm| + #ignore if it exists + #FIXME: we need to handle keeping in sync forupdates as well as + # account permissions + unless Realm.find_by_external_key_and_provider_id(realm.id, + provider.id) + ar_realm = Realm.new(:external_key => realm.id, + :name => realm.name ? realm.name : realm.id, + :provider_id => provider.id) + ar_realm.save! + end + end + images.each do |image| + #ignore if it exists + #FIXME: we need to handle keeping in sync for updates as well as + # account permissions + ar_image = Image.find_by_external_key_and_provider_id(image.id, + provider.id) + unless ar_image + ar_image = Image.new(:external_key => image.id, + :name => image.name ? image.name : + (image.description ? image.description : + image.id), + :architecture => image.architecture, + :provider_id => provider.id) + ar_image.save! + front_end_image = Image.new(:external_key => + provider.name + + Realm::AGGREGATOR_REALM_ACCOUNT_DELIMITER + + ar_image.external_key, + :name => provider.name + + Realm::AGGREGATOR_REALM_ACCOUNT_DELIMITER + + ar_image.name, + :architecture => ar_image.architecture) + front_end_image.provider_images << ar_image + front_end_image.save! + end + end + end + end + end diff --git a/src/app/models/hardware_profile.rb b/src/app/models/hardware_profile.rb index 8330388..d98c2f9 100644 --- a/src/app/models/hardware_profile.rb +++ b/src/app/models/hardware_profile.rb @@ -24,7 +24,6 @@ class HardwareProfile < ActiveRecord::Base has_many :provider_instances, :class_name => "Instance", :foreign_key => "provider_hardware_profile_id" belongs_to :provider - belongs_to :pool
has_and_belongs_to_many :aggregator_hardware_profiles, :class_name => "HardwareProfile", @@ -39,10 +38,10 @@ class HardwareProfile < ActiveRecord::Base :association_foreign_key => "provider_hardware_profile_id"
validates_presence_of :external_key - validates_uniqueness_of :external_key, :scope => [:provider_id, :pool_id] + validates_uniqueness_of :external_key, :scope => [:provider_id]
validates_presence_of :name - validates_uniqueness_of :name + validates_uniqueness_of :name, :scope => [:provider_id]
validates_presence_of :storage validates_numericality_of :storage, :greater_than => 0 @@ -51,28 +50,21 @@ class HardwareProfile < ActiveRecord::Base
validates_presence_of :architecture, :if => :provider
+ def provider_hardware_profile? + !provider.nil? + end + + # FIXME: what about custom instance profiles? def validate - if (provider.nil? and pool.nil?) + if provider.nil? if !aggregator_hardware_profiles.empty? errors.add(:aggregator_hardware_profiles, - "Aggregator profiles are not allowed for custom Instance profiles") - end - if !provider_hardware_profiles.empty? - errors.add(:provider_hardware_profiles, - "Provider profiles are not allowed for custom Instance profiles") + "Aggregator profiles only allowed for provider profiles") end - elsif (!provider.nil? and !pool.nil?) - errors.add(:provider, "provider or pool must be blank") - errors.add(:pool, "provider or pool must be blank") - elsif provider.nil? + else if !provider_hardware_profiles.empty? errors.add(:provider_hardware_profiles, - "Provider profiles only allowed for provider profiles") - end - elsif pool.nil? - if !aggregator_hardware_profiles.empty? - errors.add(:aggregator_hardware_profiles, - "Aggregator profiles only allowed for pool profiles") + "Provider profiles only allowed for aggregator profiles") end end end diff --git a/src/app/models/image.rb b/src/app/models/image.rb index 6670149..86806a4 100644 --- a/src/app/models/image.rb +++ b/src/app/models/image.rb @@ -22,7 +22,6 @@ class Image < ActiveRecord::Base has_many :instances belongs_to :provider - belongs_to :pool
has_and_belongs_to_many :aggregator_images, :class_name => "Image", @@ -37,26 +36,25 @@ class Image < ActiveRecord::Base :association_foreign_key => "provider_image_id"
validates_presence_of :external_key - validates_uniqueness_of :external_key, :scope => [:provider_id, :pool_id] + validates_uniqueness_of :external_key, :scope => [:provider_id]
validates_presence_of :name validates_length_of :name, :maximum => 1024
validates_presence_of :architecture, :if => :provider
+ + def provider_image? + !provider.nil? + end + def validate - if (provider.nil? and pool.nil?) - errors.add(:provider, "provider or pool must be specified") - errors.add(:pool, "provider or pool must be specified") - elsif (!provider.nil? and !pool.nil?) - errors.add(:provider, "provider or pool must be blank") - errors.add(:pool, "provider or pool must be blank") - elsif provider.nil? + if provider.nil? if !aggregator_images.empty? errors.add(:aggregator_images, "Aggregator image only allowed for provider images") end - elsif pool.nil? + else if !provider_images.empty? errors.add(:provider_images, "Provider images only allowed for aggregator images") diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb index 5b89a6b..a8657ee 100644 --- a/src/app/models/pool.rb +++ b/src/app/models/pool.rb @@ -48,6 +48,14 @@ class Pool < ActiveRecord::Base end end
+ def images + Image.find(:all, :conditions => {:provider_id => nil}) + end + + def hardware_profiles + HardwareProfile.find(:all, :conditions => {:provider_id => nil}) + end + #FIXME: do we still allow explicit cloud/account choice via realm selection? #FIXME: How is account list for realm defined without explicit pool-account relationship? def realms @@ -63,69 +71,4 @@ class Pool < ActiveRecord::Base realm_list end
- # FIXME: for already-mapped accounts, update rather than add new - # FIXME: this needs to be revised to handle the removal of the account-pool association - def populate_realms_and_images(accounts=CloudAccount.all) - accounts.each do |cloud_account| - client = cloud_account.connect - realms = client.realms - if client.driver_name == "ec2" - images = client.images(:owner_id=>:self) - else - images = client.images - end - # FIXME: this should probably be in the same transaction as pool.save - self.transaction do - realms.each do |realm| - #ignore if it exists - #FIXME: we need to handle keeping in sync forupdates as well as - # account permissions - unless Realm.find_by_external_key_and_provider_id(realm.id, - cloud_account.provider.id) - ar_realm = Realm.new(:external_key => realm.id, - :name => realm.name ? realm.name : realm.id, - :provider_id => cloud_account.provider.id) - ar_realm.save! - end - end - images.each do |image| - #ignore if it exists - #FIXME: we need to handle keeping in sync for updates as well as - # account permissions - ar_image = Image.find_by_external_key_and_provider_id(image.id, - cloud_account.provider.id) - unless ar_image - ar_image = Image.new(:external_key => image.id, - :name => image.name ? image.name : - (image.description ? image.description : - image.id), - :architecture => image.architecture, - :provider_id => cloud_account.provider.id) - ar_image.save! - end - front_end_image = Image.new(:external_key => - cloud_account.account_prefix_for_realm + - Realm::AGGREGATOR_REALM_ACCOUNT_DELIMITER + - ar_image.external_key, - :name => ar_image.name, - :architecture => ar_image.architecture, - :pool_id => id) - front_end_image.save! - end - cloud_account.provider.hardware_profiles.each do |hardware_profile| - front_hardware_profile = HardwareProfile.new(:external_key => - cloud_account.account_prefix_for_realm + - hardware_profile.external_key, - :name => hardware_profile.name, - :memory => hardware_profile.memory, - :storage => hardware_profile.storage, - :architecture => hardware_profile.architecture, - :pool_id => id) - front_hardware_profile.save! - end - end - end - end - - end diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb index 4e4e3b7..20b9e1e 100644 --- a/src/app/models/provider.rb +++ b/src/app/models/provider.rb @@ -61,6 +61,18 @@ class Provider < ActiveRecord::Base :architecture => hardware_profile.architecture, :provider_id => id) ar_hardware_profile.save! + front_hwp = HardwareProfile.new(:external_key => + name + + Realm::AGGREGATOR_REALM_ACCOUNT_DELIMITER + + 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) + front_hwp.provider_hardware_profiles << ar_hardware_profile + front_hwp.save! end end end diff --git a/src/app/util/taskomatic.rb b/src/app/util/taskomatic.rb index c0dcbef..e66bb62 100644 --- a/src/app/util/taskomatic.rb +++ b/src/app/util/taskomatic.rb @@ -50,6 +50,14 @@ class Taskomatic begin client = @task.instance.cloud_account.connect realm = @task.instance.realm.external_key rescue nil + + # Map aggregator HWP/image to back-end provider HWP/image in instance + unless @task.instance.image.provider_image? + @task.instance.image = @task.instance.image.provider_images[0] + end + unless @task.instance.hardware_profile.provider_hardware_profile? + @task.instance.hardware_profile = @task.instance.hardware_profile.provider_hardware_profiles[0] + end dcloud_instance = client.create_instance(@task.instance.image.external_key, :flavor => @task.instance.hardware_profile.external_key, :realm => realm, diff --git a/src/app/views/cloud_accounts/_form.erb b/src/app/views/cloud_accounts/_form.erb deleted file mode 100644 index 4fa1a79..0000000 --- a/src/app/views/cloud_accounts/_form.erb +++ /dev/null @@ -1,13 +0,0 @@ -<ul> - <li> - <%= form.label :username, "Cloud Account username" %> - <%= form.text_field :username %> - </li> - <li> - <%= form.label :password, "Cloud Account password" %> - <%= form.password_field :password %> - </li> - <li> - <%= select("provider", "id", @providers.map{|p| [p.name,p.id]} , { :include_blank => false }) %> - </li> -</ul> diff --git a/src/app/views/cloud_accounts/_form.html.erb b/src/app/views/cloud_accounts/_form.html.erb index 9b7b176..0d71953 100644 --- a/src/app/views/cloud_accounts/_form.html.erb +++ b/src/app/views/cloud_accounts/_form.html.erb @@ -3,7 +3,7 @@ <%= hidden_field :cloud_account, :id %> <%= hidden_field :cloud_account, :provider_id, :value => @provider.id %> <ul> -<% if @cloud_account.nil? %> +<% if @cloud_account.id.nil? %> <li><label>UserName<span>UserName for the account you wish to connect to this pool.</span></label><%= text_field :cloud_account, :username %></li> <% end %> <li><label>Password<span>Password for the account you wish to connect to this pool.</span></label><%=password_field :cloud_account, :password %></li> diff --git a/src/app/views/cloud_accounts/new.html.erb b/src/app/views/cloud_accounts/new.html.erb index 40f32cb..c8e6d60 100644 --- a/src/app/views/cloud_accounts/new.html.erb +++ b/src/app/views/cloud_accounts/new.html.erb @@ -1,14 +1,10 @@ -<% if @providers.size == 0 %> -<h1>No Providers available to associate with a new cloud account</h1> -<% else %> -<h2 class="greeting">New Cloud Account</h2> - <div class="dcloud_form"> + <%= error_messages_for 'account' %> + + <h2>Add a new Account from this Provider</h2><br /> <% form_for @cloud_account, :url => { :action => "create" } do |f| %> <%= f.error_messages %> <%= render :partial => "form", :object => f %> - <%= f.submit "Create Cloud Account", :class => "submit" %> <% end %> <%= link_to "Cancel", root_path, :class => 'actionlink' %> </div> -<% end %> diff --git a/src/app/views/cloud_accounts/new_from_pool.html.erb b/src/app/views/cloud_accounts/new_from_pool.html.erb deleted file mode 100644 index 747df03..0000000 --- a/src/app/views/cloud_accounts/new_from_pool.html.erb +++ /dev/null @@ -1,11 +0,0 @@ -<h2 class="greeting">New Cloud Account</h2> - -<div class="dcloud_form"> - <% form_for @cloud_account, :url => { :action => "create_from_pool" } do |f| %> - <%= f.error_messages %> - <%= render :partial => "form", :object => f %> - <%= hidden_field :pool, :id %> - <%= f.submit "Create Cloud Account", :class => "submit" %> - <% end %> - <%= link_to "Cancel", root_path, :class => 'actionlink' %> -</div> diff --git a/src/app/views/provider/accounts.html.erb b/src/app/views/provider/accounts.html.erb index a763a33..ddc8f27 100644 --- a/src/app/views/provider/accounts.html.erb +++ b/src/app/views/provider/accounts.html.erb @@ -20,4 +20,4 @@ </tbody> </table> <% end %> -<%= link_to "Add an account", {:action => "new_account", :id => @provider}, :class => "actionlink" %> +<%= link_to "Add an account", {:controller => "cloud_accounts", :action => "new", :provider_id => @provider}, :class => "actionlink" %> diff --git a/src/app/views/provider/new_account.html.erb b/src/app/views/provider/new_account.html.erb deleted file mode 100644 index eed474f..0000000 --- a/src/app/views/provider/new_account.html.erb +++ /dev/null @@ -1,11 +0,0 @@ -<div class="dcloud_form"> - <%= error_messages_for 'pool' %> - <%= error_messages_for 'account' %> - - <h2>Create an Account for this Provider</h2><br /> - - <% form_tag :action => 'create_account' do |form| -%> - <%= render :partial => 'cloud_accounts/form' %> - <% end %> - -</div> diff --git a/src/app/views/provider/show.html.erb b/src/app/views/provider/show.html.erb index 0897828..f4213a9 100644 --- a/src/app/views/provider/show.html.erb +++ b/src/app/views/provider/show.html.erb @@ -1,24 +1,3 @@ -<% if @provider.pools.size == 0 %> -<h1>There are no pools to display</h1> -<% else %> - <table> - <thead> - <tr> - <th scope="col">Name</th> - <th scope="col">Instances</th> - </tr> - </thead> - <tbody> - <%@provider.pools.each {|pool| %> - <tr> - <td><%= pool.name %></td> - <td><%= pool.instances.count %></td> - </tr> - <% } %> - </tbody> - </table> -<% end %> -<%= link_to "Add a pool", {:controller => "pool", :action => "new", :provider => @provider}, :class => "actionlink" %> <%= link_to "Realms", {:action => "realms", :id => @provider.id}, :class=>"actionlink"%> <%= link_to "Accounts", {:action => "accounts", :id => @provider.id}, :class=>"actionlink"%> <%= link_to "User access", {:controller => "permissions", :action => "list", :provider_id => @provider.id}, :class=>"actionlink" if has_view_perms? %> diff --git a/src/db/migrate/20090804135630_create_hardware_profiles.rb b/src/db/migrate/20090804135630_create_hardware_profiles.rb index f3e1b65..95bc0ce 100644 --- a/src/db/migrate/20090804135630_create_hardware_profiles.rb +++ b/src/db/migrate/20090804135630_create_hardware_profiles.rb @@ -28,7 +28,6 @@ class CreateHardwareProfiles < ActiveRecord::Migration t.float :storage, :null => false t.string :architecture, :null => false t.integer :provider_id - t.integer :pool_id t.integer :lock_version, :default => 0 t.timestamps end diff --git a/src/db/migrate/20090804140143_create_images.rb b/src/db/migrate/20090804140143_create_images.rb index bb3db82..8c9c962 100644 --- a/src/db/migrate/20090804140143_create_images.rb +++ b/src/db/migrate/20090804140143_create_images.rb @@ -26,7 +26,6 @@ class CreateImages < ActiveRecord::Migration t.string :name, :null => false, :limit => 1024 t.string :architecture, :null => false t.integer :provider_id - t.integer :pool_id t.integer :lock_version, :default => 0 t.timestamps end diff --git a/src/features/pool.feature b/src/features/pool.feature index c96c2a9..5d8f219 100644 --- a/src/features/pool.feature +++ b/src/features/pool.feature @@ -45,19 +45,3 @@ Feature: Manage Pools When I follow "Realms" Then I should see "Europe" And I should see "United States" - - Scenario: View Pool's Images - Given I own a pool named "mockpool" - And the Pool has the following Images: - | name | architecture | - | Fedora 10 | x86_64 | - | Fedora 10 | i386 | - | JBoss | i386 | - And I am on the homepage - When I follow "mockpool" - Then I should be on the show pool page - When I follow "View Images" - Then I should see the following: - | Fedora 10 | x86_64 | - | Fedora 10 | i386 | - | JBoss | i386 | diff --git a/src/spec/controllers/cloud_accounts_controller_spec.rb b/src/spec/controllers/cloud_accounts_controller_spec.rb index f1d4a0d..d3e0f18 100644 --- a/src/spec/controllers/cloud_accounts_controller_spec.rb +++ b/src/spec/controllers/cloud_accounts_controller_spec.rb @@ -48,4 +48,16 @@ describe CloudAccountsController do response.should_not be_success end
+ it "should provide ui to create new account" do + UserSession.create(@admin) + get :new, :provider_id => @provider.id + response.should be_success + response.should render_template("new") + end + + it "should fail to grant access to account UIs for unauthenticated user" do + get :new + response.should_not be_success + end + end diff --git a/src/spec/controllers/provider_controller_spec.rb b/src/spec/controllers/provider_controller_spec.rb index de894ca..096af3d 100644 --- a/src/spec/controllers/provider_controller_spec.rb +++ b/src/spec/controllers/provider_controller_spec.rb @@ -17,19 +17,9 @@ describe ProviderController do response.should render_template("accounts") end
- it "should provide ui to create new account" do - UserSession.create(@admin) - get :new_account, :id => @provider.id - response.should be_success - response.should render_template("new_account") - end - it "should fail to grant access to account UIs for unauthenticated user" do get :accounts response.should_not be_success - - get :new_account - response.should_not be_success end
it "should provide ui to view hardware profiles" do diff --git a/src/spec/factories/image.rb b/src/spec/factories/image.rb index dc55ac0..cc0bc17 100644 --- a/src/spec/factories/image.rb +++ b/src/spec/factories/image.rb @@ -8,7 +8,6 @@ Factory.define :image do |i| i.provider { |p| Provider.new } end
-Factory.define :pool_image, :parent => :image do |i| +Factory.define :front_end_image, :parent => :image do |i| i.provider nil - i.association(:pool) end diff --git a/src/spec/models/hardware_profile_spec.rb b/src/spec/models/hardware_profile_spec.rb index 78fc0a2..e2b09b4 100644 --- a/src/spec/models/hardware_profile_spec.rb +++ b/src/spec/models/hardware_profile_spec.rb @@ -55,69 +55,28 @@ describe HardwareProfile do @hp.should_not be_valid end
- it "should reject Aggregator profiles for custom Instance profiles" do + it "should allow Aggregator profiles only for provider profiles" do + @hp.provider = nil + @hp.aggregator_hardware_profiles << @hp - @hp.should_not be_valid @hp.should have(1).error_on(:aggregator_hardware_profiles) @hp.errors.on(:aggregator_hardware_profiles).should eql( - "Aggregator profiles are not allowed for custom Instance profiles") + "Aggregator profiles only allowed for provider profiles")
@hp.aggregator_hardware_profiles.clear @hp.should be_valid - end - - it "should reject Provider profiles for custom Instance profiles" do - @hp.provider_hardware_profiles << @hp - @hp.should_not be_valid - @hp.should have(1).error_on(:provider_hardware_profiles) - @hp.errors.on(:provider_hardware_profiles).should eql( - "Provider profiles are not allowed for custom Instance profiles") - - @hp.provider_hardware_profiles.clear - @hp.should be_valid end
- it "should require either provider or pool to be blank" do + it "should allow Provider profiles only for aggregator profiles" do @hp.provider = Provider.new - @hp.pool = Pool.new - @hp.should_not be_valid - @hp.should have(1).error_on(:provider) - @hp.errors.on(:provider).should eql("provider or pool must be blank") - @hp.should have(1).error_on(:pool) - @hp.errors.on(:pool).should eql("provider or pool must be blank") - - @hp.provider = nil - @hp.should be_valid
- @hp.provider = Provider.new - @hp.pool = nil - @hp.should be_valid - end - - it "should allow Provider profiles only for provider profiles" do - @hp.provider = nil - @hp.pool = Pool.new - - @hp.provider_hardware_profiles << @hp + @hp.aggregator_hardware_profiles << @hp @hp.should have(1).error_on(:provider_hardware_profiles) @hp.errors.on(:provider_hardware_profiles).should eql( - "Provider profiles only allowed for provider profiles") + "Provider profiles only allowed for aggregator profiles")
@hp.provider_hardware_profiles.clear @hp.should be_valid end
- it "should allow Aggregator profiles only for pool profiles" do - @hp.provider = Provider.new - @hp.pool = nil - - @hp.aggregator_hardware_profiles << @hp - @hp.should have(1).error_on(:aggregator_hardware_profiles) - @hp.errors.on(:aggregator_hardware_profiles).should eql( - "Aggregator profiles only allowed for pool profiles") - - @hp.aggregator_hardware_profiles.clear - @hp.should be_valid - end - end diff --git a/src/spec/models/image_spec.rb b/src/spec/models/image_spec.rb index de68475..a0fd8ca 100644 --- a/src/spec/models/image_spec.rb +++ b/src/spec/models/image_spec.rb @@ -46,33 +46,8 @@ describe Image do i.should be_valid end
- it "should have either a provider or a pool specified" do - i = Factory.build(:image, :provider => nil, :pool => nil) - i.should have(1).error_on(:provider) - i.should have(1).error_on(:pool) - i.errors.on(:provider).should eql( - "provider or pool must be specified") - i.errors.on(:pool).should eql( - "provider or pool must be specified") - - i.provider = @provider - i.should be_valid - - i.pool = Factory.build(:pool) - i.should have(1).error_on(:provider) - i.should have(1).error_on(:pool) - i.errors.on(:provider).should eql( - "provider or pool must be blank") - i.errors.on(:pool).should eql( - "provider or pool must be blank") - - i.provider = nil - i.should be_valid - end - it "should have provider images only if it has a provider" do - i = Factory.create(:image, :pool => Pool.new, - :provider => nil) + i = Factory.create(:image, :provider => nil)
i.aggregator_images << i i.should have(1).error_on(:aggregator_images)
On Fri, May 14, 2010 at 10:36:28AM -0400, Scott Seago wrote:
fixed image/HWP associations and some additional fixes to spec tests and controllers/forms.
Signed-off-by: Scott Seago sseago@redhat.com
src/app/controllers/cloud_accounts_controller.rb | 19 +++--- src/app/controllers/provider_controller.rb | 13 ---- src/app/models/cloud_account.rb | 61 ++++++++++++++++ src/app/models/hardware_profile.rb | 30 +++----- src/app/models/image.rb | 18 ++--- src/app/models/pool.rb | 73 ++----------------- src/app/models/provider.rb | 12 +++ src/app/util/taskomatic.rb | 8 ++ src/app/views/cloud_accounts/_form.erb | 13 ---- src/app/views/cloud_accounts/_form.html.erb | 2 +- src/app/views/cloud_accounts/new.html.erb | 10 +-- .../views/cloud_accounts/new_from_pool.html.erb | 11 --- src/app/views/provider/accounts.html.erb | 2 +- src/app/views/provider/new_account.html.erb | 11 --- src/app/views/provider/show.html.erb | 21 ------ .../20090804135630_create_hardware_profiles.rb | 1 - src/db/migrate/20090804140143_create_images.rb | 1 - src/features/pool.feature | 16 ---- .../controllers/cloud_accounts_controller_spec.rb | 12 +++ src/spec/controllers/provider_controller_spec.rb | 10 --- src/spec/factories/image.rb | 3 +- src/spec/models/hardware_profile_spec.rb | 55 ++------------- src/spec/models/image_spec.rb | 27 +------- 23 files changed, 144 insertions(+), 285 deletions(-) delete mode 100644 src/app/views/cloud_accounts/_form.erb delete mode 100644 src/app/views/cloud_accounts/new_from_pool.html.erb delete mode 100644 src/app/views/provider/new_account.html.erb
diff --git a/src/app/controllers/cloud_accounts_controller.rb b/src/app/controllers/cloud_accounts_controller.rb index ea0a34a..29be676 100644 --- a/src/app/controllers/cloud_accounts_controller.rb +++ b/src/app/controllers/cloud_accounts_controller.rb @@ -23,20 +23,21 @@ class CloudAccountsController < ApplicationController before_filter :require_user
def new
- @provider = Provider.find(params[:provider_id]) @cloud_account = CloudAccount.new
- @providers = []
- all_providers = Provider.all
- all_providers.each {|provider|
@providers << provider if authorized?(Privilege::PROVIDER_VIEW,provider)
- }
require_privilege(Privilege::ACCOUNT_MODIFY, @provider) end
def create
- @cloud_account = CloudAccount.new(params[:cloud_account])
- @provider = Provider.find(params[:provider][:id])
- @provider = Provider.find(params[:cloud_account][:provider_id]) require_privilege(Privilege::ACCOUNT_MODIFY,@provider)
- @cloud_account.provider = @provider
- @cloud_account.save!
@cloud_account = CloudAccount.new(params[:cloud_account])
if request.post? && @cloud_account.save && @cloud_account.populate_realms_and_images
flash[:notice] = "Provider account added."
redirect_to :controller => "provider", :action => "accounts", :id => @provider
else
render :action => "new"
end end
def edit
diff --git a/src/app/controllers/provider_controller.rb b/src/app/controllers/provider_controller.rb index 23b5ba0..7a4cefc 100644 --- a/src/app/controllers/provider_controller.rb +++ b/src/app/controllers/provider_controller.rb @@ -67,17 +67,4 @@ class ProviderController < ApplicationController require_privilege(Privilege::PROVIDER_VIEW, @provider) end
- def new_account
@provider = Provider.find(params[:id])
require_privilege(Privilege::ACCOUNT_MODIFY, @provider)
- end
- def create_account
require_privilege(Privilege::ACCOUNT_MODIFY)
@acct = CloudAccount.find_or_create(params[:cloud_account])
@provider = Provider.find(params[:cloud_account][:provider_id])
@provider.cloud_accounts << @acct
redirect_to :action => 'accounts', :id => @provider.id
- end
end diff --git a/src/app/models/cloud_account.rb b/src/app/models/cloud_account.rb index f3dc317..9b8458d 100644 --- a/src/app/models/cloud_account.rb +++ b/src/app/models/cloud_account.rb @@ -63,7 +63,68 @@ class CloudAccount < ActiveRecord::Base provider.name + Realm::AGGREGATOR_REALM_PROVIDER_DELIMITER + username end
- def pools
- pools = []
- instances.each do |instance|
pools << instance.pool
- end
- end
- def name username end
- # FIXME: for already-mapped accounts, update rather than add new
- def populate_realms_and_images
- client = connect
- realms = client.realms
- # FIXME: the "self" filtering has to go as soon as we have a decent image selection UI
- if client.driver_name == "ec2"
images = client.images(:owner_id=>:self)
- else
images = client.images
- end
- # FIXME: this should probably be in the same transaction as cloud_account.save
- self.transaction do
realms.each do |realm|
#ignore if it exists
#FIXME: we need to handle keeping in sync forupdates as well as
# account permissions
unless Realm.find_by_external_key_and_provider_id(realm.id,
provider.id)
ar_realm = Realm.new(:external_key => realm.id,
:name => realm.name ? realm.name : realm.id,
:provider_id => provider.id)
ar_realm.save!
end
end
images.each do |image|
#ignore if it exists
#FIXME: we need to handle keeping in sync for updates as well as
# account permissions
ar_image = Image.find_by_external_key_and_provider_id(image.id,
provider.id)
unless ar_image
ar_image = Image.new(:external_key => image.id,
:name => image.name ? image.name :
(image.description ? image.description :
image.id),
:architecture => image.architecture,
:provider_id => provider.id)
ar_image.save!
front_end_image = Image.new(:external_key =>
provider.name +
Realm::AGGREGATOR_REALM_ACCOUNT_DELIMITER +
ar_image.external_key,
:name => provider.name +
Realm::AGGREGATOR_REALM_ACCOUNT_DELIMITER +
ar_image.name,
:architecture => ar_image.architecture)
front_end_image.provider_images << ar_image
front_end_image.save!
end
end
- end
- end
end diff --git a/src/app/models/hardware_profile.rb b/src/app/models/hardware_profile.rb index 8330388..d98c2f9 100644 --- a/src/app/models/hardware_profile.rb +++ b/src/app/models/hardware_profile.rb @@ -24,7 +24,6 @@ class HardwareProfile < ActiveRecord::Base has_many :provider_instances, :class_name => "Instance", :foreign_key => "provider_hardware_profile_id" belongs_to :provider
belongs_to :pool
has_and_belongs_to_many :aggregator_hardware_profiles, :class_name => "HardwareProfile",
@@ -39,10 +38,10 @@ class HardwareProfile < ActiveRecord::Base :association_foreign_key => "provider_hardware_profile_id"
validates_presence_of :external_key
- validates_uniqueness_of :external_key, :scope => [:provider_id, :pool_id]
validates_uniqueness_of :external_key, :scope => [:provider_id]
validates_presence_of :name
- validates_uniqueness_of :name
validates_uniqueness_of :name, :scope => [:provider_id]
validates_presence_of :storage validates_numericality_of :storage, :greater_than => 0
@@ -51,28 +50,21 @@ class HardwareProfile < ActiveRecord::Base
validates_presence_of :architecture, :if => :provider
- def provider_hardware_profile?
- !provider.nil?
- end
- # FIXME: what about custom instance profiles? def validate
- if (provider.nil? and pool.nil?)
- if provider.nil? if !aggregator_hardware_profiles.empty? errors.add(:aggregator_hardware_profiles,
"Aggregator profiles are not allowed for custom Instance profiles")
end
if !provider_hardware_profiles.empty?
errors.add(:provider_hardware_profiles,
"Provider profiles are not allowed for custom Instance profiles")
"Aggregator profiles only allowed for provider profiles") end
- elsif (!provider.nil? and !pool.nil?)
errors.add(:provider, "provider or pool must be blank")
errors.add(:pool, "provider or pool must be blank")
- elsif provider.nil?
- else if !provider_hardware_profiles.empty? errors.add(:provider_hardware_profiles,
"Provider profiles only allowed for provider profiles")
end
- elsif pool.nil?
if !aggregator_hardware_profiles.empty?
errors.add(:aggregator_hardware_profiles,
"Aggregator profiles only allowed for pool profiles")
end end"Provider profiles only allowed for aggregator profiles") end
diff --git a/src/app/models/image.rb b/src/app/models/image.rb index 6670149..86806a4 100644 --- a/src/app/models/image.rb +++ b/src/app/models/image.rb @@ -22,7 +22,6 @@ class Image < ActiveRecord::Base has_many :instances belongs_to :provider
belongs_to :pool
has_and_belongs_to_many :aggregator_images, :class_name => "Image",
@@ -37,26 +36,25 @@ class Image < ActiveRecord::Base :association_foreign_key => "provider_image_id"
validates_presence_of :external_key
- validates_uniqueness_of :external_key, :scope => [:provider_id, :pool_id]
validates_uniqueness_of :external_key, :scope => [:provider_id]
validates_presence_of :name validates_length_of :name, :maximum => 1024
validates_presence_of :architecture, :if => :provider
def provider_image?
!provider.nil?
end
def validate
- if (provider.nil? and pool.nil?)
errors.add(:provider, "provider or pool must be specified")
errors.add(:pool, "provider or pool must be specified")
- elsif (!provider.nil? and !pool.nil?)
errors.add(:provider, "provider or pool must be blank")
errors.add(:pool, "provider or pool must be blank")
- elsif provider.nil?
- if provider.nil? if !aggregator_images.empty? errors.add(:aggregator_images, "Aggregator image only allowed for provider images") end
- elsif pool.nil?
- else if !provider_images.empty? errors.add(:provider_images, "Provider images only allowed for aggregator images")
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb index 5b89a6b..a8657ee 100644 --- a/src/app/models/pool.rb +++ b/src/app/models/pool.rb @@ -48,6 +48,14 @@ class Pool < ActiveRecord::Base end end
- def images
- Image.find(:all, :conditions => {:provider_id => nil})
- end
- def hardware_profiles
- HardwareProfile.find(:all, :conditions => {:provider_id => nil})
- end
- #FIXME: do we still allow explicit cloud/account choice via realm selection? #FIXME: How is account list for realm defined without explicit pool-account relationship? def realms
@@ -63,69 +71,4 @@ class Pool < ActiveRecord::Base realm_list end
- # FIXME: for already-mapped accounts, update rather than add new
- # FIXME: this needs to be revised to handle the removal of the account-pool association
- def populate_realms_and_images(accounts=CloudAccount.all)
- accounts.each do |cloud_account|
client = cloud_account.connect
realms = client.realms
if client.driver_name == "ec2"
images = client.images(:owner_id=>:self)
else
images = client.images
end
# FIXME: this should probably be in the same transaction as pool.save
self.transaction do
realms.each do |realm|
#ignore if it exists
#FIXME: we need to handle keeping in sync forupdates as well as
# account permissions
unless Realm.find_by_external_key_and_provider_id(realm.id,
cloud_account.provider.id)
ar_realm = Realm.new(:external_key => realm.id,
:name => realm.name ? realm.name : realm.id,
:provider_id => cloud_account.provider.id)
ar_realm.save!
end
end
images.each do |image|
#ignore if it exists
#FIXME: we need to handle keeping in sync for updates as well as
# account permissions
ar_image = Image.find_by_external_key_and_provider_id(image.id,
cloud_account.provider.id)
unless ar_image
ar_image = Image.new(:external_key => image.id,
:name => image.name ? image.name :
(image.description ? image.description :
image.id),
:architecture => image.architecture,
:provider_id => cloud_account.provider.id)
ar_image.save!
end
front_end_image = Image.new(:external_key =>
cloud_account.account_prefix_for_realm +
Realm::AGGREGATOR_REALM_ACCOUNT_DELIMITER +
ar_image.external_key,
:name => ar_image.name,
:architecture => ar_image.architecture,
:pool_id => id)
front_end_image.save!
end
cloud_account.provider.hardware_profiles.each do |hardware_profile|
front_hardware_profile = HardwareProfile.new(:external_key =>
cloud_account.account_prefix_for_realm +
hardware_profile.external_key,
:name => hardware_profile.name,
:memory => hardware_profile.memory,
:storage => hardware_profile.storage,
:architecture => hardware_profile.architecture,
:pool_id => id)
front_hardware_profile.save!
end
end
- end
- end
end diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb index 4e4e3b7..20b9e1e 100644 --- a/src/app/models/provider.rb +++ b/src/app/models/provider.rb @@ -61,6 +61,18 @@ class Provider < ActiveRecord::Base :architecture => hardware_profile.architecture, :provider_id => id) ar_hardware_profile.save!
front_hwp = HardwareProfile.new(:external_key =>
name +
Realm::AGGREGATOR_REALM_ACCOUNT_DELIMITER +
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)
front_hwp.provider_hardware_profiles << ar_hardware_profile
end endfront_hwp.save! end
diff --git a/src/app/util/taskomatic.rb b/src/app/util/taskomatic.rb index c0dcbef..e66bb62 100644 --- a/src/app/util/taskomatic.rb +++ b/src/app/util/taskomatic.rb @@ -50,6 +50,14 @@ class Taskomatic begin client = @task.instance.cloud_account.connect realm = @task.instance.realm.external_key rescue nil
# Map aggregator HWP/image to back-end provider HWP/image in instance
unless @task.instance.image.provider_image?
@task.instance.image = @task.instance.image.provider_images[0]
end
unless @task.instance.hardware_profile.provider_hardware_profile?
@task.instance.hardware_profile = @task.instance.hardware_profile.provider_hardware_profiles[0]
end dcloud_instance = client.create_instance(@task.instance.image.external_key, :flavor => @task.instance.hardware_profile.external_key, :realm => realm,
diff --git a/src/app/views/cloud_accounts/_form.erb b/src/app/views/cloud_accounts/_form.erb deleted file mode 100644 index 4fa1a79..0000000 --- a/src/app/views/cloud_accounts/_form.erb +++ /dev/null @@ -1,13 +0,0 @@ -<ul>
<li>
- <%= form.label :username, "Cloud Account username" %>
- <%= form.text_field :username %>
</li>
<li>
- <%= form.label :password, "Cloud Account password" %>
- <%= form.password_field :password %>
</li>
<li>
<%= select("provider", "id", @providers.map{|p| [p.name,p.id]} , { :include_blank => false }) %>
</li>
-</ul> diff --git a/src/app/views/cloud_accounts/_form.html.erb b/src/app/views/cloud_accounts/_form.html.erb index 9b7b176..0d71953 100644 --- a/src/app/views/cloud_accounts/_form.html.erb +++ b/src/app/views/cloud_accounts/_form.html.erb @@ -3,7 +3,7 @@ <%= hidden_field :cloud_account, :id %> <%= hidden_field :cloud_account, :provider_id, :value => @provider.id %>
<ul> -<% if @cloud_account.nil? %> +<% if @cloud_account.id.nil? %> <li><label>UserName<span>UserName for the account you wish to connect to this pool.</span></label><%= text_field :cloud_account, :username %></li> <% end %> <li><label>Password<span>Password for the account you wish to connect to this pool.</span></label><%=password_field :cloud_account, :password %></li> diff --git a/src/app/views/cloud_accounts/new.html.erb b/src/app/views/cloud_accounts/new.html.erb index 40f32cb..c8e6d60 100644 --- a/src/app/views/cloud_accounts/new.html.erb +++ b/src/app/views/cloud_accounts/new.html.erb @@ -1,14 +1,10 @@ -<% if @providers.size == 0 %> -<h1>No Providers available to associate with a new cloud account</h1> -<% else %> -<h2 class="greeting">New Cloud Account</h2> - <div class="dcloud_form"> + <%= error_messages_for 'account' %> + + <h2>Add a new Account from this Provider</h2><br /> <% form_for @cloud_account, :url => { :action => "create" } do |f| %> <%= f.error_messages %> <%= render :partial => "form", :object => f %> - <%= f.submit "Create Cloud Account", :class => "submit" %> <% end %> <%= link_to "Cancel", root_path, :class => 'actionlink' %> </div> -<% end %> diff --git a/src/app/views/cloud_accounts/new_from_pool.html.erb b/src/app/views/cloud_accounts/new_from_pool.html.erb deleted file mode 100644 index 747df03..0000000 --- a/src/app/views/cloud_accounts/new_from_pool.html.erb +++ /dev/null @@ -1,11 +0,0 @@ -<h2 class="greeting">New Cloud Account</h2> - -<div class="dcloud_form"> - <% form_for @cloud_account, :url => { :action => "create_from_pool" } do |f| %> - <%= f.error_messages %> - <%= render :partial => "form", :object => f %> - <%= hidden_field :pool, :id %> - <%= f.submit "Create Cloud Account", :class => "submit" %> - <% end %> - <%= link_to "Cancel", root_path, :class => 'actionlink' %> -</div> diff --git a/src/app/views/provider/accounts.html.erb b/src/app/views/provider/accounts.html.erb index a763a33..ddc8f27 100644 --- a/src/app/views/provider/accounts.html.erb +++ b/src/app/views/provider/accounts.html.erb @@ -20,4 +20,4 @@ </tbody> </table> <% end %> -<%= link_to "Add an account", {:action => "new_account", :id => @provider}, :class => "actionlink" %> +<%= link_to "Add an account", {:controller => "cloud_accounts", :action => "new", :provider_id => @provider}, :class => "actionlink" %> diff --git a/src/app/views/provider/new_account.html.erb b/src/app/views/provider/new_account.html.erb deleted file mode 100644 index eed474f..0000000 --- a/src/app/views/provider/new_account.html.erb +++ /dev/null @@ -1,11 +0,0 @@ -<div class="dcloud_form"> - <%= error_messages_for 'pool' %> - <%= error_messages_for 'account' %> - - <h2>Create an Account for this Provider</h2><br /> - - <% form_tag :action => 'create_account' do |form| -%> - <%= render :partial => 'cloud_accounts/form' %> - <% end %> - -</div> diff --git a/src/app/views/provider/show.html.erb b/src/app/views/provider/show.html.erb index 0897828..f4213a9 100644 --- a/src/app/views/provider/show.html.erb +++ b/src/app/views/provider/show.html.erb @@ -1,24 +1,3 @@ -<% if @provider.pools.size == 0 %> -<h1>There are no pools to display</h1> -<% else %> - <table> - <thead> - <tr> - <th scope="col">Name</th> - <th scope="col">Instances</th> - </tr> - </thead> - <tbody> - <%@provider.pools.each {|pool| %> - <tr> - <td><%= pool.name %></td> - <td><%= pool.instances.count %></td> - </tr> - <% } %> - </tbody> - </table> -<% end %> -<%= link_to "Add a pool", {:controller => "pool", :action => "new", :provider => @provider}, :class => "actionlink" %> <%= link_to "Realms", {:action => "realms", :id => @provider.id}, :class=>"actionlink"%> <%= link_to "Accounts", {:action => "accounts", :id => @provider.id}, :class=>"actionlink"%> <%= link_to "User access", {:controller => "permissions", :action => "list", :provider_id => @provider.id}, :class=>"actionlink" if has_view_perms? %> diff --git a/src/db/migrate/20090804135630_create_hardware_profiles.rb b/src/db/migrate/20090804135630_create_hardware_profiles.rb index f3e1b65..95bc0ce 100644 --- a/src/db/migrate/20090804135630_create_hardware_profiles.rb +++ b/src/db/migrate/20090804135630_create_hardware_profiles.rb @@ -28,7 +28,6 @@ class CreateHardwareProfiles < ActiveRecord::Migration t.float :storage, :null => false t.string :architecture, :null => false t.integer :provider_id - t.integer :pool_id t.integer :lock_version, :default => 0 t.timestamps end diff --git a/src/db/migrate/20090804140143_create_images.rb b/src/db/migrate/20090804140143_create_images.rb index bb3db82..8c9c962 100644 --- a/src/db/migrate/20090804140143_create_images.rb +++ b/src/db/migrate/20090804140143_create_images.rb @@ -26,7 +26,6 @@ class CreateImages < ActiveRecord::Migration t.string :name, :null => false, :limit => 1024 t.string :architecture, :null => false t.integer :provider_id - t.integer :pool_id t.integer :lock_version, :default => 0 t.timestamps end diff --git a/src/features/pool.feature b/src/features/pool.feature index c96c2a9..5d8f219 100644 --- a/src/features/pool.feature +++ b/src/features/pool.feature @@ -45,19 +45,3 @@ Feature: Manage Pools When I follow "Realms" Then I should see "Europe" And I should see "United States" - - Scenario: View Pool's Images - Given I own a pool named "mockpool" - And the Pool has the following Images: - | name | architecture | - | Fedora 10 | x86_64 | - | Fedora 10 | i386 | - | JBoss | i386 | - And I am on the homepage - When I follow "mockpool" - Then I should be on the show pool page - When I follow "View Images" - Then I should see the following: - | Fedora 10 | x86_64 | - | Fedora 10 | i386 | - | JBoss | i386 | diff --git a/src/spec/controllers/cloud_accounts_controller_spec.rb b/src/spec/controllers/cloud_accounts_controller_spec.rb index f1d4a0d..d3e0f18 100644 --- a/src/spec/controllers/cloud_accounts_controller_spec.rb +++ b/src/spec/controllers/cloud_accounts_controller_spec.rb @@ -48,4 +48,16 @@ describe CloudAccountsController do response.should_not be_success end
- it "should provide ui to create new account" do
UserSession.create(@admin)
get :new, :provider_id => @provider.id
response.should be_success
response.should render_template("new")
- end
- it "should fail to grant access to account UIs for unauthenticated user" do
get :new
response.should_not be_success
- end
end diff --git a/src/spec/controllers/provider_controller_spec.rb b/src/spec/controllers/provider_controller_spec.rb index de894ca..096af3d 100644 --- a/src/spec/controllers/provider_controller_spec.rb +++ b/src/spec/controllers/provider_controller_spec.rb @@ -17,19 +17,9 @@ describe ProviderController do response.should render_template("accounts") end
it "should provide ui to create new account" do
UserSession.create(@admin)
get :new_account, :id => @provider.id
response.should be_success
response.should render_template("new_account")
end
it "should fail to grant access to account UIs for unauthenticated user" do get :accounts response.should_not be_success
get :new_account
response.should_not be_success
end
it "should provide ui to view hardware profiles" do
diff --git a/src/spec/factories/image.rb b/src/spec/factories/image.rb index dc55ac0..cc0bc17 100644 --- a/src/spec/factories/image.rb +++ b/src/spec/factories/image.rb @@ -8,7 +8,6 @@ Factory.define :image do |i| i.provider { |p| Provider.new } end
-Factory.define :pool_image, :parent => :image do |i| +Factory.define :front_end_image, :parent => :image do |i| i.provider nil
- i.association(:pool)
end diff --git a/src/spec/models/hardware_profile_spec.rb b/src/spec/models/hardware_profile_spec.rb index 78fc0a2..e2b09b4 100644 --- a/src/spec/models/hardware_profile_spec.rb +++ b/src/spec/models/hardware_profile_spec.rb @@ -55,69 +55,28 @@ describe HardwareProfile do @hp.should_not be_valid end
- it "should reject Aggregator profiles for custom Instance profiles" do
- it "should allow Aggregator profiles only for provider profiles" do
- @hp.provider = nil
- @hp.aggregator_hardware_profiles << @hp
- @hp.should_not be_valid @hp.should have(1).error_on(:aggregator_hardware_profiles) @hp.errors.on(:aggregator_hardware_profiles).should eql(
"Aggregator profiles are not allowed for custom Instance profiles")
"Aggregator profiles only allowed for provider profiles")
@hp.aggregator_hardware_profiles.clear @hp.should be_valid
end
it "should reject Provider profiles for custom Instance profiles" do
@hp.provider_hardware_profiles << @hp
@hp.should_not be_valid
@hp.should have(1).error_on(:provider_hardware_profiles)
@hp.errors.on(:provider_hardware_profiles).should eql(
"Provider profiles are not allowed for custom Instance profiles")
@hp.provider_hardware_profiles.clear
@hp.should be_valid end
it "should require either provider or pool to be blank" do
- it "should allow Provider profiles only for aggregator profiles" do @hp.provider = Provider.new
@hp.pool = Pool.new
@hp.should_not be_valid
@hp.should have(1).error_on(:provider)
@hp.errors.on(:provider).should eql("provider or pool must be blank")
@hp.should have(1).error_on(:pool)
@hp.errors.on(:pool).should eql("provider or pool must be blank")
@hp.provider = nil
@hp.should be_valid
@hp.provider = Provider.new
@hp.pool = nil
@hp.should be_valid
end
it "should allow Provider profiles only for provider profiles" do
@hp.provider = nil
@hp.pool = Pool.new
@hp.provider_hardware_profiles << @hp
- @hp.aggregator_hardware_profiles << @hp @hp.should have(1).error_on(:provider_hardware_profiles) @hp.errors.on(:provider_hardware_profiles).should eql(
"Provider profiles only allowed for provider profiles")
"Provider profiles only allowed for aggregator profiles")
@hp.provider_hardware_profiles.clear @hp.should be_valid end
- it "should allow Aggregator profiles only for pool profiles" do
- @hp.provider = Provider.new
- @hp.pool = nil
- @hp.aggregator_hardware_profiles << @hp
- @hp.should have(1).error_on(:aggregator_hardware_profiles)
- @hp.errors.on(:aggregator_hardware_profiles).should eql(
"Aggregator profiles only allowed for pool profiles")
- @hp.aggregator_hardware_profiles.clear
- @hp.should be_valid
- end
end diff --git a/src/spec/models/image_spec.rb b/src/spec/models/image_spec.rb index de68475..a0fd8ca 100644 --- a/src/spec/models/image_spec.rb +++ b/src/spec/models/image_spec.rb @@ -46,33 +46,8 @@ describe Image do i.should be_valid end
- it "should have either a provider or a pool specified" do
- i = Factory.build(:image, :provider => nil, :pool => nil)
- i.should have(1).error_on(:provider)
- i.should have(1).error_on(:pool)
- i.errors.on(:provider).should eql(
"provider or pool must be specified")
- i.errors.on(:pool).should eql(
"provider or pool must be specified")
- i.provider = @provider
- i.should be_valid
- i.pool = Factory.build(:pool)
- i.should have(1).error_on(:provider)
- i.should have(1).error_on(:pool)
- i.errors.on(:provider).should eql(
"provider or pool must be blank")
- i.errors.on(:pool).should eql(
"provider or pool must be blank")
- i.provider = nil
- i.should be_valid
- end
- it "should have provider images only if it has a provider" do
- i = Factory.create(:image, :pool => Pool.new,
:provider => nil)
i = Factory.create(:image, :provider => nil)
i.aggregator_images << i i.should have(1).error_on(:aggregator_images)
-- 1.6.2.5
deltacloud-devel mailing list deltacloud-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/deltacloud-devel
ACK ACK! ACK ACK! </mars attacks>
deltacloud-devel@lists.fedorahosted.org