Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add orchestration_stacks to ServiceAnsiblePlaybook #14248

Merged
merged 1 commit into from Mar 10, 2017

Conversation

@jntullo
Copy link

jntullo commented Mar 9, 2017

The SUI needs to be able to access the jobs on a ServiceAnsiblePlaybook. This returns them as an array:

[
  #<ManageIQ::Providers::AnsibleTower::AutomationManager::Job:0x007fe6608789c8
   id: 371,
   name: "miq-playbook-demo_provision_hd2g8j13",
   type: "ManageIQ::Providers::AnsibleTower::AutomationManager::Job",
   description: nil,
   status: "successful",
   ems_ref: "114",
   ancestry: nil,
   ems_id: 10,
   orchestration_template_id: 516,
   created_at: Tue, 14 Feb 2017 19:20:42 UTC +00:00,
   updated_at: Fri, 24 Feb 2017 16:45:02 UTC +00:00,
   retired: nil,
   retires_on: nil,
   retirement_warn: nil,
   retirement_last_warn: nil,
   retirement_state: nil,
   retirement_requester: nil,
   status_reason: nil,
   cloud_tenant_id: nil,
   resource_group: nil,
   start_time: Tue, 14 Feb 2017 20:20:55 UTC +00:00,
   finish_time: Tue, 14 Feb 2017 20:20:58 UTC +00:00,
   configuration_script_base_id: 488,
   verbosity: 0,
   hosts: ["localhost"]>
]

cc: @AllenBW
thoughts @bzwei ?
@miq-bot add_label enhancement, services

@AllenBW

This comment has been minimized.

Copy link
Member

AllenBW commented Mar 9, 2017

Well put @jntullo SUI NEEDS 🌮 💃

app/models/service.rb Outdated
@@ -37,6 +37,7 @@ class Service < ApplicationRecord
virtual_has_many :vms
virtual_has_many :all_vms
virtual_has_many :power_states, :uses => :all_vms
virtual_has_many :jobs

This comment has been minimized.

Copy link
@bzwei

bzwei Mar 9, 2017

Contributor

change :jobs to :orchestration_stacks

app/models/service_ansible_playbook.rb Outdated
@@ -45,6 +45,12 @@ def postprocess(action)
delete_inventory(action) unless use_default_inventory?(hosts)
end

def jobs

This comment has been minimized.

Copy link
@bzwei

bzwei Mar 9, 2017

Contributor
def orchestration_stacks
  service_resources.where(:resource_type => 'OrchestrationStack').each_with_object({}) do |sr, hash|
    hash[sr.name] = sr.resource
  end
end

This comment has been minimized.

Copy link
@jntullo

jntullo Mar 9, 2017

Author

@bzwei I had this initially, but I found that my sample data had multiple resources for the same action, which is why I thought I'd utilize job. Is this just an anomaly due to bad data?

This comment has been minimized.

Copy link
@bzwei

bzwei Mar 9, 2017

Contributor

We should have one Provision and one Retirement job, but for Reconfigure that we may support in the future, it can have multiple jobs.
I am hesitated too about this matter. We don't have clear answer whether we want to support Reconfigure. If so, even the existing job method needs to return an array.

@jntullo jntullo changed the title Add Jobs to ServiceAnsiblePlaybook Add orchestration_stacks to ServiceAnsiblePlaybook Mar 9, 2017
@jntullo

This comment has been minimized.

Copy link
Author

jntullo commented Mar 9, 2017

app/models/service_ansible_playbook.rb Outdated
@@ -45,6 +45,12 @@ def postprocess(action)
delete_inventory(action) unless use_default_inventory?(hosts)
end

def orchestration_stacks

This comment has been minimized.

Copy link
@bzwei

bzwei Mar 9, 2017

Contributor

A second thought - the method should not return a hash. Just like vms, an array is better. And this method can be moved to Service thus good for Orchestration and AnsibleTower services too.

Jillian Tullo
jobs to orchestration stacks

move from service_ansible_playbook to service
@miq-bot

This comment has been minimized.

Copy link
Member

miq-bot commented Mar 9, 2017

Checked commit jntullo@b92057d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🍪

@gmcculloug

This comment has been minimized.

Copy link
Member

gmcculloug commented Mar 10, 2017

@jntullo Please update the description to reflect the updated return format.

Looks like we lose the action with the recent change. Not sure if that is required information to retain here. cc @bzwei

@jntullo

This comment has been minimized.

Copy link
Author

jntullo commented Mar 10, 2017

@gmcculloug done 👍

@jntullo

This comment has been minimized.

Copy link
Author

jntullo commented Mar 10, 2017

while it would be nice to retain the action, the action can be retrieved from the service resource

@gmcculloug gmcculloug merged commit 8b010d5 into ManageIQ:master Mar 10, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.0008%) to 44.387%
Details
@AllenBW

This comment has been minimized.

Copy link
Member

AllenBW commented Mar 10, 2017

OH GLORIOUS DAY!!!! 🌮 💃

@jntullo jntullo deleted the jntullo:enhancement/expose_jobs branch Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.