[PATCH aeolus-conductor] BZ#790434 Check unique assembly names on deployable xml
by Martyn Taylor
From: Martyn Taylor <mtaylor(a)redhat.com>
---
src/app/models/deployable.rb | 4 +++-
src/app/util/deployable_xml.rb | 4 ++++
src/config/locales/cs.yml | 1 +
src/config/locales/en.yml | 1 +
src/spec/factories/deployable.rb | 15 +++++++++++++++
src/spec/models/deployable_spec.rb | 5 +++++
6 files changed, 29 insertions(+), 1 deletions(-)
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb
index c9df4c5..3c8bdbd 100644
--- a/src/app/models/deployable.rb
+++ b/src/app/models/deployable.rb
@@ -48,8 +48,10 @@ class Deployable < ActiveRecord::Base
def valid_deployable_xml?
begin
deployable_xml = DeployableXML.new(xml)
- unless deployable_xml.validate!
+ if !deployable_xml.validate!
errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid'))
+ elsif !deployable_xml.unique_assembly_names?
+ errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid_duplicate_assembly_names'))
end
rescue Nokogiri::XML::SyntaxError => e
errors.add(:base, I18n.t("deployments.errors.not_valid_deployable_xml", :msg => "#{e.message}"))
diff --git a/src/app/util/deployable_xml.rb b/src/app/util/deployable_xml.rb
index 3032b5e..272476d 100644
--- a/src/app/util/deployable_xml.rb
+++ b/src/app/util/deployable_xml.rb
@@ -212,6 +212,10 @@ class DeployableXML
errors.empty?
end
+ def unique_assembly_names?
+ @root.xpath("/deployable/assemblies/assembly/@name").collect { |e| e.value }.uniq!.nil?
+ end
+
def assemblies
@assemblies ||=
@root.xpath('/deployable/assemblies/assembly').collect do |assembly_node|
diff --git a/src/config/locales/cs.yml b/src/config/locales/cs.yml
index 7ada0cc..e004fa1 100644
--- a/src/config/locales/cs.yml
+++ b/src/config/locales/cs.yml
@@ -216,6 +216,7 @@ cs:
warning:
not_valid_or_reachable: mcdmhig jiama afc nfdi ef jhifen bagekcj di dd bjokkm kefddkmcb dn
not_valid: cdjodmkfmo cmg jmjkbee namcgnb foblo kio hkgk
+ not_valid_duplicate_assembly_names: dsadf asdfsad oi ppoi jflf opifas
properties:
properties_for: nmiknfjdii hdk
permissions:
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index fbacbc0..6de7502 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -662,6 +662,7 @@ en:
not_valid_or_reachable: "Deployable XML file is either invalid or no longer reachable at %{url}"
no_url_provided: "No URL provided for the deployable XML file."
not_valid: "Deployable XML file doesn't resolve valid XML"
+ not_valid_duplicate_assembly_names: "Deployable XML must contain unique assembly names"
failed: Deployable was not created.
notice:
added: Deployable added to catalog %{catalog}.
diff --git a/src/spec/factories/deployable.rb b/src/spec/factories/deployable.rb
index d5afa1e..efbfc6e 100644
--- a/src/spec/factories/deployable.rb
+++ b/src/spec/factories/deployable.rb
@@ -34,6 +34,21 @@ FactoryGirl.define do
end
+ factory :deployable_unique_name_violation, :parent => :deployable do
+ xml "<?xml version=\"1.0\"?>
+ <deployable version='1.0' name='my'>
+ <description>This is my testing image</description>
+ <assemblies>
+ <assembly name='frontend' hwp='front_hwp1'>
+ <image id='53d2a281-448b-4872-b1b0-680edaad5922' build='63838705-8608-44c6-aded-7c243137172c'></image>
+ </assembly>
+ <assembly name='frontend' hwp='front_hwp2'>
+ <image id='53d2a281-448b-4872-b1b0-680edaad5922' build='63838705-8608-44c6-aded-7c243137172c'></image>
+ </assembly>
+ </assemblies>
+ </deployable>"
+ end
+
factory :deployable_with_parameters, :parent => :deployable do
xml "<?xml version=\"1.0\"?>
<deployable version='1.0' name='deployable_with_launch_parameters'>
diff --git a/src/spec/models/deployable_spec.rb b/src/spec/models/deployable_spec.rb
index 7e91ae2..af3f4b0 100644
--- a/src/spec/models/deployable_spec.rb
+++ b/src/spec/models/deployable_spec.rb
@@ -38,4 +38,9 @@ describe Deployable do
deployable.should_not be_valid
end
+ it "should not be valid if xml has multiple assemblies with the same name" do
+ deployable = FactoryGirl.build(:deployable_unique_name_violation)
+ deployable.valid_deployable_xml?
+ deployable.errors[:xml].should == ["Deployable XML must contain unique assembly names"]
+ end
end
--
1.7.6.4
12 years, 2 months
[PATCH 0/1]
by Jason Guiditta
This is the same patch, just resending because it didnt get to list yet.
-j
12 years, 2 months
[PATCH conductor 0/3] Two related BZs -- 795794, 795891
by Matt Wagner
Hi all,
This is a trio of inter-related patches that do a handful of different things. They all started from https://bugzilla.redhat.com/show_bug.cgi?id=795794 which led me to conclude that there were two separate bugs.
tl;dr -- The patches for different BZs are technically separate, but are related. The "BZ #795794 - Don't delete :vanished instances" patch could be omitted if required, though obviously I suggest using it.
== BZ #795794 - Display a saner error message if launch fails ==
This fixes the simplest possible form of the bug -- the error message generated when you attempt to launch a deployment for which the image has been deleted is a huge blob of nonsense. You get one line which tells you the actual error -- the assembly could not be launched because the image is missing. Then there is a newline and the massive blob of what almost appears to be the request object itself. It includes things like your AWS account ID. So I simply chop off anything after the first line.
== BZ #795891 - Deployment should not show "running" with no instances ==
This fixes another bug that relates to the mess that is #795794. If an instances goes into "vanished" state and then gets destroyed, we would show the deployment as running, even though it had no instances. This was a terrible mess, but turned out to have an easy fix -- "instances.all?" with a block will always return true if instances is an empty array. So I just checked that the instance existed first. (The "instances.empty?" conditional I added to the next block doesn't actually change any behavior -- the existing code would have implicitly matched that case anyway. But it makes it more obvious that we have considered that case.)
I added the Instance::FAILED_STATES array after realizing that I was going to introduce a little bit of duplication.
I considered that we might want to delete the whole deployment if we deleted the last instance because it had gone messing, but didn't like this idea, which leads to my third and final patch:
== BZ #795794 - Don't delete :vanished instances ==
This fixes something I argued for, but now think was a bad idea. I had proposed that we should delete instances when they disappeared on the backend provider as an interim solution until next sprint, when we could identify how to elegantly style them to indicate an error. However, this turned into a huge mess. For one, it's not safe -- if an instances temporarily stops being advertised by a provider for some reason, we will trash it and never add it back. The user would have no idea that the instance was still running and costing them money. Second, when we delete the instance, the associated event that we created gets destroyed. Ultimately, what pushed me over the edge was that I wanted to also delete the deployment to avoid leaving dangling garbage in the app, but this just felt like a bad idea, and it made me realize that the whole thing was a terrible idea on my part. With this patch, when an instance vanishes from the cloud provider, we set its state to "Vanished" and f!
ire an event. If it comes back, dbomatic will recover gracefully. If not, it will just stay in vanished state. This is stretching the BZ a little bit, but I think it's ultimately the best solution.
Best,
Matt
12 years, 2 months
[PATCH conductor] BZ796098 Fix UI inconsistencies
by Jirka Tomasek
From: Jiri Tomasek <jtomasek(a)redhat.com>
Middle align section-controls content
Image detail - separate properties section
Provider Account detail - edit/test_connection buttons in header next to return_to link
---
src/app/stylesheets/layout.scss | 1 +
src/app/views/images/show.html.haml | 10 +++++++++-
src/app/views/provider_accounts/show.html.haml | 7 +++----
src/config/locales/en.yml | 2 +-
4 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/app/stylesheets/layout.scss b/src/app/stylesheets/layout.scss
index 6ad0810..1b4b33b 100644
--- a/src/app/stylesheets/layout.scss
+++ b/src/app/stylesheets/layout.scss
@@ -1720,6 +1720,7 @@ section.admin-content-section{
right: 0px;
bottom: 4px;
@include display_inline_block;
+ vertical-align: middle;
font-size: 11px;
a.collapse{
diff --git a/src/app/views/images/show.html.haml b/src/app/views/images/show.html.haml
index 82c00f5..9e941a6 100644
--- a/src/app/views/images/show.html.haml
+++ b/src/app/views/images/show.html.haml
@@ -16,7 +16,15 @@
%section.admin-content-section
%header
%h2=t'properties'
- %h3= t('images.environment', :environment => @image.environment)
+ .content
+ %table
+ %tbody
+ %tr
+ %td= t('images.environment')
+ %td= @image.environment
+
+%section.admin-content-section
+ %header
%h2= t('.provider_images')
.section-controls
- if @builds.any?
diff --git a/src/app/views/provider_accounts/show.html.haml b/src/app/views/provider_accounts/show.html.haml
index b9a3597..63dcc72 100644
--- a/src/app/views/provider_accounts/show.html.haml
+++ b/src/app/views/provider_accounts/show.html.haml
@@ -5,14 +5,13 @@
.return_to
=t'return_to'
= link_to t('provider_accounts.new.cloud_providers'), edit_provider_path(@provider, :details_tab => 'accounts')
+ .button-group
+ = link_to t('edit'), edit_provider_provider_account_path(@provider, @provider_account), :class => "button pill", :title => "Edit Provider Account"
+ = link_to t('provider_accounts.show.test_connection'), provider_provider_account_path(@provider, @provider_account, :test_account => true), :class => 'button pill'
%section.admin-content-section
%header
%h2=t'properties'
- .section-controls
- #obj_actions.button-group
- = link_to t('edit'), edit_provider_provider_account_path(@provider, @provider_account), :class => "button pill", :title => "Edit Provider Account"
- = link_to t('provider_accounts.show.test_connection'), provider_provider_account_path(@provider, @provider_account, :test_account => true), :class => 'button pill'
.content
= render :partial => 'properties'
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index abf308a..d0aebe1 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -724,7 +724,7 @@ en:
delete_failed: Unable to Delete Target Image
not_found: Target Image not found
images:
- environment: "%{environment} Environment"
+ environment: "Environment:"
environment_header: Environment
import:
import_image: Import Image
--
1.7.7.6
12 years, 2 months
[PATCH configure] BZ 794755 - Static assets don't set Cache-Control headers https://bugzilla.redhat.com/show_bug.cgi?id=794755
by John Eckersberg
Use mod_expires to set expiration for css/js/png to one year.
>From the BZ:
AssetTagHelper appends timestamps as a query parameter, so we can set a
Cache-Control directive allowing these assets to be cached forever -- if we
change a stylesheet, it will end up with a new timestamp in the URL and get
reloaded automatically.
---
recipes/aeolus/files/aggregator-httpd-ssl.conf | 5 +++++
recipes/aeolus/files/aggregator-httpd.conf | 5 +++++
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/recipes/aeolus/files/aggregator-httpd-ssl.conf b/recipes/aeolus/files/aggregator-httpd-ssl.conf
index fce2f0d..9ace5de 100644
--- a/recipes/aeolus/files/aggregator-httpd-ssl.conf
+++ b/recipes/aeolus/files/aggregator-httpd-ssl.conf
@@ -24,6 +24,11 @@ Alias /fonts "/usr/share/aeolus-conductor/public/fonts"
RewriteRule ^/conductor/images/(.*).(png|jpg|gif|svg)$ /conductor/graphics/$1.$2 [R]
+ExpiresActive on
+ExpiresByType text/css "access plus 1 year"
+ExpiresByType text/javascript "access plus 1 year"
+ExpiresByType image/png "access plus 1 year"
+
ProxyPass /conductor/graphics !
ProxyPass /conductor/stylesheets !
ProxyPass /conductor/errors !
diff --git a/recipes/aeolus/files/aggregator-httpd.conf b/recipes/aeolus/files/aggregator-httpd.conf
index 567d648..8a0331e 100644
--- a/recipes/aeolus/files/aggregator-httpd.conf
+++ b/recipes/aeolus/files/aggregator-httpd.conf
@@ -19,6 +19,11 @@ Alias /fonts "/usr/share/aeolus-conductor/public/fonts"
RewriteRule ^/conductor/images/(.*).(png|jpg|gif|svg)$ /conductor/graphics/$1.$2 [R]
+ExpiresActive on
+ExpiresByType text/css "access plus 1 year"
+ExpiresByType text/javascript "access plus 1 year"
+ExpiresByType image/png "access plus 1 year"
+
ProxyPass /conductor/graphics !
ProxyPass /conductor/stylesheets !
ProxyPass /conductor/errors !
--
1.7.7.6
12 years, 2 months
[PATCH conductor] Added check if there is provider account when adding provider account to poolfamily.
by Tomas Hrcka
https://bugzilla.redhat.com/show_bug.cgi?id=795659
---
src/app/controllers/pool_families_controller.rb | 14 ++++++++++----
src/config/locales/en.yml | 1 +
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/app/controllers/pool_families_controller.rb b/src/app/controllers/pool_families_controller.rb
index 7e33977..c4c0da8 100644
--- a/src/app/controllers/pool_families_controller.rb
+++ b/src/app/controllers/pool_families_controller.rb
@@ -106,10 +106,16 @@ class PoolFamiliesController < ApplicationController
def add_provider_accounts
@pool_family = PoolFamily.find(params[:id])
require_privilege(Privilege::MODIFY, @pool_family)
- @provider_accounts = ProviderAccount.
- list_for_user(current_user, Privilege::USE).
- where('id not in (?)', @pool_family.provider_accounts.empty? ?
- 0 : @pool_family.provider_accounts.map(&:id))
+
+ if ProviderAccount.count == 0
+ flash[:error] = "#{t('pool_families.flash.error.no_provider_accounts')}"
+ redirect_to pool_family_path(@pool_family, :details_tab => 'provider_accounts') and return
+ else
+ @provider_accounts = ProviderAccount.
+ list_for_user(current_user, Privilege::USE).
+ where('id not in (?)', @pool_family.provider_accounts.empty? ?
+ 0 : @pool_family.provider_accounts.map(&:id))
+ end
load_tab_captions_and_details_tab('provider_accounts')
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index addb68e..6640d69 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -636,6 +636,7 @@ en:
provider_accounts_not_removed: "Could not remove these Provider Accounts"
select_to_add_accounts: "You must select at least one Provider Account to add."
select_to_remove_accounts: "You must select at least one Provider Account to remove."
+ no_provider_accounts: "There are no provider accounts in system."
catalog_entries:
new_catalog_entry: New Deployable
index:
--
1.7.1
12 years, 2 months
[PATCH conductor] BZ790822 administer section form items alignment fix
by Jirka Tomasek
From: Jiri Tomasek <jtomasek(a)redhat.com>
The checkboxes are now aligned properly, in IE it seems that the left margin is ignored, so there isn't much to do about it.
Also the file inputs vary in height in different browsers, so the label alignment is little off in ie. Firefox and Chrome are okay.
---
src/app/stylesheets/layout.scss | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/src/app/stylesheets/layout.scss b/src/app/stylesheets/layout.scss
index b067af2..57e55b1 100644
--- a/src/app/stylesheets/layout.scss
+++ b/src/app/stylesheets/layout.scss
@@ -3441,10 +3441,6 @@ body.administer form.generic{
font-size: 12px;
text-shadow: 0px 1px 0px #ffffff;
font-weight: normal;
-
- &.checkbox{
- padding: 3px 7px 0px 0px;
- }
}
label span{
@@ -3461,6 +3457,7 @@ body.administer form.generic{
-moz-box-shadow:inset 0 1px 2px rgba(0, 0, 0, 0.50);
-webkit-box-shadow:inset 0 1px 2px rgba(0, 0, 0, 0.50);
}
+ input[type="checkbox"]{ margin: 6px 0px 5px 0px; }
input.long{
width: 400px;
--
1.7.7.6
12 years, 2 months
[PATCH conductor] BZ#795921 Unsupported provider types are not listed in UI
by Tomas Hrcka
https://bugzilla.redhat.com/show_bug.cgi?id=795921
---
src/app/controllers/providers_controller.rb | 6 ++++++
src/app/views/providers/_form.html.haml | 2 +-
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/app/controllers/providers_controller.rb b/src/app/controllers/providers_controller.rb
index 340955d..dfbe7c2 100644
--- a/src/app/controllers/providers_controller.rb
+++ b/src/app/controllers/providers_controller.rb
@@ -17,6 +17,7 @@
class ProvidersController < ApplicationController
before_filter :require_user
before_filter :load_providers, :only => [:index, :show, :new, :edit, :create, :update]
+ before_filter :load_providers_types, :only => [:new, :edit]
def index
@params = params
@@ -46,6 +47,7 @@ class ProvidersController < ApplicationController
@provider = Provider.new
@provider.url = Provider::DEFAULT_DELTACLOUD_URL
@title = t("providers.new.new_provider")
+
end
def edit
@@ -235,6 +237,10 @@ class ProvidersController < ApplicationController
return alerts
end
+ def load_providers_types
+ @provider_types = ProviderType.where(:name => ["Mock","Amazon EC2","RHEV-M","VMware vSphere"]).collect {|provider_type| [provider_type.name,provider_type.id]}
+ end
+
def load_provider_tabs
@realms = @provider.realms.apply_filters(:preset_filter_id => params[:provider_realms_preset_filter], :search_filter => params[:provider_realms_search])
#TODO add links to real data for history,properties,permissions
diff --git a/src/app/views/providers/_form.html.haml b/src/app/views/providers/_form.html.haml
index fe8f17f..e66f64a 100644
--- a/src/app/views/providers/_form.html.haml
+++ b/src/app/views/providers/_form.html.haml
@@ -9,7 +9,7 @@
= form.text_field :url, :title => t('providers.form.provider_url'), :value => @provider.url, :class => 'long'
%p
= form.label :provider_type, t('providers.form.provider_type')
- = form.select(:provider_type_id, ProviderType.all.collect {|provider_type| [provider_type.name,provider_type.id] }, :prompt => t('providers.form.select_type_of_provider'))
+ = form.select(:provider_type_id, @provider_types, :prompt => t('providers.form.select_type_of_provider'))
%p
= form.label :url, t('providers.form.x_deltacloud_provider')
= form.text_field :deltacloud_provider, :title => t('providers.form.x_deltacloud_provider'), :value => @provider.deltacloud_provider, :class => 'long'
--
1.7.1
12 years, 2 months
