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 stack targeted refresh method #15936

Merged

Conversation

Ladas
Copy link
Contributor

@Ladas Ladas commented Sep 5, 2017

Add orchestration stack targeted refresh method, that based on
settings will perform targeted or full refresh.

Required by:
ManageIQ/manageiq-automation_engine#68
ManageIQ/manageiq-content#182

Add orchestration stack targeted refresh method, that based on
settings will perform targeted or full refresh.
@Ladas
Copy link
Contributor Author

Ladas commented Sep 5, 2017

@miq-bot assign @agrare
@miq-bot add_label enhancement

Add class method for OrchestrationStack refresh
@miq-bot
Copy link
Member

miq-bot commented Sep 7, 2017

Checked commits Ladas/manageiq@0537cc8~...300deb4 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

# Queue new targeted refresh if allowed
orchestration_stack_target = ManagerRefresh::Target.new(:manager => manager,
:association => :orchestration_stacks,
:manager_ref => {:ems_ref => manager_ref})
Copy link
Member

Choose a reason for hiding this comment

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

Much better than refresh_new_target 👍

end
unless manager.authentication_status_ok?
raise _("%{table} failed last authentication check") % {:table => ui_lookup(:table => "ext_management_systems")}
end
Copy link
Member

Choose a reason for hiding this comment

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

We should move these conditions to a common method, the same code pattern is in ext_management_system, host, and two places in vm_or_template

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I plan to extract it

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

👍
Lets follow this up with some refactoring to move the manager checks to a common place

@agrare agrare merged commit 300deb4 into ManageIQ:master Sep 7, 2017
agrare added a commit that referenced this pull request Sep 7, 2017
…ation_stack

Add orchestration stack targeted refresh method
@agrare agrare added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants