-
Notifications
You must be signed in to change notification settings - Fork 896
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 my_zone to ansible tower service template. #15233
Conversation
@miq-bot assign @gmcculloug |
@@ -52,6 +52,10 @@ def self.validate_config_info(options) | |||
config_info | |||
end | |||
|
|||
def my_zone | |||
job_template.manager.try(:my_zone) || MiqServer.my_zone |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the job_template.manager.try(:my_zone)
part. Why default to MiqServer.my_zone
? What happens if my_zone
just returns nil instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gmcculloug If we return nil instead of defaulting to MiqServer.my_zone, the work is put on the queue with a nil zone. I initially thought the service_template_provision_task deliver_to_automate method would catch it, but it only defaults it if the source doesn't respond to my_zone.
determine_queue_zone will leave it as nil because the options key zone exists:
if options.key?(:zone) options[:zone] # return specified zone including nil (aka ANY zone)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are referring to service_template_provision_task.rb#L146.
I guess my follow-up question would be: Is the service_template valid if there is no manager or if the manager has a nil
zone? I do not think it is, therefore I still see value in simplifying this method to job_template.manager.try(:my_zone)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gmcculloug Good point. Changes made.
6b067d0
to
3347c30
Compare
end | ||
|
||
context 'without job template manager' do | ||
it "takes the zone from MiqServer" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description needs to be updated to match the new functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gmcculloug Changed description.
3347c30
to
1d0bb97
Compare
Checked commit tinaafitz@1d0bb97 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
Add my_zone to ansible tower service template. (cherry picked from commit e871897) https://bugzilla.redhat.com/show_bug.cgi?id=1460356
Fine backport details:
|
Adding my_zone to service_template_ansible_tower enables Ansible Tower Service provisioning to queue work in the provider zone.
https://bugzilla.redhat.com/show_bug.cgi?id=1451473