The overall separation of job setup and status in the SDK tries to segment job submission into more discrete manageable units around the workload itself and where/how the workload will run. This avoids it growing to an untenable number of mutually exclusive sets of arguments that are only applicable to specific executors, while allowing new executor and workload types to be added later. However, there's still a need for better executor-specific configuration and status- the current executors take their specific options as constructor args, which may not scale well for complex executor types. To bring in the remainder of runner's capabilities, we'll need a better place to hang some of those items, as well as providing executor-specific status information for in-flight jobs (eg, for local processes, the outer runner and/or core PIDs, for containers, the container ID and possibly runner PID, for mesh, the unit_id), while ideally maintaining a common call pattern across executors for the submit_jobs method.
Leaving aside how to accommodate new workload types for the moment (since we don't have clear requirements for that yet), it seems like a couple of changes would address the need for an organized place to hang these things:
an executor-specific Options type that's passed to submit_job with option values to customize the job submission that are apropos to that executor type (eg, local container types can accept a container ID and volume mounts; mesh jobs could contain the target node, job signature, worktype name)
making each executor's submit_job method return an executor-specific AnsibleJobStatus subclass that contains metadata germane only to that executor type (in addition to the base common events and executor-agnostic status properties)
The custom status object types would also allow for a generic cancellation mechanism that can still "do the right thing" depending on the type of the executor.
Moving to this model and making the options object required on submit_job would also imply that all constructor args to an executor type would be removed. Both of these changes should make the API itself less brittle, since we'll have less to worry about with varying arg lists between superclass invocations.
Proposed sample podman definitions:
class AnsiblePodmanJobExecutor(AnsibleContainerJobExecutor):
# note no `__init__` at all; we could probably go `classmethod` if we really wanted, but it's nice to have an instance to hang things on for future "session-based" kinds of multi-job workloads, among other things
async def submit_job(job_def: AnsibleJobDef, options: AnsiblePodmanJobOptions) -> AnsiblePodmanJobStatus:
...
@dataclass
class AnsiblePodmanJobOptions:
container_image_ref: str
running_container_ref: str | None
terminate_on_exit: bool = True
volume_mounts: list[str] | None
...
raw_options: list[str] | None # bad idea, or good escape hatch?
@dataclass
class AnsiblePodmanJobStatus(AnsibleJobStatus):
container_name: str
...
Another possibility might be to make XXXJobOptions a subclass of AnsibleJobDef, but keeping the job definition separate from the job-specific options seems like an easier way to support future workload types, rather than turning the common job definition into a larger dumping ground of options for different workload types, but again, we don't currently have many use cases for things beyond adhoc/direct-role/playbook execution here, so it's difficult to predict which way makes more sense. Especially for the job submission side, being able to split documentation between executor-agnostic properties and executor-specific seems like a win.
Akasurde
commented
2 years ago
The overall separation of job setup and status in the SDK tries to segment job submission into more discrete manageable units around the workload itself and where/how the workload will run. This avoids it growing to an untenable number of mutually exclusive sets of arguments that are only applicable to specific executors, while allowing new executor and workload types to be added later. However, there's still a need for better executor-specific configuration and status- the current executors take their specific options as constructor args, which may not scale well for complex executor types. To bring in the remainder of runner's capabilities, we'll need a better place to hang some of those items, as well as providing executor-specific status information for in-flight jobs (eg, for local processes, the outer runner and/or core PIDs, for containers, the container ID and possibly runner PID, for mesh, the unit_id), while ideally maintaining a common call pattern across executors for the
submit_jobsmethod.Leaving aside how to accommodate new workload types for the moment (since we don't have clear requirements for that yet), it seems like a couple of changes would address the need for an organized place to hang these things:
Optionstype that's passed tosubmit_jobwith option values to customize the job submission that are apropos to that executor type (eg, local container types can accept a container ID and volume mounts; mesh jobs could contain the target node, job signature, worktype name)submit_jobmethod return an executor-specificAnsibleJobStatussubclass that contains metadata germane only to that executor type (in addition to the base common events and executor-agnostic status properties)The custom status object types would also allow for a generic cancellation mechanism that can still "do the right thing" depending on the type of the executor.
Moving to this model and making the
optionsobject required onsubmit_jobwould also imply that all constructor args to an executor type would be removed. Both of these changes should make the API itself less brittle, since we'll have less to worry about with varying arg lists between superclass invocations.Proposed sample podman definitions:
Another possibility might be to make
XXXJobOptionsa subclass ofAnsibleJobDef, but keeping the job definition separate from the job-specific options seems like an easier way to support future workload types, rather than turning the common job definition into a larger dumping ground of options for different workload types, but again, we don't currently have many use cases for things beyond adhoc/direct-role/playbook execution here, so it's difficult to predict which way makes more sense. Especially for the job submission side, being able to split documentation between executor-agnostic properties and executor-specific seems like a win.