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

Jump to bottom

Override add resource to no-op in service orchestration subclass #18358

Merged
merged 2 commits into from
Jan 17, 2019
Merged

Override add resource to no-op in service orchestration subclass #18358

merged 2 commits into from
Jan 17, 2019

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Jan 14, 2019 •
edited
Loading

The service orchestration shouldn't be doing add_resource except for stacks and we should be telling people that they shouldn't try it for things except for stacks.

It fixes attempts to prevent someone from reopening https://bugzilla.redhat.com/show_bug.cgi?id=1664121

@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 14, 2019 •
edited
Loading

@miq-bot add_label enhancement
because it's not a bug
@miq-bot add_reviewer @tinaafitz
@miq-bot add_reviewer @bdunne

@miq-bot miq-bot requested review from tinaafitz and bdunne January 14, 2019 16:14
@d-m-u d-m-u force-pushed the prevent_add_resource_from_being_used_on_service_orchestration_thingys_for_great_justice branch 2 times, most recently from b4e0984 to 99edde6 Compare January 14, 2019 16:22
@bdunne
Copy link
Member

bdunne commented Jan 14, 2019

I have no idea what that error message should be saying, really.

Should we raise an exception here?

@d-m-u d-m-u closed this Jan 14, 2019
@d-m-u d-m-u reopened this Jan 15, 2019
@d-m-u d-m-u force-pushed the prevent_add_resource_from_being_used_on_service_orchestration_thingys_for_great_justice branch 3 times, most recently from b5976fa to d44d6d1 Compare January 15, 2019 19:57
@d-m-u d-m-u changed the title Override add_to_service to no-op in service orchestration subclass Override add resource to no-op in service orchestration subclass Jan 15, 2019
@d-m-u d-m-u force-pushed the prevent_add_resource_from_being_used_on_service_orchestration_thingys_for_great_justice branch 7 times, most recently from b1b4421 to 4daa00f Compare January 17, 2019 16:41
@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 17, 2019

🐌 🌹 🔑 🐢

@d-m-u d-m-u force-pushed the prevent_add_resource_from_being_used_on_service_orchestration_thingys_for_great_justice branch 2 times, most recently from f1b6080 to 967b0cd Compare January 17, 2019 21:00
@d-m-u d-m-u force-pushed the prevent_add_resource_from_being_used_on_service_orchestration_thingys_for_great_justice branch from 967b0cd to e7db10b Compare January 17, 2019 21:14
@miq-bot
Copy link
Member

miq-bot commented Jan 17, 2019

Checked commits d-m-u/manageiq@c953e29~...e7db10b with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@gmcculloug gmcculloug self-assigned this Jan 17, 2019
@gmcculloug gmcculloug merged commit d1bfff8 into ManageIQ:master Jan 17, 2019
@gmcculloug gmcculloug added this to the Sprint 103 Ending Jan 21, 2019 milestone Jan 17, 2019
@d-m-u d-m-u deleted the prevent_add_resource_from_being_used_on_service_orchestration_thingys_for_great_justice branch January 17, 2019 21:39
lfu
lfu reviewed Jan 29, 2019
View reviewed changes
app/models/service_orchestration.rb
@@ -77,6 +78,11 @@ def all_vms
orchestration_stack.vms
end

def add_resource(rsc, _options = {})
raise "Service Orchestration subclass does not support add_resource for #{rsc.class.name}" unless rsc.kind_of?(OrchestrationStack)
Copy link
Member

Choose a reason for hiding this comment

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

@d-m-u This would break the Orchestration Service.

Error: Service Orchestration subclass does not support add_resource for ManageIQ::Providers::Azure::CloudManager::OrchestrationTemplate

Copy link
Member

Choose a reason for hiding this comment

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

@lfu Thanks we were limiting what can be connected to an Orchestration Service object. We can expand this list, but it should be minimal since created resources would get connected through the Stack object.

Would have have a link to where the OrchestrationTemplate gets attached?

@d-m-u d-m-u mentioned this pull request May 21, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lfu lfu lfu left review comments

@gmcculloug gmcculloug gmcculloug left review comments

@tinaafitz tinaafitz tinaafitz approved these changes

@bdunne bdunne Awaiting requested review from bdunne

Assignees

@gmcculloug gmcculloug

Labels
enhancement lifecycle/orchestration lifecycle/services
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@d-m-u @bdunne @miq-bot @tinaafitz @gmcculloug @lfu