This is another resend of this patch after a couple more updates from running tests
This patch throws errors to the user when they have used features in the deployable that require audrey agent 0.5.0 but the 0.4.x agent is installed in the image the assembly uses.
--- src/app/models/deployable.rb | 38 ++++++++++++++++++++++++++++++++++++++ src/config/locales/en.yml | 1 + 2 files changed, 39 insertions(+)
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb index 7659688..68e8c4b 100644 --- a/src/app/models/deployable.rb +++ b/src/app/models/deployable.rb @@ -192,6 +192,8 @@ class Deployable < ActiveRecord::Base deployable_errors << "#{assembly_hash[:name]}: " + I18n.t('deployables.error.attribute_not_exist') end assemblies_array << assembly_hash + audrey_error = check_audrey_api_compatibility(image, assembly) + deployable_errors << "#{assembly_hash[:name]}: " + audrey_error if not audrey_error.nil? end [assemblies_array, images, missing_images, deployable_errors] end @@ -245,6 +247,42 @@ class Deployable < ActiveRecord::Base warnings end
+ def check_audrey_api_compatibility(image, assembly) + # get icicle for agent + icicle_uuid = image.latest_pushed_or_unpushed_build.target_images.first.icicle rescue nil + icicle = Aeolus::Image::Warehouse::Icicle.find(icicle_uuid) if icicle_uuid + agent_v = icicle ? icicle.packages.find_all { |p| p =~ /aeolus-audrey-agent(.*)/ } : "" + agent_v = agent_v.first.split('-')[3] if agent_v.present? + + # calculate audrey api version + audrey_api_v = if agent_v >= "0.5.0" + 1..2 + elsif agent_v >= "0.4.0" + 1..1 + else 0 + end + + # initalize compatibility + audrey_api_compat = 1 + + # do cs_compat + ## All agents are compatible with all Config Servers right now + ## so no need to check this right now + ## this check should call the cs passing the agent_v and validating + ## the response the CS sends back + + # audrey api version 2 added service references, lets check for any + assembly.services.each do |service| + service.parameters.each do |param| + audrey_api_compat = 2 if param.reference_service + end + end + + if audrey_api_v != 0 + audrey_api_v.include?(audrey_api_compat) ? nil : I18n.t('deployables.error.audrey_api_incompatibility') + end + end + private
def self.apply_search_filter(search) diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml index 1a1c69e..6900522 100644 --- a/src/config/locales/en.yml +++ b/src/config/locales/en.yml @@ -1008,6 +1008,7 @@ en: error: attribute_not_exists: invalid hwp_not_exists: "Hardware profile %{name} specified in XML doesn't exist." + audrey_api_incompatibility: "The deployable uses features not supported by the audrey agent installed in the latest built image." properties: edit_deployable_xml: Edit Deployable XML name: Name
On Fri, Jun 08, 2012 at 03:17:17PM -0400, Dan Radez wrote:
src/app/models/deployable.rb | 38 ++++++++++++++++++++++++++++++++++++++ src/config/locales/en.yml | 1 + 2 files changed, 39 insertions(+)
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb index 7659688..68e8c4b 100644 --- a/src/app/models/deployable.rb +++ b/src/app/models/deployable.rb @@ -192,6 +192,8 @@ class Deployable < ActiveRecord::Base deployable_errors << "#{assembly_hash[:name]}: " + I18n.t('deployables.error.attribute_not_exist') end assemblies_array << assembly_hash
audrey_error = check_audrey_api_compatibility(image, assembly)
end [assemblies_array, images, missing_images, deployable_errors] enddeployable_errors << "#{assembly_hash[:name]}: " + audrey_error if not audrey_error.nil?
@@ -245,6 +247,42 @@ class Deployable < ActiveRecord::Base warnings end
def check_audrey_api_compatibility(image, assembly)
# get icicle for agent
icicle_uuid = image.latest_pushed_or_unpushed_build.target_images.first.icicle rescue nil
icicle = Aeolus::Image::Warehouse::Icicle.find(icicle_uuid) if icicle_uuid
agent_v = icicle ? icicle.packages.find_all { |p| p =~ /aeolus-audrey-agent(.*)/ } : ""
agent_v = agent_v.first.split('-')[3] if agent_v.present?
# calculate audrey api version
audrey_api_v = if agent_v >= "0.5.0"
1..2
elsif agent_v >= "0.4.0"
1..1
else 0
end
# initalize compatibility
audrey_api_compat = 1
# do cs_compat
## All agents are compatible with all Config Servers right now
## so no need to check this right now
## this check should call the cs passing the agent_v and validating
## the response the CS sends back
# audrey api version 2 added service references, lets check for any
assembly.services.each do |service|
service.parameters.each do |param|
audrey_api_compat = 2 if param.reference_service
end
end
if audrey_api_v != 0
audrey_api_v.include?(audrey_api_compat) ? nil : I18n.t('deployables.error.audrey_api_incompatibility')
end
end
private
def self.apply_search_filter(search)
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml index 1a1c69e..6900522 100644 --- a/src/config/locales/en.yml +++ b/src/config/locales/en.yml @@ -1008,6 +1008,7 @@ en: error: attribute_not_exists: invalid hwp_not_exists: "Hardware profile %{name} specified in XML doesn't exist."
properties: edit_deployable_xml: Edit Deployable XML name: Nameaudrey_api_incompatibility: "The deployable uses features not supported by the audrey agent installed in the latest built image."
-- 1.7.10.2
ACK! This fixes the issues I had seen in the earlier set, maintains test compatibility, and looks good.
-- Matt
On 06/08/2012 09:19 PM, Matt Wagner wrote:
On Fri, Jun 08, 2012 at 03:17:17PM -0400, Dan Radez wrote:
src/app/models/deployable.rb | 38 ++++++++++++++++++++++++++++++++++++++ src/config/locales/en.yml | 1 + 2 files changed, 39 insertions(+)
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb index 7659688..68e8c4b 100644 --- a/src/app/models/deployable.rb +++ b/src/app/models/deployable.rb @@ -192,6 +192,8 @@ class Deployable< ActiveRecord::Base deployable_errors<< "#{assembly_hash[:name]}: " + I18n.t('deployables.error.attribute_not_exist') end assemblies_array<< assembly_hash
audrey_error = check_audrey_api_compatibility(image, assembly)
enddeployable_errors<< "#{assembly_hash[:name]}: " + audrey_error if not audrey_error.nil? end [assemblies_array, images, missing_images, deployable_errors]
@@ -245,6 +247,42 @@ class Deployable< ActiveRecord::Base warnings end
- def check_audrey_api_compatibility(image, assembly)
- # get icicle for agent
- icicle_uuid = image.latest_pushed_or_unpushed_build.target_images.first.icicle rescue nil
- icicle = Aeolus::Image::Warehouse::Icicle.find(icicle_uuid) if icicle_uuid
- agent_v = icicle ? icicle.packages.find_all { |p| p =~ /aeolus-audrey-agent(.*)/ } : ""
- agent_v = agent_v.first.split('-')[3] if agent_v.present?
- # calculate audrey api version
- audrey_api_v = if agent_v>= "0.5.0"
agent_v can be empty array, then this raises an exception: undefined method `>=' for []:Array
1..2
elsif agent_v>= "0.4.0"
1..1
else 0
end
# initalize compatibility
audrey_api_compat = 1
# do cs_compat
## All agents are compatible with all Config Servers right now
## so no need to check this right now
## this check should call the cs passing the agent_v and validating
## the response the CS sends back
# audrey api version 2 added service references, lets check for any
assembly.services.each do |service|
service.parameters.each do |param|
audrey_api_compat = 2 if param.reference_service
end
end
if audrey_api_v != 0
audrey_api_v.include?(audrey_api_compat) ? nil : I18n.t('deployables.error.audrey_api_incompatibility')
end
end
private
def self.apply_search_filter(search)
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml index 1a1c69e..6900522 100644 --- a/src/config/locales/en.yml +++ b/src/config/locales/en.yml @@ -1008,6 +1008,7 @@ en: error: attribute_not_exists: invalid hwp_not_exists: "Hardware profile %{name} specified in XML doesn't exist."
name: Nameaudrey_api_incompatibility: "The deployable uses features not supported by the audrey agent installed in the latest built image." properties: edit_deployable_xml: Edit Deployable XML
-- 1.7.10.2
ACK! This fixes the issues I had seen in the earlier set, maintains test compatibility, and looks good.
-- Matt
Hi, I'm hitting an error with this patch (described inline).
Jan
On 06/11/2012 10:18 AM, Jan Provaznik wrote:
On 06/08/2012 09:19 PM, Matt Wagner wrote:
On Fri, Jun 08, 2012 at 03:17:17PM -0400, Dan Radez wrote:
src/app/models/deployable.rb | 38 ++++++++++++++++++++++++++++++++++++++ src/config/locales/en.yml | 1 + 2 files changed, 39 insertions(+)
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb index 7659688..68e8c4b 100644 --- a/src/app/models/deployable.rb +++ b/src/app/models/deployable.rb @@ -192,6 +192,8 @@ class Deployable< ActiveRecord::Base deployable_errors<< "#{assembly_hash[:name]}: " + I18n.t('deployables.error.attribute_not_exist') end assemblies_array<< assembly_hash
audrey_error = check_audrey_api_compatibility(image, assembly)
deployable_errors<< "#{assembly_hash[:name]}: " +
audrey_error if not audrey_error.nil? end [assemblies_array, images, missing_images, deployable_errors] end @@ -245,6 +247,42 @@ class Deployable< ActiveRecord::Base warnings end
- def check_audrey_api_compatibility(image, assembly)
- # get icicle for agent
- icicle_uuid =
image.latest_pushed_or_unpushed_build.target_images.first.icicle rescue nil
- icicle = Aeolus::Image::Warehouse::Icicle.find(icicle_uuid) if
icicle_uuid
- agent_v = icicle ? icicle.packages.find_all { |p| p =~
/aeolus-audrey-agent(.*)/ } : ""
- agent_v = agent_v.first.split('-')[3] if agent_v.present?
- # calculate audrey api version
- audrey_api_v = if agent_v>= "0.5.0"
agent_v can be empty array, then this raises an exception: undefined method `>=' for []:Array
I think I'm expecting audrey_v to be a string. probably should rewrite agent_v = agent_v.first.split('-')[3] if agent_v.present?
to
agent_v = agent_v.present? ? agent_v.first.split('-')[3] : ""
Thoughts about that? I'll have a patch ready to go.
Dan
1..2
elsif agent_v>= "0.4.0"
1..1
else 0
end
- # initalize compatibility
- audrey_api_compat = 1
- # do cs_compat
- ## All agents are compatible with all Config Servers right now
- ## so no need to check this right now
- ## this check should call the cs passing the agent_v and validating
- ## the response the CS sends back
- # audrey api version 2 added service references, lets check for any
- assembly.services.each do |service|
service.parameters.each do |param|
audrey_api_compat = 2 if param.reference_service
end
- end
- if audrey_api_v != 0
audrey_api_v.include?(audrey_api_compat) ? nil :
I18n.t('deployables.error.audrey_api_incompatibility')
end
end
private
def self.apply_search_filter(search)
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml index 1a1c69e..6900522 100644 --- a/src/config/locales/en.yml +++ b/src/config/locales/en.yml @@ -1008,6 +1008,7 @@ en: error: attribute_not_exists: invalid hwp_not_exists: "Hardware profile %{name} specified in XML doesn't exist."
audrey_api_incompatibility: "The deployable uses features not
supported by the audrey agent installed in the latest built image." properties: edit_deployable_xml: Edit Deployable XML name: Name -- 1.7.10.2
ACK! This fixes the issues I had seen in the earlier set, maintains test compatibility, and looks good.
-- Matt
Hi, I'm hitting an error with this patch (described inline).
Jan
On 06/11/2012 04:54 PM, Dan Radez wrote:
On 06/11/2012 10:18 AM, Jan Provaznik wrote:
On 06/08/2012 09:19 PM, Matt Wagner wrote:
On Fri, Jun 08, 2012 at 03:17:17PM -0400, Dan Radez wrote:
src/app/models/deployable.rb | 38 ++++++++++++++++++++++++++++++++++++++ src/config/locales/en.yml | 1 + 2 files changed, 39 insertions(+)
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb index 7659688..68e8c4b 100644 --- a/src/app/models/deployable.rb +++ b/src/app/models/deployable.rb @@ -192,6 +192,8 @@ class Deployable< ActiveRecord::Base deployable_errors<< "#{assembly_hash[:name]}: " + I18n.t('deployables.error.attribute_not_exist') end assemblies_array<< assembly_hash
audrey_error = check_audrey_api_compatibility(image, assembly)
deployable_errors<< "#{assembly_hash[:name]}: " +
audrey_error if not audrey_error.nil? end [assemblies_array, images, missing_images, deployable_errors] end @@ -245,6 +247,42 @@ class Deployable< ActiveRecord::Base warnings end
- def check_audrey_api_compatibility(image, assembly)
- # get icicle for agent
- icicle_uuid =
image.latest_pushed_or_unpushed_build.target_images.first.icicle rescue nil
- icicle = Aeolus::Image::Warehouse::Icicle.find(icicle_uuid) if
icicle_uuid
- agent_v = icicle ? icicle.packages.find_all { |p| p =~
/aeolus-audrey-agent(.*)/ } : ""
- agent_v = agent_v.first.split('-')[3] if agent_v.present?
- # calculate audrey api version
- audrey_api_v = if agent_v>= "0.5.0"
agent_v can be empty array, then this raises an exception: undefined method `>=' for []:Array
I think I'm expecting audrey_v to be a string. probably should rewrite agent_v = agent_v.first.split('-')[3] if agent_v.present?
to
agent_v = agent_v.present? ? agent_v.first.split('-')[3] : ""
Thoughts about that? I'll have a patch ready to go.
Dan
Maybe the code could be simplified by putting more logic into Icicle model (which is in aeolus-image-rubygem component):
diff --git a/lib/aeolus_image/model/warehouse/icicle.rb b/lib/aeolus_image/model/warehouse/icicle.rb index 1a11da8..0e33ee0 100644 --- a/lib/aeolus_image/model/warehouse/icicle.rb +++ b/lib/aeolus_image/model/warehouse/icicle.rb @@ -18,6 +18,11 @@ module Aeolus class Icicle < WarehouseModel @bucket_name = 'icicles'
+ def audrey_version + packages.each { return $1 if p =~ /aeolus-audrey-agent-(.*)/ } + nil + end + def packages unless @packages begin diff --git a/lib/aeolus_image/model/warehouse/target_image.rb b/lib/aeolus_image/model/warehouse/target_image.rb index 5e6b84c..f1cb9ec 100644 --- a/lib/aeolus_image/model/warehouse/target_image.rb +++ b/lib/aeolus_image/model/warehouse/target_image.rb @@ -22,6 +22,10 @@ module Aeolus ImageBuild.find(@build) if @build end
+ def icicle + Icicle.find(@icicle) if @icicle + end + def provider_images ProviderImage.where("($target_image == "" + @uuid.to_s + "")") end
then agent_v variable can be set by: - # get icicle for agent - icicle_uuid = image.latest_pushed_or_unpushed_build.target_images.first.icicle rescue nil - icicle = Aeolus::Image::Warehouse::Icicle.find(icicle_uuid) if icicle_uuid - agent_v = icicle ? icicle.packages.find_all { |p| p =~ /aeolus-audrey-agent(.*)/ } : "" - agent_v = agent_v.first.split('-')[3] if agent_v.present? + agent_v = image.latest_pushed_or_unpushed_build. + target_images.first.icicle.audrey_version.to_s rescue ''
(Thought I didn't test the code above)
Jan
On 06/12/2012 08:58 AM, Jan Provaznik wrote:
On 06/11/2012 04:54 PM, Dan Radez wrote:
On 06/11/2012 10:18 AM, Jan Provaznik wrote:
On 06/08/2012 09:19 PM, Matt Wagner wrote:
On Fri, Jun 08, 2012 at 03:17:17PM -0400, Dan Radez wrote:
src/app/models/deployable.rb | 38 ++++++++++++++++++++++++++++++++++++++ src/config/locales/en.yml | 1 + 2 files changed, 39 insertions(+)
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb index 7659688..68e8c4b 100644 --- a/src/app/models/deployable.rb +++ b/src/app/models/deployable.rb @@ -192,6 +192,8 @@ class Deployable< ActiveRecord::Base deployable_errors<< "#{assembly_hash[:name]}: " + I18n.t('deployables.error.attribute_not_exist') end assemblies_array<< assembly_hash
audrey_error = check_audrey_api_compatibility(image, assembly)
deployable_errors<< "#{assembly_hash[:name]}: " +
audrey_error if not audrey_error.nil? end [assemblies_array, images, missing_images, deployable_errors] end @@ -245,6 +247,42 @@ class Deployable< ActiveRecord::Base warnings end
- def check_audrey_api_compatibility(image, assembly)
- # get icicle for agent
- icicle_uuid =
image.latest_pushed_or_unpushed_build.target_images.first.icicle rescue nil
- icicle = Aeolus::Image::Warehouse::Icicle.find(icicle_uuid) if
icicle_uuid
- agent_v = icicle ? icicle.packages.find_all { |p| p =~
/aeolus-audrey-agent(.*)/ } : ""
- agent_v = agent_v.first.split('-')[3] if agent_v.present?
- # calculate audrey api version
- audrey_api_v = if agent_v>= "0.5.0"
agent_v can be empty array, then this raises an exception: undefined method `>=' for []:Array
I think I'm expecting audrey_v to be a string. probably should rewrite agent_v = agent_v.first.split('-')[3] if agent_v.present?
to
agent_v = agent_v.present? ? agent_v.first.split('-')[3] : ""
Thoughts about that? I'll have a patch ready to go.
Dan
Maybe the code could be simplified by putting more logic into Icicle model (which is in aeolus-image-rubygem component):
diff --git a/lib/aeolus_image/model/warehouse/icicle.rb b/lib/aeolus_image/model/warehouse/icicle.rb index 1a11da8..0e33ee0 100644 --- a/lib/aeolus_image/model/warehouse/icicle.rb +++ b/lib/aeolus_image/model/warehouse/icicle.rb @@ -18,6 +18,11 @@ module Aeolus class Icicle < WarehouseModel @bucket_name = 'icicles'
def audrey_version
packages.each { return $1 if p =~ /aeolus-audrey-agent-(.*)/ }
nil
end
def packages unless @packages begin
diff --git a/lib/aeolus_image/model/warehouse/target_image.rb b/lib/aeolus_image/model/warehouse/target_image.rb index 5e6b84c..f1cb9ec 100644 --- a/lib/aeolus_image/model/warehouse/target_image.rb +++ b/lib/aeolus_image/model/warehouse/target_image.rb @@ -22,6 +22,10 @@ module Aeolus ImageBuild.find(@build) if @build end
def icicle
Icicle.find(@icicle) if @icicle
end
def provider_images ProviderImage.where("($target_image == \"" + @uuid.to_s + "\")") end
then agent_v variable can be set by:
- # get icicle for agent
- icicle_uuid =
image.latest_pushed_or_unpushed_build.target_images.first.icicle rescue nil
- icicle = Aeolus::Image::Warehouse::Icicle.find(icicle_uuid) if
icicle_uuid
- agent_v = icicle ? icicle.packages.find_all { |p| p =~
/aeolus-audrey-agent(.*)/ } : ""
- agent_v = agent_v.first.split('-')[3] if agent_v.present?
- agent_v = image.latest_pushed_or_unpushed_build.
target_images.first.icicle.audrey_version.to_s rescue ''
(Thought I didn't test the code above)
Jan
I'm tracking this with https://bugzilla.redhat.com/show_bug.cgi?id=831201
aeolus-devel@lists.fedorahosted.org