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.

Sign up for GitHub

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

Tower CUD to invoke targeted refresh #14954

Merged
merged 1 commit into from
May 4, 2017
Merged

Tower CUD to invoke targeted refresh #14954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions ...manageiq/providers/ansible_tower/shared/automation_manager/configuration_script_source.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,15 @@ def provider_collection(manager)
connection.api.projects
end
end
end

def provider_object(connection = nil)
(connection || connection_source.connect).api.projects.find(manager_ref)
end

REFRESH_ON_TOWER_SLEEP = 1.second
def refresh_in_provider
with_provider_object do |project|
def refresh_in_provider(project, id = nil)
return unless project.can_update?

project_update = project.update

# this is really just a quick hack. We should do this properly once
# https://github.com/ManageIQ/manageiq/pull/14405 is merged
log_header = "updating project #{project.id} (#{self.class.name} #{id})"
log_header = "updating project #{project.id} (#{name} #{id})"
_log.info "#{log_header}..."
Timeout.timeout(5.minutes) do
loop do
Expand All @@ -45,4 +38,15 @@ def refresh_in_provider
_log.info "#{log_header}...Complete"
end
end

def provider_object(connection = nil)
(connection || connection_source.connect).api.projects.find(manager_ref)
end

REFRESH_ON_TOWER_SLEEP = 1.second
def refresh_in_provider
with_provider_object do |project|
self.class.refresh_in_provider(project, id)
end
end
end
13 changes: 7 additions & 6 deletions app/models/manageiq/providers/ansible_tower/shared/automation_manager/tower_api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ def create_in_provider(manager_id, params)
params = provider_params(params) if respond_to?(:provider_params)
manager = ExtManagementSystem.find(manager_id)
tower_object = provider_collection(manager).create!(params)

if respond_to?(:refresh_in_provider)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we already have the tower object and so we can try to trigger the sync.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this better ... The full refresh can stay for now until we have @agrare's refresh_new_target in place.

And, this avoids doing the double refresh just to make sure we get the updated Tower git repo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice @jameswnl this looks much better :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont want to be the spoilsport, but refresh_new_target is for the old refresh and this is using the graph refresh.
But good news follows: afaik the graph refresh supports that out of the box. ManagerRefresh::Target
@Ladas can you point to a small code sample in amazon, where you create objects from hashes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/Ladas/manageiq-providers-amazon/blob/cd02dc09e805424ef90cf30f082f710000c09319/spec/models/manageiq/providers/amazon/cloud_manager/vcr_specs/targeted_refresh_spec.rb#L273-L273

I think we can integrate that to the refresh_new_target and switch based on configuration.

refresh_in_provider(tower_object)
end
refresh(manager)
find_by!(:manager_id => manager.id, :manager_ref => tower_object.id)
rescue AnsibleTowerClient::ClientError, ActiveRecord::RecordNotFound => error
Expand Down Expand Up @@ -34,10 +36,9 @@ def notify(op, manager_id, params, success)
)
end

def refresh(manager)
# Get the record in our database
# TODO: This needs to be targeted refresh so it doesn't take too long
task_ids = EmsRefresh.queue_refresh_task(manager)
def refresh(target)
# Get the record into our database
task_ids = EmsRefresh.queue_refresh_task(target)
task_ids.each { |tid| MiqTask.wait_for_taskid(tid) }
end

Expand Down Expand Up @@ -66,7 +67,7 @@ def update_in_provider(params)
with_provider_object do |provider_object|
provider_object.update_attributes!(params)
end
self.class.send('refresh', manager)
self.class.send('refresh', self)
reload
rescue AnsibleTowerClient::ClientError => error
raise
Expand Down
12 changes: 8 additions & 4 deletions spec/support/ansible_shared/automation_manager/configuration_script_source.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
context "create through API" do
let(:projects) { double("AnsibleTowerClient::Collection", :create! => project) }
let(:project) { AnsibleTowerClient::Project.new(nil, project_json) }
let(:css) { store_new_project(project, manager) }

let(:project_json) do
params.merge(
Expand Down Expand Up @@ -41,7 +42,8 @@
it ".create_in_provider to succeed and send notification" do
expect(AnsibleTowerClient::Connection).to receive(:new).and_return(atc)
store_new_project(project, manager)
expect(EmsRefresh).to receive(:queue_refresh_task).and_return([finished_task])
expect(described_class).to receive(:refresh_in_provider).and_return(nil)
expect(EmsRefresh).to receive(:queue_refresh_task).with(manager).and_return([finished_task])
expect(ExtManagementSystem).to receive(:find).with(manager.id).and_return(manager)
expect(projects).to receive(:create!).with(params)
expect(Notification).to receive(:create).with(expected_notify)
Expand All @@ -50,6 +52,7 @@

it ".create_in_provider to fail(not found during refresh) and send notification" do
expect(AnsibleTowerClient::Connection).to receive(:new).and_return(atc)
expect(described_class).to receive(:refresh_in_provider).and_return(nil)
expect(EmsRefresh).to receive(:queue_refresh_task).and_return([finished_task])
expect(ExtManagementSystem).to receive(:find).with(manager.id).and_return(manager)
expected_notify[:type] = :tower_op_failure
Expand All @@ -61,7 +64,8 @@
params[:authentication_id] = credential.id
expect(AnsibleTowerClient::Connection).to receive(:new).and_return(atc)
store_new_project(project, manager)
expect(EmsRefresh).to receive(:queue_refresh_task).and_return([finished_task])
expect(described_class).to receive(:refresh_in_provider).and_return(nil)
expect(EmsRefresh).to receive(:queue_refresh_task).with(manager).and_return([finished_task])
expect(ExtManagementSystem).to receive(:find).with(manager.id).and_return(manager)
expected_params = params.clone.merge(:credential => '1')
expected_params.delete(:authentication_id)
Expand Down Expand Up @@ -111,7 +115,7 @@ def store_new_project(project, manager)

it "#delete_in_provider to succeed and send notification" do
expect(AnsibleTowerClient::Connection).to receive(:new).and_return(atc)
expect(EmsRefresh).to receive(:queue_refresh_task).and_return([finished_task])
expect(EmsRefresh).to receive(:queue_refresh_task).with(manager).and_return([finished_task])
expect(Notification).to receive(:create).with(expected_notify)
project.delete_in_provider
end
Expand Down Expand Up @@ -156,7 +160,7 @@ def store_new_project(project, manager)

it "#update_in_provider to succeed and send notification" do
expect(AnsibleTowerClient::Connection).to receive(:new).and_return(atc)
expect(EmsRefresh).to receive(:queue_refresh_task).and_return([finished_task])
expect(EmsRefresh).to receive(:queue_refresh_task).with(project).and_return([finished_task])
expect(Notification).to receive(:create).with(expected_notify)
expect(project.update_in_provider({})).to be_a(described_class)
end
Expand Down
2 changes: 1 addition & 1 deletion spec/support/ansible_shared/automation_manager/credential.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
expected_params[:organization] = 1 if described_class.name.include?("::EmbeddedAnsible::")
expect(AnsibleTowerClient::Connection).to receive(:new).and_return(atc)
store_new_credential(credential, manager)
expect(EmsRefresh).to receive(:queue_refresh_task).and_return([finished_task])
expect(EmsRefresh).to receive(:queue_refresh_task).with(manager).and_return([finished_task])
expect(ExtManagementSystem).to receive(:find).with(manager.id).and_return(manager)
expect(credentials).to receive(:create!).with(expected_params)
expect(Notification).to receive(:create).with(expected_notify)
Expand Down