[PATCH] Fix return value from ec2 *_instance methods.
by Chris Lalancette
The instance_action method in server.rb is clearly expecting
the individual drivers to return an "Instance" object when the
request is successfully fulfilled. Unfortunately, the return
value from the individual drivers is woefully inconsistent here;
some of them return a new Instance object, and some do not.
Further, as per
https://fedorahosted.org/pipermail/deltacloud-devel/2010-May/001130.html
it seems like we should be doing a better job handling
cases where we can create a full Instance object vs. a
truncated one.
To help fix both of these, the following patch:
1) Changes the haml file to only show the fields of
the instance that are actually available. This means that
if an individual driver cannot reasonably return good
information about an instance after an action has been
performed, it can at least return the href and id of
the instance; it would then be up to the client to do
another GET with the instance HREF to get additional
information.
2) Fixes up the EC2 driver to return the expected
instance object. In particular, we now throw an exception
if the action fails, but if the follow-up request for
instance information fails, we just return a truncated
Instance object.
In going over the drivers, it looks like:
Mock - returns Instance object - good
EC2 - (after this patch) returns Instance object - good
GoGrid - does not return Instance object - bad
OpenNebula - returns Instance object - good
RackSpace - does not return Instance object - bad
RHEV-M - returns Instance object - good
Rimu - does not return Instance object - bad
So if it is agreed this is the direction to go, I will
fix up the other broken drivers as well.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
server/lib/deltacloud/drivers/ec2/ec2_driver.rb | 54 +++++++++++++++++++--
server/views/instances/show.xml.haml | 57 +++++++++++++----------
2 files changed, 81 insertions(+), 30 deletions(-)
diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
index 69356e2..492a059 100644
--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
@@ -168,22 +168,64 @@ class EC2Driver < Deltacloud::BaseDriver
def reboot_instance(credentials, id)
ec2 = new_client(credentials)
- safely do
- ec2.reboot_instances( :instance_id => id )
+ backup = ec2.reboot_instances( :instance_id => id )
+
+ begin
+ this_instance = ec2.describe_instances( :instance_id => id).reservationSet.item.first.instancesSet.item.first
+ convert_instance(this_instance, this_instance.ownerId)
+ rescue Exception => e
+ puts "ERROR: #{e.message}"
+ # at this point, the reboot has succeeded but our follow-up
+ # "describe_instances" failed for some reason. Create a simple Instance
+ # object with only the ID and new state in place
+ state = backup.instancesSet.item.first.currentState.name
+ Instance.new( {
+ :id => id,
+ :state => state,
+ :actions => instance_actions_for( state ),
+ } )
end
end
def stop_instance(credentials, id)
ec2 = new_client(credentials)
- safely do
- ec2.terminate_instances( :instance_id => id )
+ backup = ec2.terminate_instances( :instance_id => id )
+
+ begin
+ this_instance = ec2.describe_instances( :instance_id => id ).reservationSet.item.first.instancesSet.item.first
+ convert_instance(this_instance, this_instance.ownerId)
+ rescue Exception => e
+ puts "ERROR: #{e.message}"
+ # at this point, the stop has succeeded but our follow-up
+ # "describe_instances" failed for some reason. Create a simple Instance
+ # object with only the ID and new state in place
+ state = backup.instancesSet.item.first.currentState.name
+ Instance.new( {
+ :id => id,
+ :state => state,
+ :actions => instance_actions_for( state ),
+ } )
end
end
def destroy_instance(credentials, id)
ec2 = new_client(credentials)
- safely do
- ec2.terminate_instances( :instance_id => id )
+ backup = ec2.terminate_instances( :instance_id => id )
+
+ begin
+ this_instance = ec2.describe_instances( :instance_id => id).reservationSet.item.first.instancesSet.item.first
+ convert_instance(this_instance, this_instance.ownerId)
+ rescue Exception => e
+ puts "ERROR: #{e.message}"
+ # at this point, the destroy has succeeded but our follow-up
+ # "describe_instances" failed for some reason. Create a simple Instance
+ # object with only the ID and new state in place
+ state = backup.instancesSet.item.first.currentState.name
+ Instance.new( {
+ :id => id,
+ :state => state,
+ :actions => instance_actions_for( state ),
+ } )
end
end
diff --git a/server/views/instances/show.xml.haml b/server/views/instances/show.xml.haml
index a4859b0..9ed1b89 100644
--- a/server/views/instances/show.xml.haml
+++ b/server/views/instances/show.xml.haml
@@ -2,27 +2,36 @@
%instance{:href => instance_url((a)instance.id)}
%id<
=(a)instance.id
- %name<
- =(a)instance.name
- %owner_id<
- =(a)instance.owner_id
- %image{:href => image_url((a)instance.image_id)}
- %realm{:href => realm_url((a)instance.realm_id)}
- %state<
- =(a)instance.state
- - haml_tag :"hardware-profile", {:href => hardware_profile_url((a)instance.instance_profile.id)} do
- %id<
- =(a)instance.instance_profile.id
- - @instance.instance_profile.overrides.each do |p, v|
- %property{:kind => 'fixed', :name => p, :value => v, :unit => Deltacloud::HardwareProfile::unit(p)}
- %actions
- - @instance.actions.compact.each do |instance_action|
- %link{:rel => instance_action, :method => instance_action_method(instance_action), :href => self.send("#{instance_action}_instance_url", @instance.id)}
- %public-addresses
- - @instance.public_addresses.each do |address|
- %address<
- =address
- %private-addresses
- - @instance.private_addresses.each do |address|
- %address<
- =address
+ - if @instance.name
+ %name<
+ =(a)instance.name
+ - if @instance.owner_id
+ %owner_id<
+ =(a)instance.owner_id
+ - if @instance.image_id
+ %image{:href => image_url((a)instance.image_id)}
+ - if @instance.realm_id
+ %realm{:href => realm_url((a)instance.realm_id)}
+ - if @instance.state
+ %state<
+ =(a)instance.state
+ - if @instance.instance_profile
+ - haml_tag :"hardware-profile", {:href => hardware_profile_url((a)instance.instance_profile.id)} do
+ %id<
+ =(a)instance.instance_profile.id
+ - @instance.instance_profile.overrides.each do |p, v|
+ %property{:kind => 'fixed', :name => p, :value => v, :unit => Deltacloud::HardwareProfile::unit(p)}
+ - if @instance.actions
+ %actions
+ - @instance.actions.compact.each do |instance_action|
+ %link{:rel => instance_action, :method => instance_action_method(instance_action), :href => self.send("#{instance_action}_instance_url", @instance.id)}
+ - if @instance.public_addresses
+ %public-addresses
+ - @instance.public_addresses.each do |address|
+ %address<
+ =address
+ - if @instance.private_addresses
+ %private-addresses
+ - @instance.private_addresses.each do |address|
+ %address<
+ =address
--
1.6.6.1
14 years, 1 month
[PATCH aggregator] Missing fixtures in these tests.
by jason.guiditta
This was causing intermittent failures, as the
fixtures set up some required data that is not
handled in factories (at least at this time).
---
src/spec/controllers/pool_controller_spec.rb | 2 +-
.../controllers/user_sessions_controller_spec.rb | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/spec/controllers/pool_controller_spec.rb b/src/spec/controllers/pool_controller_spec.rb
index bb559ee..7917ed3 100644
--- a/src/spec/controllers/pool_controller_spec.rb
+++ b/src/spec/controllers/pool_controller_spec.rb
@@ -1,7 +1,7 @@
require 'spec_helper'
describe PoolController do
-
+ fixtures :all
before(:each) do
@admin_permission = Factory :admin_permission
@admin = @admin_permission.user
diff --git a/src/spec/controllers/user_sessions_controller_spec.rb b/src/spec/controllers/user_sessions_controller_spec.rb
index 1c0cfcc..5240770 100644
--- a/src/spec/controllers/user_sessions_controller_spec.rb
+++ b/src/spec/controllers/user_sessions_controller_spec.rb
@@ -1,7 +1,7 @@
require 'spec_helper'
describe UserSessionsController do
-
+ fixtures :all
before(:each) do
@tuser = Factory :tuser
activate_authlogic
--
1.6.6.1
14 years, 1 month
[PATCH 1/2] First round of 'remove pool-account association' changes.
by Scott Seago
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(a)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?((a)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?((a)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>
- <%(a)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 bb559ee..fa7857b 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/#{id}")
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
--
1.6.2.5
14 years, 1 month
Re: [deltacloud-devel] Git config/patch standards for developers
by Martyn Taylor
Hi Guys,
I wrote a script to clone the repos, and activate the hooks. Thought it may be useful to others.
It just saves time messing around every time you want to checkout a clean repo.
As default all the hooks are set to true, you can set the ones you don't want as false at the top of the script,
Use ./clone_dc.sh (-a -c -d) to clone the aggregator, core, docs repo respectively
Cheers
Martyn
----- Original Message -----
From: "Jason Guiditta" <jason.guiditta(a)gmail.com>
To: deltacloud-devel(a)lists.fedorahosted.org
Sent: Tuesday, May 11, 2010 10:04:23 PM GMT +00:00 GMT Britain, Ireland, Portugal
Subject: Re: [deltacloud-devel] Git config/patch standards for developers
A link might be handy, eh?
http://deltacloud.org/page/Git_config
On Tue, May 11, 2010 at 4:51 PM, Jason Guiditta < jason.guiditta(a)gmail.com > wrote:
Hello all,
We have had some issues with inconsistency in the setup of people's git configuration (whitespace sneaking in, for example), so I thought I would try to put together a page with examples of how things should be set up, as well as some guidelines for patch submittal. I don't expect I have covered everything, so feel free to add points that did not cross my mind for this first pass, or make a correction if you see a mistake. If any new developer has questions on how to start submitting patches, I would like to be able to direct them to this as our canonical guideline doc. Patches that apply cleanly, are clearly for one repo or another, and have useful descriptions are much more pleasant to review. This also increases the likelihood of said review happening. Thank you for your time.
</soapbox>
-j
_______________________________________________
deltacloud-devel mailing list
deltacloud-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/deltacloud-devel
14 years, 1 month
Re: [deltacloud-devel] [rest-practices] ID as an attribute [was Re: Towards declaring the Deltacloud API stable]
by Michal Fojtik
Hi,
On 12/05/10 10:44 +0100, Mark McLoughlin wrote:
>Just wanted to bring this up again - it got lost in the mix
>
>I don't feel very strongly about it either way, but I do prefer:
>
> POST /storagedomains/1234/attachments HTTP/1.1
>
> <attachment><datacenter id="5678"/></attachment>
>
>over:
>
> POST /storagedomains/1234/attachments HTTP/1.1
>
> <attachment><datacenter><id>"5678"</id></datacenter></attachment>
Using 'id' as an atribute looks more naturally for me.
From w3schools.com:
<quote>
The ID above is just an identifier, to identify the different notes. It is
not a part of the note itself.
What I'm trying to say here is that metadata (data about data) should be
stored as attributes, and that data itself should be stored as elements."
</quote>
-- Michal
--
--------------------------------------------------------
Michal Fojtik, mfojtik(a)redhat.com, +420 532 294 4307
Ruby / Ruby On Rails Developer
Deltacloud API: http://deltacloud.org
--------------------------------------------------------
14 years, 1 month
Instance authentification features (rev 2)
by Michal Fojtik
Hi,
Second revision of authentification features includes new XML schema, suggested by lutter:
<authentication name="key|password">
</authentication>
And:
- Fixed authentification -> authentication
- Added more error reporting:
- GoGrid: report if no password / username
- EC2: report if no key was assigned to instance
-- Michal
14 years, 1 month
Cucumber features updates
by Michal Fojtik
Hi,
I updated all Cucumber features to reflect recent changes in API and added a couple of new tests to achieve better coverage. Also I removed configuration for Cucumber tests, because I plan to do a 'auto-learning' tests for drivers, which will be shipped separately.
Last thing I added is a jUnit output in Cucumber for better integration with Hudson. Also I deleted obsolete XML converter (we are using pure HAML for generating XML output now).
Sadly, there are two issues which blows up after I completed all features:
1) When you stop/reboot/start instance, XML which is returned has blank <id/>
2) We are missing JSON output for hardware_profiles and instance states
-- Michal
14 years, 1 month
removed will_paginate plugin
by Jan Provazník
will_paginate is already packed in gem, plugin removed from vendor and gem dependency added to environment.rb
14 years, 1 month
Re: [deltacloud-devel] [PATCH] Added Validation for Cloud Account Provider Credentials
by Martyn Taylor
Mo,
Thanks for reviewing this so quickly, I've replied to your comments inline,
Cheers
Martyn
----- Original Message -----
From: "Mohammed Morsi" <mmorsi(a)redhat.com>
To: mtaylor(a)redhat.com
Cc: deltacloud-devel(a)lists.fedorahosted.org
Sent: Monday, May 10, 2010 8:45:15 PM GMT +00:00 GMT Britain, Ireland, Portugal
Subject: Re: [deltacloud-devel] [PATCH] Added Validation for Cloud Account Provider Credentials
On 05/10/2010 09:56 AM, mtaylor(a)redhat.com wrote:
> From: martyntaylor<mtaylor(a)redhat.com>
>
> ---
> src/app/controllers/cloud_accounts_controller.rb | 2 +-
> src/app/controllers/provider_controller.rb | 11 ++++++++---
> src/app/models/cloud_account.rb | 16 ++++++++++++++++
> src/app/views/cloud_accounts/new.html.erb | 1 +
> src/features/support/env.rb | 6 ++++++
> .../controllers/cloud_accounts_controller_spec.rb | 7 +++++++
> src/spec/spec_helper.rb | 7 +++++++
> 7 files changed, 46 insertions(+), 4 deletions(-)
>
Tried it out and it looks good for the most part, though there are some
caveats below.
> diff --git a/src/app/controllers/cloud_accounts_controller.rb b/src/app/controllers/cloud_accounts_controller.rb
> index 05e024e..23b3113 100644
> --- a/src/app/controllers/cloud_accounts_controller.rb
> +++ b/src/app/controllers/cloud_accounts_controller.rb
> @@ -95,4 +95,4 @@ class CloudAccountsController< ApplicationController
> end
> redirect_to :controller => 'provider', :action => 'accounts', :id => provider.id
> end
> -end
> +end
> \ No newline at end of file
>
-What is the change here? I'm not even seeing a whitespace difference.
-Though that being said, I was unable to apply your patch until I ran
-dos2unix on it. I'm not sure whose end this is a problem on, I remember
-having this issue in the past, but thought I had resolved it. Could you
-tell me if either of your ~/.gitconfig or your aggregator checkout
-.git/config has a CRLF option configured, and what it is set to. If not,
-we should see if anyone else has a problem applying it to see who has
-the issue.
OK, I dont actually think there is a change here. I'm not sure if this is a result of my improper use of GIT or whether this has occurred due to some obscure reason. Anyways, I think it is safe to ignore this.
Myself and Marios have tried applying the patch on a fresh Aggregator and it seems to apply OK for us.
> diff --git a/src/app/controllers/provider_controller.rb b/src/app/controllers/provider_controller.rb
> index 23b5ba0..092054c 100644
> --- a/src/app/controllers/provider_controller.rb
> +++ b/src/app/controllers/provider_controller.rb
> @@ -76,8 +76,13 @@ class ProviderController< ApplicationController
> 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
> + if @acct.save
> + @provider.cloud_accounts<< @acct
> + redirect_to :action => 'accounts', :id => @provider.id
> + else
> + flash[:notice] = @acct.errors.full_messages[0]
> + redirect_to :action => 'new_account', :id => @provider.id
> + end
> end
>
> -end
> +end
> \ No newline at end of file
> diff --git a/src/app/models/cloud_account.rb b/src/app/models/cloud_account.rb
> index 1d6b3e9..f04187d 100644
> --- a/src/app/models/cloud_account.rb
> +++ b/src/app/models/cloud_account.rb
> @@ -67,4 +67,20 @@ class CloudAccount< ActiveRecord::Base
> def name
> username
> end
> +
> + protected
> + def validate
> + errors.add_to_base("Login Credentials are Invalid for this Provider") unless valid_credentials?
> + end
> +
> + private
> + def valid_credentials?
> + begin
> + deltacloud = DeltaCloud.new(username, password, provider.url)
> + deltacloud.instances
> + rescue Exception => e
> + return false
> + end
> + return true
> + end
> end
>
-Why not just use the 'connect' method defined above in the CloudAccount
-class, it does the exact same thing. The Provider validate method does
-it that way.
We can't just use the connect method here because the connect method only connects to the Provider, it does not perform any action that requires authentication. Since we are checking the validation of login credentials we need to test for an AuthenticationException to see if they are valid. To do this I've tried listing the instances (since this needs authentication). However, there was some discussion on IRC yesterday about possibly adding a "test credentials" type method to the API.
> diff --git a/src/app/views/cloud_accounts/new.html.erb b/src/app/views/cloud_accounts/new.html.erb
> index 40f32cb..8d4fcc8 100644
> --- a/src/app/views/cloud_accounts/new.html.erb
> +++ b/src/app/views/cloud_accounts/new.html.erb
> @@ -4,6 +4,7 @@
> <h2 class="greeting">New Cloud Account</h2>
>
> <div class="dcloud_form">
> +<%= error_messages_for 'cloud_account' %>
> <% form_for @cloud_account, :url => { :action => "create" } do |f| %>
> <%= f.error_messages %>
> <%= render :partial => "form", :object => f %>
> diff --git a/src/features/support/env.rb b/src/features/support/env.rb
> index 2d6e9a1..91327a8 100644
> --- a/src/features/support/env.rb
> +++ b/src/features/support/env.rb
> @@ -30,6 +30,12 @@ Provider.class_eval do
> end
> end
>
> +CloudAccount.class_eval do
> + alias_method :original_validate, :validate
> + def validate
> + end
> +end
> +
>
-I'm still pretty new to rspec / cucumber, but is it wise to overload the
-validate method at this level? eg in other specs / features we might
-want to test the validate method itself, especially if it gets expanded.
-What is the benefit of doing this here?
Good point here, I was in some deliberation over this. After giving this some thought I decided to go ahead with overriding the method here. I think however, that it maybe better to simply override the valid_credentials? method.
The reasoning behind doing this, is that each time a CloudAccount is created it tries to validate the credentials. For our tests, we don't have a core setup to be able to do this, (the same applies for Provider validation). An alternative way to do this is to use mock objects and override the method on the individual object. This is how Tomas, overcome the problem in some of the spec tests. The problem here is that when running a test that uses a REST, generally a new object is created from the parameters specified in the call. Since this new object is different from the object we stubbed, the method is not overidden.
Also if we really did want to test "valid_credentials?" and also the "connect" method on the provider, we need some form of integrated testing, so we can integrate core and aggregator, which we currently don't have.
I'm open to suggestion on this tho, so if you can think of a better way to do this, it would be great :-)
> # If you set this to false, any error raised from within your app will bubble
> # up to your step definition and out to cucumber unless you catch it somewhere
> # on the way. You can make Rails rescue errors and render error pages on a
> diff --git a/src/spec/controllers/cloud_accounts_controller_spec.rb b/src/spec/controllers/cloud_accounts_controller_spec.rb
> index f1d4a0d..4dbac78 100644
> --- a/src/spec/controllers/cloud_accounts_controller_spec.rb
> +++ b/src/spec/controllers/cloud_accounts_controller_spec.rb
> @@ -48,4 +48,11 @@ describe CloudAccountsController do
> response.should_not be_success
> end
>
> + it "should fail to create a cloud account if the provider credentials are invalid" do
> + cloud_account = Factory.build(:mock_cloud_account)
> + cloud_account.stub!(:valid_credentials?).and_return(false)
> + cloud_account.save.should == false
> + end
> +
> +
> end
> diff --git a/src/spec/spec_helper.rb b/src/spec/spec_helper.rb
> index 5181389..b880a52 100644
> --- a/src/spec/spec_helper.rb
> +++ b/src/spec/spec_helper.rb
> @@ -21,6 +21,13 @@ Spec::Runner.configure do |config|
> config.use_instantiated_fixtures = false
> config.fixture_path = RAILS_ROOT + '/spec/fixtures/'
>
> +CloudAccount.class_eval do
> + alias_method :original_valid_credentials?, :valid_credentials?
> + def valid_credentials?
> + return true
> + end
> +end
> +
>
Same comment as above as far as overriding the validation methods.
-Mo
14 years, 1 month