search

openshift / ansible-service-broker

Ansible Service Broker
Apache License 2.0
226 stars 84 forks source link

JobState should be sent as part of the JobMsg rather than be set by the subscribers #608

Closed maleck13 closed 6 years ago

maleck13 commented 6 years ago

Feature:

As discussed on IRC with @eriknelson, while looking at the last operation PR it occurred to me it would make more sense to have the Job send through the JobState as part of its msg that is consumed by the subscribers rather than the subscribers deciding what state to set as can be seen here: https://github.com/openshift/ansible-service-broker/blob/master/pkg/broker/provision_subscriber.go#L55
I intended to include this change in my main PR for the last_operation feature, but after discussion, it probably makes more sense to have it as a standalone PR.

jmrodri commented 6 years ago

Maybe this is okay. I'd have to think more about it, but originally the idea was the jobs did some work, sent a message with some information, and the subscribers would then decide what the state should be.

eriknelson commented 6 years ago

@jmrodri We spoke through this at some point over the last week. Commenting here for posterity (and to remind myself of my own thoughts). Conceptually, it makes sense for the jobs themselves to own and report their own JobState rather than subscribers. We got away with subscribers setting the value because jobs have always only triggered a singular event at their conclusion, and returned an error if something went wrong, otherwise they were assumed to have successfully completed. It's apparent that jobs with progress updates will likely need to report them as state, and a subscriber will be unable to set this value since the Job is ultimately the authority on its own state.