[PATCH aggregator] BZ#640192 Fixed Validation on Quota Fields
by Martyn Taylor
From: martyntaylor <mtaylor(a)redhat.com>
---
src/app/controllers/settings_controller.rb | 8 +++-
src/app/models/quota.rb | 10 +++--
src/app/views/settings/self_service.haml | 2 +-
src/features/settings.feature | 44 +++++++++++++++++++++++
src/features/step_definitions/settings_steps.rb | 10 +++++
src/features/support/paths.rb | 6 +++
6 files changed, 73 insertions(+), 7 deletions(-)
create mode 100644 src/features/settings.feature
create mode 100644 src/features/step_definitions/settings_steps.rb
diff --git a/src/app/controllers/settings_controller.rb b/src/app/controllers/settings_controller.rb
index b040f7f..ae20e12 100644
--- a/src/app/controllers/settings_controller.rb
+++ b/src/app/controllers/settings_controller.rb
@@ -53,8 +53,12 @@ class SettingsController < ApplicationController
KEYS.each do |key|
if params[key]
if key == SELF_SERVICE_DEFAULT_QUOTA
- self_service_default_quota = MetadataObject.lookup(key)
- self_service_default_quota.update_attributes(params[key])
+ @self_service_default_quota = MetadataObject.lookup(key)
+ if !(a)self_service_default_quota.update_attributes(params[key])
+ flash[:notice] = "Could not update the default quota"
+ render :self_service
+ return
+ end
elsif key == SELF_SERVICE_DEFAULT_POOL
if Pool.exists?(params[key])
MetadataObject.set(key, Pool.find(params[key]))
diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
index 83c20c4..cfc4555 100644
--- a/src/app/models/quota.rb
+++ b/src/app/models/quota.rb
@@ -28,14 +28,16 @@ class Quota < ActiveRecord::Base
validates_numericality_of :maximum_total_instances,
:greater_than_or_equal_to => 0,
:less_than_or_equal_to => 2147483647,
- :integer_only => true,
- :allow_nil => true
+ :only_integer => true,
+ :allow_nil => true,
+ :message => "must be a positive whole number less than 2147483647"
validates_numericality_of :maximum_running_instances,
:greater_than_or_equal_to => 0,
:less_than_or_equal_to => 2147483647,
- :integer_only => true,
- :allow_nil => true
+ :only_integer => true,
+ :allow_nil => true,
+ :message => "must be a positive whole number less than 2147483647"
QuotaResource = Struct.new(:name, :used, :max, :available, :unit)
diff --git a/src/app/views/settings/self_service.haml b/src/app/views/settings/self_service.haml
index c1da0ae..8fd9950 100644
--- a/src/app/views/settings/self_service.haml
+++ b/src/app/views/settings/self_service.haml
@@ -8,7 +8,6 @@
.grid_13
= error_messages_for @parent_type
- = error_messages_for 'self_service_default_quota'
%h2
= t('.self_service_default')
- form_for @self_service_default_quota, :url => { :action => 'update' } do |form|
@@ -53,6 +52,7 @@
= t('.quota') + ":"
= text_field :self_service_default_quota, :maximum_running_instances, :class => 'grid_5'
.grid_2.la (instances)
+ = form.error_message_on :maximum_running_instances, 'Maximum Running Instances '
%h3 POOLS
%fieldset.clearfix
%label.grid_2.alpha Permissions:
diff --git a/src/features/settings.feature b/src/features/settings.feature
new file mode 100644
index 0000000..f41e188
--- /dev/null
+++ b/src/features/settings.feature
@@ -0,0 +1,44 @@
+Feature: Manage System wide Settings
+ In order to manage my cloud engine
+ As a user
+ I want to manage system settings
+
+ Background:
+ Given I am an authorised user
+ And I am logged in
+
+ Scenario: Change the self service default quota
+ Given the default quota is set to 5
+ And I am on the self service settings page
+ When I fill in "self_service_default_quota[maximum_running_instances]" with "8"
+ And I press "Save"
+ Then I should see "Settings Updated!"
+ And the default quota should be 8
+ And I should be on the self service settings page
+
+ Scenario: Invalid decimal entry for the self service default quota
+ Given the default quota is set to 5
+ And I am on the self service settings page
+ When I fill in "self_service_default_quota[maximum_running_instances]" with "1.5"
+ And I press "Save"
+ Then I should see "Could not update the default quota"
+ And the default quota should be 5
+ And I should be on the settings update page
+
+ Scenario: Invalid chars entry for the self service default quota
+ Given the default quota is set to 5
+ And I am on the self service settings page
+ When I fill in "self_service_default_quota[maximum_running_instances]" with "abc"
+ And I press "Save"
+ Then I should see "Could not update the default quota"
+ And the default quota should be 5
+ And I should be on the settings update page
+
+ Scenario: Invalid special chars entry for the self service default quota
+ Given the default quota is set to 5
+ And I am on the self service settings page
+ When I fill in "self_service_default_quota[maximum_running_instances]" with "^&(*_!"
+ And I press "Save"
+ Then I should see "Could not update the default quota"
+ And the default quota should be 5
+ And I should be on the settings update page
diff --git a/src/features/step_definitions/settings_steps.rb b/src/features/step_definitions/settings_steps.rb
new file mode 100644
index 0000000..7b1a7d7
--- /dev/null
+++ b/src/features/step_definitions/settings_steps.rb
@@ -0,0 +1,10 @@
+Given /^the default quota is set to (\d+)$/ do |no_instances|
+ @default_quota = MetadataObject.lookup("self_service_default_quota")
+ @default_quota.maximum_running_instances = no_instances
+ @default_quota.save
+end
+
+Then /^the default quota should be (\d+)$/ do |no_instances|
+ @default_quota.reload
+ @default_quota.maximum_running_instances.should == no_instances.to_i
+end
\ No newline at end of file
diff --git a/src/features/support/paths.rb b/src/features/support/paths.rb
index b0a5f7e..fdc63f5 100644
--- a/src/features/support/paths.rb
+++ b/src/features/support/paths.rb
@@ -74,6 +74,12 @@ module NavigationHelpers
when /the create template page/
url_for :action => 'create', :controller => 'templates', :only_path => true
+ when /the self service settings page/
+ url_for :action => 'self_service', :controller => 'settings', :only_path => true
+
+ when /the settings update page/
+ url_for :action => 'update', :controller => 'settings', :only_path => true
+
# Add more mappings here.
# Here is an example that pulls values out of the Regexp:
#
--
1.7.2.3
13 years, 6 months
[PATCH aggregator] BZ #644050 - instances launched by non-admin users are listed as started by Admin
by Jan Provazník
From: Jan Provaznik <jprovazn(a)redhat.com>
User object was assigned to id. Also moved name generation into helper.
---
src/app/controllers/instance_controller.rb | 2 +-
src/app/helpers/instance_helper.rb | 10 ++++++++++
src/app/views/instance/index.haml | 4 ++--
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/app/controllers/instance_controller.rb b/src/app/controllers/instance_controller.rb
index e0aaf3b..ad90404 100644
--- a/src/app/controllers/instance_controller.rb
+++ b/src/app/controllers/instance_controller.rb
@@ -92,7 +92,7 @@ class InstanceController < ApplicationController
@instance = Instance.new(params[:instance])
@instance.state = Instance::STATE_NEW
- @instance.owner_id = current_user
+ @instance.owner = current_user
require_privilege(Privilege::INSTANCE_MODIFY,
Pool.find((a)instance.pool_id))
diff --git a/src/app/helpers/instance_helper.rb b/src/app/helpers/instance_helper.rb
index e0b6e98..fea3cf4 100644
--- a/src/app/helpers/instance_helper.rb
+++ b/src/app/helpers/instance_helper.rb
@@ -20,4 +20,14 @@
# Likewise, all the methods added will be available for all controllers.
module InstanceHelper
+ def owner_name(inst)
+ return '' unless inst.owner
+ # if last_name is set, use full name,
+ # else use login
+ if inst.owner.last_name.blank?
+ inst.owner.login
+ else
+ "#{inst.owner.first_name} #{inst.owner.last_name}"
+ end
+ end
end
diff --git a/src/app/views/instance/index.haml b/src/app/views/instance/index.haml
index 8024a2f..83690b7 100644
--- a/src/app/views/instance/index.haml
+++ b/src/app/views/instance/index.haml
@@ -6,7 +6,7 @@
{:name => 'TEMPLATE', :sort_attr => 'templates.name'}, |
{:name => 'PUBLIC ADDRESS', :sort_attr => 'public_addresses'}, |
{:name => 'PROVIDER', :sortable => false}, |
- {:name => 'STARTED BY', :sort_attr => 'users.last_name'}, |
+ {:name => 'CREATED BY', :sort_attr => 'users.last_name'}, |
] |
- pool_columns = [ |
@@ -124,4 +124,4 @@
%td= inst.template.name
%td= inst.public_addresses
%td= inst.cloud_account ? inst.cloud_account.provider.name : ''
- %td= "#{inst.owner.first_name} #{inst.owner.last_name}" # TODO, there is "started by" in comps pdf, but we don't save this info
+ %td= owner_name(inst)
--
1.7.2.3
13 years, 6 months
[PATCH image-factory 1/2] Test running 2 builds at once.
by Jason Guiditta
---
console/lib/console_harness.rb | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/console/lib/console_harness.rb b/console/lib/console_harness.rb
index 7c17384..d952127 100644
--- a/console/lib/console_harness.rb
+++ b/console/lib/console_harness.rb
@@ -34,11 +34,13 @@ i = ImageBuilderConsole.new(:logger =>log, :retry_limit => 3, :delay => 10)
log.info "Sleeping for 10 seconds before calling build_image"
sleep 10
ab = i.build_image(get_sample(log), "mock","some-id","security xml")
-log.info 'target is: ' + ab.target
-log.info 'status of build is: ' + ab.status
+ab2 = i.build_image(get_sample(log), "mock","another-id","security xml")
+log.info "targets are: #{ab.target}, #{ab2.target}"
+log.info "status of builds: #{ab.status} #{ab2.status}"
loop do
log.info 'checking for status update...'
- log.info "Got: " + i.check_status(ab).to_s
- break if ab.status.eql?("complete") || i.check_status(ab).to_s.eql?("failed")
+ log.info "Got: #{i.check_status(ab).to_s}, #{i.check_status(ab2).to_s}"
+ break if (ab.status.eql?("complete") || i.check_status(ab).to_s.eql?("failed")) &&
+ (ab2.status.eql?("complete") || i.check_status(ab2).to_s.eql?("failed"))
sleep 20
end
--
1.7.2.3
13 years, 6 months
[PATCH aggregator] BZ#644534 Added check for Quota before creating instance
by Martyn Taylor
From: martyntaylor <mtaylor(a)redhat.com>
---
src/app/controllers/instance_controller.rb | 24 +++++++++++++++---------
src/app/models/quota.rb | 20 ++++++++++++--------
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/src/app/controllers/instance_controller.rb b/src/app/controllers/instance_controller.rb
index ad90404..a81af6a 100644
--- a/src/app/controllers/instance_controller.rb
+++ b/src/app/controllers/instance_controller.rb
@@ -98,16 +98,22 @@ class InstanceController < ApplicationController
Pool.find((a)instance.pool_id))
#FIXME: This should probably be in a transaction
if @instance.save!
-
- @task = InstanceTask.new({:user => current_user,
- :task_target => @instance,
- :action => InstanceTask::ACTION_CREATE})
- if @task.save
- condormatic_instance_create(@task)
- flash[:notice] = "Instance added."
- redirect_to :action => 'index'
+ if Quota.can_start_instance?(@instance, nil)
+ @task = InstanceTask.new({:user => current_user,
+ :task_target => @instance,
+ :action => InstanceTask::ACTION_CREATE})
+ if @task.save
+ condormatic_instance_create(@task)
+ flash[:notice] = "Instance added."
+ redirect_to :action => 'index'
+ else
+ @pool = @instance.pool
+ render :action => 'configure'
+ end
else
- @pool = @instance.pool
+ @instance.destroy
+ flash[:notice] = "Quota Exceeded: Could not create instance"
+ @hardware_profiles = HardwareProfile.all
render :action => 'configure'
end
else
diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
index 83c20c4..49cd0ce 100644
--- a/src/app/models/quota.rb
+++ b/src/app/models/quota.rb
@@ -49,10 +49,12 @@ class Quota < ActiveRecord::Base
def self.can_create_instance?(instance, cloud_account)
[instance.owner, instance.pool, cloud_account].each do |parent|
- quota = Quota.find(parent.quota_id)
- potential_total_instances = quota.total_instances + 1
- if !Quota.no_limit(quota.maximum_total_instances) && (quota.maximum_total_instances < potential_total_instances)
- return false
+ if parent
+ quota = Quota.find(parent.quota_id)
+ potential_total_instances = quota.total_instances + 1
+ if !Quota.no_limit(quota.maximum_total_instances) && (quota.maximum_total_instances < potential_total_instances)
+ return false
+ end
end
end
return true
@@ -60,10 +62,12 @@ class Quota < ActiveRecord::Base
def self.can_start_instance?(instance, cloud_account)
[instance.owner, instance.pool, cloud_account].each do |parent|
- quota = Quota.find(parent.quota_id)
- potential_running_instances = quota.running_instances + 1
- if !Quota.no_limit(quota.maximum_running_instances) && quota.maximum_running_instances < potential_running_instances
- return false
+ if parent
+ quota = Quota.find(parent.quota_id)
+ potential_running_instances = quota.running_instances + 1
+ if !Quota.no_limit(quota.maximum_running_instances) && quota.maximum_running_instances < potential_running_instances
+ return false
+ end
end
end
return true
--
1.7.2.3
13 years, 6 months
[PATCH aggregator] BZ #645370 Filter HWPs based on Template Arch
by Scott Seago
On the 'new instance' form, the template is chosen on the first step. On the second page, filter out any hardware profiles that don't match the architecture of the chosen template.
Signed-off-by: Scott Seago <sseago(a)redhat.com>
---
src/app/controllers/instance_controller.rb | 9 +++++++++
src/app/views/instance/configure.haml | 2 +-
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/src/app/controllers/instance_controller.rb b/src/app/controllers/instance_controller.rb
index ad90404..a62a32e 100644
--- a/src/app/controllers/instance_controller.rb
+++ b/src/app/controllers/instance_controller.rb
@@ -82,6 +82,15 @@ class InstanceController < ApplicationController
def configure
@instance = Instance.new(params[:instance])
require_privilege(Privilege::INSTANCE_MODIFY, @instance.pool)
+ # FIXME: change template.architecture to match expected HWP arch strings
+ if (@instance.template.architecture == "64-bit")
+ arch_value = "x86_64"
+ else
+ arch_value = "i386"
+ end
+ @hardware_profiles = HardwareProfile.find(:all, :include => :architecture,
+ :conditions => {:provider_id => nil,
+ "hardware_profile_properties.value" => arch_value})
end
def create
diff --git a/src/app/views/instance/configure.haml b/src/app/views/instance/configure.haml
index 20929b4..809c044 100644
--- a/src/app/views/instance/configure.haml
+++ b/src/app/views/instance/configure.haml
@@ -14,7 +14,7 @@
= text_field_tag :pool_name, @instance.pool ? @instance.pool.name : '', :disabled => true
%li
= label :instance, :hardware_profile
- = select :instance, :hardware_profile_id, @instance.pool.hardware_profiles.map {|p| [ p.name, p.id ]}, { :include_blank => true }
+ = select :instance, :hardware_profile_id, @hardware_profiles.map {|p| [ p.name, p.id ]}, { :include_blank => false }
%li
= label :instance, :realm
= select :instance, :realm_id, @instance.pool.realms.map {|r| [ r.name, r.id ]}, { :include_blank => true }
--
1.7.2.3
13 years, 6 months
[PATCH aggregator] Remove rhel, as we are not building for that yet.
by Jason Guiditta
---
.../image_descriptor_platform_repositories.yml | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/src/config/image_descriptor_platform_repositories.yml b/src/config/image_descriptor_platform_repositories.yml
index 1e9704f..c29a244 100644
--- a/src/config/image_descriptor_platform_repositories.yml
+++ b/src/config/image_descriptor_platform_repositories.yml
@@ -2,7 +2,3 @@ fedora:
name: Fedora
version: 13
architecture: 64-bit
-rhel:
- name: RHEL
- version: 5
- architecture: 64-bit
--
1.7.2.3
13 years, 6 months
[PATCH] Use escapes when submitting some user supplied strings.
by Ian Main
This patch uses escapes when submitting some user supplied values to
condor. This combined with the escapes/whitespace patch to condor
allows whitespace in the instance name and account name to work.
Signed-off-by: Ian Main <imain(a)redhat.com>
---
src/app/util/condormatic.rb | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/app/util/condormatic.rb b/src/app/util/condormatic.rb
index ea18308..74c5a80 100644
--- a/src/app/util/condormatic.rb
+++ b/src/app/util/condormatic.rb
@@ -20,6 +20,11 @@
require 'nokogiri'
require 'socket'
+def escape(str)
+ str = str.gsub('\\', '\\\\')
+ str = str.gsub(' ', '\\ ')
+end
+
def condormatic_instance_create(task)
begin
@@ -39,7 +44,7 @@ def condormatic_instance_create(task)
pipe.puts "executable = #{job_name}\n"
Rails.logger.info "executable = #{job_name}\n"
- resource = "grid_resource = dcloud $$(provider_url) $$(username) $$(password) $$(image_key) #{instance.name}"
+ resource = "grid_resource = dcloud $$(provider_url) $$(username) $$(password) $$(image_key) #{escape(instance.name)}"
if realm != nil
resource += " $$(realm_key)"
else
@@ -226,7 +231,7 @@ def condormatic_classads_sync
pipe.puts "username=\"#{account.username}\""
pipe.puts "password=\"#{account.password}\""
pipe.puts "cloud_account_id=\"#{account.id}\""
- pipe.puts "keypair=\"#{account.instance_key.name}\""
+ pipe.puts "keypair=\"#{escape(account.instance_key.name)}\""
pipe.close_write
out = pipe.read
--
1.7.2.3
13 years, 6 months
Re: [deltacloud-devel] [PATCH aggregator] Added check for Quota before creating instance
by Martyn Taylor
I've not managed to do a proper full end to end test, since (on my setup anyway) when using the Mock Driver and Mock Target for building images, when I start an instance, the instance never goes into the state "Running", this means the Quotas are not updated, in addition Image Builder fails to build for EC2 on my box, every time.
I have tested this on users when Quotas are set to 0 and it works fine.
Please could someone with full end to end using EC2 or a working Mock setup, please try this out :)
Since the Quota checking is now done in the CloudEngine, we should be able to write a Cuke test. However, I'm not sure if starting instances, using Cuke is possible? Since we have an external dependency on Image Builder, unless its possible to Mock all the objects we need???
I am on PTO for the rest of this week, but I will be checking my mail, so feel free to drop me a line
Thanks
Martyn
----- Original Message -----
From: mtaylor(a)redhat.com
To: deltacloud-devel(a)lists.fedorahosted.org
Cc: "martyntaylor" <mtaylor(a)redhat.com>
Sent: Thursday, October 28, 2010 10:34:10 AM GMT +00:00 GMT Britain, Ireland, Portugal
Subject: [PATCH aggregator] Added check for Quota before creating instance
From: martyntaylor <mtaylor(a)redhat.com>
---
src/app/controllers/instance_controller.rb | 24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/src/app/controllers/instance_controller.rb b/src/app/controllers/instance_controller.rb
index ad90404..a81af6a 100644
--- a/src/app/controllers/instance_controller.rb
+++ b/src/app/controllers/instance_controller.rb
@@ -98,16 +98,22 @@ class InstanceController < ApplicationController
Pool.find((a)instance.pool_id))
#FIXME: This should probably be in a transaction
if @instance.save!
-
- @task = InstanceTask.new({:user => current_user,
- :task_target => @instance,
- :action => InstanceTask::ACTION_CREATE})
- if @task.save
- condormatic_instance_create(@task)
- flash[:notice] = "Instance added."
- redirect_to :action => 'index'
+ if Quota.can_start_instance?(@instance, nil)
+ @task = InstanceTask.new({:user => current_user,
+ :task_target => @instance,
+ :action => InstanceTask::ACTION_CREATE})
+ if @task.save
+ condormatic_instance_create(@task)
+ flash[:notice] = "Instance added."
+ redirect_to :action => 'index'
+ else
+ @pool = @instance.pool
+ render :action => 'configure'
+ end
else
- @pool = @instance.pool
+ @instance.destroy
+ flash[:notice] = "Quota Exceeded: Could not create instance"
+ @hardware_profiles = HardwareProfile.all
render :action => 'configure'
end
else
--
1.7.2.3
13 years, 6 months
