Properly handle return code after error

[lpramuk]

Broker should properly handle RC after hitting errors.
Deploy WFs are not in any way special in this.

I won't go grepping broker output for ERROR - that's purpose of return codes

[INFO 221104 15:39:26] Using provider AnsibleTower to checkout
[INFO 221104 15:39:26] Using username and password authentication
[INFO 221104 15:39:28] Waiting for job: 
    API: https://infra-ansible-tower-01/api/v2/workflow_jobs/142076/
    UI: https://infra-ansible-tower-01/#/jobs/workflow/142076/output
[ERROR 221104 15:40:00] ProviderError: AnsibleTower encountered the following error: {'reason': 'Unable to determine failure cause for deploy-satellite-upgrade ar /api/v2/workflow_jobs/142076/'}
[Pipeline] echo
Broker RC for checkout: 0

Actual Result:
RC = 0

Expected Result:
RC != 0

[lpramuk]

Bump !
@jyejare Do we have a progress on this ? Accepted issue ?

[jyejare]

I dont see anywhere in discussions this issue is popped up. Could add some more details on severity of this issue please. That should help us to prioritize this fix.

[ogajduse]

I have just hit this issue by running the following command:

# passing non-existent RHEL version
$broker checkout --workflow deploy-base-rhel --deploy_rhel_version 99
[E 230117 17:13:40 exceptions:12] ProviderError: AnsibleTower encountered the following error: {'job': 'satlab-tower-deploy-version-choice', 'task': 'Take Deploy Workflow inputs and output correct set-stats for the remainder of workflow', 'reason': 'No template found for passed arguments'}
$ echo $?
0

Or just pass a wrong workflow name.

[ogajduse]

We are suppressing the actual exception, which is why the ExceptionHandler does not get it and therefore does not set the correct return code.

Here is the code snippet of _checkout and checkout functions where the changes need to happen.

broker/broker/broker.py

Lines 123 to 162 in 6881543

	@mp_decorator
	def _checkout(self):
	"""checkout one or more VMs
	:return: List of Host objects
	"""
	hosts = []
	if not self._provider_actions:
	raise self.BrokerError("Could not determine an appropriate provider")
	for action in self._provider_actions.keys():
	provider, method = PROVIDER_ACTIONS[action]
	logger.info(f"Using provider {provider.__name__} to checkout")
	try:
	host = self._act(provider, method, checkout=True)
	except exceptions.ProviderError as err:
	host = err
	if host and not isinstance(host, exceptions.ProviderError):
	hosts.append(host)
	logger.info(f"{host.__class__.__name__}: {host.hostname}")
	return hosts
	def checkout(self):
	"""checkout one or more VMs
	:return: Host obj or list of Host objects
	"""
	hosts = self._checkout()
	err, to_emit = None, []
	for host in hosts:
	if not isinstance(host, exceptions.ProviderError):
	to_emit.append(host.to_dict())
	else:
	err = host
	hosts.remove(host)
	helpers.emit(hosts=[host.to_dict() for host in hosts])
	self._hosts.extend(hosts)
	helpers.update_inventory([host.to_dict() for host in hosts])
	if err:
	raise err
	return hosts if not len(hosts) == 1 else hosts[0]

[JacobCallahan]

This was previously a workaround when Broker was using multiprocessing instead of multithreading. It should no longer be an issue to bubble this up again.