[PATCH conductor] Add Build & Push to Deployable Details

Jan Provazník jprovazn at redhat.com
Thu Dec 1 12:52:40 UTC 2011


On 12/01/2011 12:27 PM, tsedovic at redhat.com wrote:
> From: Tomas Sedovic<tomas at sedovic.cz>
>
> https://www.aeolusproject.org/redmine/issues/2853
>
> It shows the following states: not built, building, not pushed, pushing,
> pushed. And it allows the user to rebuild and re-push all images.
>
> As things are now, building and pushing is a 2-step operation.
>
> Note that the page isn't styled properly yet.
> ---
>   src/app/controllers/deployables_controller.rb |   91 +++++++++++++++++++++++++
>   src/app/views/deployables/show.html.haml      |   12 ++--
>   src/config/locales/en.yml                     |    9 +++
>   src/config/routes.rb                          |    1 +
>   4 files changed, 106 insertions(+), 7 deletions(-)
>
> diff --git a/src/app/controllers/deployables_controller.rb b/src/app/controllers/deployables_controller.rb
> index c5e7a4e..a51b496 100644
> --- a/src/app/controllers/deployables_controller.rb
> +++ b/src/app/controllers/deployables_controller.rb
> @@ -63,6 +63,12 @@ class DeployablesController<  ApplicationController
>           flash[:error] = assembly[key] if key.to_s =~ /^error\w+/
>         end
>       end
> +    images = @catalog_entry.deployable.fetch_images

If an image doesn't exist in warehouse, fetch_images deosn't return the 
image (probably bug) -> then build_status returns "pushed" for all not 
existing images.

Not part of this patch, but fetch_image could be optimized to call 
fetch_image_uuids only once (to avoid multiple parsing of XML file).

For deployable it's also allowed to have specific "build_id" attribute 
for an image -> then this particular build_id should be used instead of 
latest build, now it's getting complicated: if we allow rebuild of such 
image, should be this build_id updated in xml, or are we going to ignore 
this attribute for deployable builds?

> +    builder = Aeolus::Image::Factory::Builder.first
> +    @build_results = []
> +    ProviderAccount.group_by_type(current_user).each do |driver, groups|
> +      @build_results<<  build_status(images, builder, driver, groups[:accounts].first)

^ hm, so we are going to support only one provider account per provider? 
I thought that now multiple provider accounts are supported.

> +    end
>     end
>
>     def create
> @@ -172,6 +178,23 @@ class DeployablesController<  ApplicationController
>       redirect_to original_path.merge(original_params).merge("catalog_entries_preset_filter" =>  params[:catalog_entries_preset_filter], "catalog_entries_search" =>  params[:catalog_entries_search])
>     end
>
> +  def build
> +    catalog = Catalog.find(params[:catalog_id])
> +    deployable = Deployable.find(params[:deployable_id])

^ catalog_entries and deploaybles views are messed after yesterday's 
renaming -> there is in fact catalog_entry id in params[:deployable_id], 
so redirect at the end of method doesn't work - redirects to wrong id. 
And if id is not found, we redirect on index page by default, where is 
is another bug on deployables index page (not related to this patch):
@catalog = @catalog_entries.first.catalog

This obviously doesn't work if there is no catalog_entry for a catalog, 
I guess there should be just @catalog = params[:catalog_id]

> +    require_privilege(Privilege::MODIFY, catalog)
> +    require_privilege(Privilege::MODIFY, deployable)
> +
> +    images = deployable.fetch_images
> +    options = params[:build_options].to_sym
> +    case options
> +    when :rebuild_all
> +      build_all(images)
> +    when :push_all
> +      push_all(images)
> +    end
> +    redirect_to catalog_deployable_path(catalog, deployable)
> +  end
> +
>     private
>
>     def set_header
> @@ -198,4 +221,72 @@ class DeployablesController<  ApplicationController
>         nil
>       end
>     end

Why not to move methods bellow to Deploayble model?

> +
> +  def build_status(images, builder, driver, account)
> +    result = { :provider =>  account.provider.name }
> +
> +    builds = images.map { |image| image.latest_pushed_or_unpushed_build }
> +    return result.merge(:status =>  :not_built) if builds.any? { |b| b.blank? }
> +
> +    building = builds.any? { |b| builder.find_active_build(b.id, driver) }
> +    return result.merge(:status =>  :building) if building
> +
> +    target_images = builds.map do |build|
> +      build.target_images.find { |ti| ti.target == driver }
> +    end
> +
> +    if target_images.any? { |ti| ti.blank? }
> +      return result.merge(:status =>  :not_built)
> +    end
> +
> +    pushing = target_images.any? do |ti|
> +      builder.find_active_push(ti.id, account.provider.name, account.credentials_hash["username"])
> +    end
> +    if pushing
> +      return result.merge(:status =>  :pushing)
> +    end
> +
> +    provider_images = target_images.map do |target_image|
> +      target_image.find_provider_image_by_provider_and_account(account.provider.name, account.warehouse_id).first
> +    end
> +
> +    if provider_images.any? { |pi| pi.blank? }
> +      return result.merge(:status =>  :not_pushed)
> +    end
> +    return result.merge(:status =>  :pushed)
> +  end
> +
> +  def build_all(images)
> +    images.each do |image|
> +      factory_image = Aeolus::Image::Factory::Image.new(:id =>  image.id)
> +      factory_image.targets = Provider.list_for_user(current_user, Privilege::VIEW).map {|p| p.provider_type.deltacloud_driver}.uniq.join(',')

^ targets can be generated only once before each loop

> +      factory_image.template = image.template
> +      factory_image.save!
> +    end
> +  end
> +
> +  def push_all(images)
> +    provider_images = []
> +    images.each do |image|
> +      ProviderAccount.group_by_type(current_user).each do |driver, groups|
> +        account = groups[:accounts].first
> +
> +        build = image.latest_pushed_or_unpushed_build
> +        raise Exception.new t('catalog_entries.errors.images_not_built') unless build

^ s/catalog_entries/deployables/ (multiple places where it's used)

> +        target_image = build.target_images.find { |ti| ti.target == driver }
> +        raise Exception.new t('catalog_entries.errors.images_not_built') unless target_image
> +        provider_images<<  Aeolus::Image::Factory::ProviderImage.new(
> +          :provider =>  account.provider.name,
> +          :credentials =>  account.to_xml(:with_credentials =>  true),
> +          :image_id =>  image.uuid,
> +          :build_id =>  build.uuid,
> +          :target_image_id =>  target_image.uuid
> +        )
> +      end
> +    end
> +
> +    provider_images.each do |provider_image|
> +      provider_image.save!
> +    end
> +  end
>   end
> diff --git a/src/app/views/deployables/show.html.haml b/src/app/views/deployables/show.html.haml
> index 1eab46e..02ab7a9 100644
> --- a/src/app/views/deployables/show.html.haml
> +++ b/src/app/views/deployables/show.html.haml
> @@ -39,19 +39,17 @@
>       %header
>         %h2= t('.build')
>
> -      = form_tag() do
> -        = select_tag :build_option, options_for_select([[t("rebuild_all"), 1]])
> +      = form_tag(catalog_deployable_build_path(@catalog_entry.catalog, @catalog_entry.deployable)) do
> +        = select_tag :build_options, options_for_select([[t(".rebuild_all"), :rebuild_all], [t(".push_all"), :push_all]])
>           = submit_tag t('.start'), :id =>  :start_build
>
>         %p.description= t('.build_description')
>
>         %ul#providers-list
> -        - @providers.each do |provider|
> +        - @build_results.each do |result|
>             %li
> -            %span.provider= provider.name
> -            %span.status Building 98%
> -            -#= form_tag()
> -            -#  %button on/off
> +            %span.provider= result[:provider]
> +            %span.status= result[:status]
>
>
>     %section.catalogs
> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> index 64512ec..57e0e41 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -783,12 +783,21 @@ en:
>         not_deleted:
>           one: "Deployable %{list} was not deleted. There are deployments associated with it."
>           other: "Deployables %{list} were not deleted. They have deployments associated with them."
> +    show:
> +      build: Build
> +      build_description:
> +        "Allow this deployable's associated image(s) to be built and pushed to
> +        providers by enabling them below."
> +      start: Start
> +      rebuild_all: Rebuild all images
> +      push_all: Push all images
>       flash:
>         error:
>           attribute_not_exist: "Some attribute(s) is missing in XML, please check the file!"
>       error:
>         attribute_not_exists: invalid
>         hwp_not_exists: "Hardware profile %{name} doesn't exist in DB"
> +      images_not_built: Error building images
>     name: Name
>     summary: Summary
>     settings:
> diff --git a/src/config/routes.rb b/src/config/routes.rb
> index 7a7df6e..b07ba9e 100644
> --- a/src/config/routes.rb
> +++ b/src/config/routes.rb
> @@ -205,6 +205,7 @@ Conductor::Application.routes.draw do
>       resources :deployables do
>         delete 'multi_destroy', :on =>  :collection
>         post :filter, :on =>  :collection
> +      post :build
>       end
>     end
>

Hi Tomas,
this patch basically works, but found couple of things (inline).

Jan



More information about the aeolus-devel mailing list