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 zone to service templates. #358

Merged
merged 1 commit into from
Mar 29, 2019

Conversation

lfu
Copy link
Member

@lfu lfu commented Mar 27, 2019

Add zone to service templates to allow the running of service-related automate tasks in a specific zone.

Blocks ManageIQ/manageiq#18601
https://bugzilla.redhat.com/show_bug.cgi?id=1415106

@miq-bot add_label enhancement
cc @tinaafitz

@@ -0,0 +1,5 @@
class AddZoneToServiceTemplates < ActiveRecord::Migration[5.0]
def change
add_column :service_templates, :zone, :bigint
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be adding a reference? Not a column called zone?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I changed it from add_reference to add_column and forgot renaming the field.

Index seems not necessary so I selected add_column over add_reference. Seems there is no any place where we would query select service templates by a zone.

@carbonin
Copy link
Member

Should we set a zone for the already existing service templates as a part of this change as well?

@carbonin
Copy link
Member

Also, why is this on the service template and not the service? Should this not be overrideable at the service level?

@lfu lfu force-pushed the add_zone_to_service_1415106 branch from a2cbc39 to d2008ef Compare March 27, 2019 16:21
@miq-bot
Copy link
Member

miq-bot commented Mar 27, 2019

Checked commit lfu@d2008ef with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@lfu
Copy link
Member Author

lfu commented Mar 28, 2019

A service template's zone is an optional column. If the zone is not specified, the service provision would behave like before that the work can be handled in any zone.

A service template is the blueprint of the service provision where it defines what resources would be provisioned and how the provision should be processed. As a result of the provision, a service object along with its resources would be created.

The added zone column would be used to decide in which zone the provision should be handled.

@carbonin Is this helpful?

@carbonin
Copy link
Member

@lfu Yes, but it leads to another question. If a service is provisioned to a particular zone, should we be recording that in the service? What if a user changes the zone of the service template after the service was provisioned? Should that zone change be inherited by the service or should we always know the zone the service was originally assigned to?

@pemcg
Copy link

pemcg commented Mar 29, 2019

My view is that we wouldn't need to record this in the service. The advantage of being able to specify the zone at service template creation time is to be able to target the service's workflow towards a particular zone in the service definition, rather than it being run in any zone as at present. It's about defining where the workflow will run to take into account things like network restrictions.

Does that make sense?

@tinaafitz
Copy link
Member

Thanks @pemcg, that makes sense.

@carbonin
Copy link
Member

Yeah, I'm not questioning the motivation for specifying the zone in the template, I'm just questioning if this is all the information we need.

None of your justification addresses the situation I proposed in my last comment where the template zone is changed after provisioning. So to take that example further, is the service-specific automation the only place we will use this zone or might it get used for some custom retirement logic in the future? And will that logic always be run where the template specifies or should it be run where the service was first provisioned?

@carbonin
Copy link
Member

carbonin commented Mar 29, 2019

I guess this conversation doesn't preclude merging this PR though, so this looks good to me as is. If we end up needing this on the service as well that can be a follow-up.

@carbonin carbonin self-assigned this Mar 29, 2019
@carbonin carbonin merged commit 970699c into ManageIQ:master Mar 29, 2019
@carbonin carbonin added this to the Sprint 108 Ending Apr 1, 2019 milestone Mar 29, 2019
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.

5 participants