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

Fixes #32030, #32188 - process events with unknown hosts #398

Closed

Conversation

ezr-ondrej
Copy link
Member

When ansible-runner executes a local task (through delegate_to: localhost or any other available mechanism in Ansible to do so), it
breaks the implicit assumption that all logs are related to a host
that we know about (with “we” being Foreman in general, and the
inputs["targets"] Dynflow structure in this particular case).

  • Guard against this possibility (and drop the log entry in this case).
  • Thwart silly Rubocop check - IMO "bite if angry" style with a
    composite "angry" clause (as in here) makes the code easy to misunderstand.
  • Broadcast messages for localhost
  • Raise an error for unknown other hosts

For events that don't have host, try to get the remote_addr that is
being reported for underlying communication events and guess hosts from
there.

When ansible-runner executes a local task (through `delegate_to:
localhost` or any other available mechanism in Ansible to do so), it
breaks the implicit assumption that all logs are related to a host
that we know about (with “we” being Foreman in general, and the
`inputs["targets"]` Dynflow structure in this particular case).

- Guard against this possibility (and drop the log entry in this case).
- Thwart silly Rubocop check - IMO "bite if angry" style with a
composite "angry" clause (as in here) makes the code easy to misunderstand.
- Broadcast messages for `localhost`
- Raise an error for unknown other hosts

For events that don't have host, try to get the `remote_addr` that is
being reported for underlying communication events and guess hosts from
there.
handle_host_event(hostname, event)
else
elsif event.key?('stdout')
Copy link
Contributor

Choose a reason for hiding this comment

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

So if we have a play which runs on hosts A and B and which has a task which gets delegated to localhost for both, would this mean that outputs of both hosts will contain outputs for both delegated tasks?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, indeed, that's what would happen. I can try to investigate if there is some other indication as for what host this is running FOR, but unless I have any luck with that, I don't see much of better ways.

Copy link
Contributor

Choose a reason for hiding this comment

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

From what I've seen, if a task is delegated, then .event_data.res has a _ansible_delegated_vars key, otherwise it is missing. Although I'm not sure how much we can rely on this.

@ezr-ondrej
Copy link
Member Author

Just for this not to die: should we have some general log, we would put the uncategorized logs and show those in all host logs?

@adamruzicka
Copy link
Contributor

This came out of a different issue, but maybe it would be applicable here as well. Currently when showing the output, we just show the lines, with numbers for reference and color (white vs red) marking whether the line was an error or not.

How about instead of doing this, we would give the output more structure, maybe add some columns to convey some further meaning and maybe even inject our own messages into the log such as "smart-proxy: Job was delegated to the proxy" and "Remote host: Job was started on the remote host".

If we had this facility, then we could reuse it for things like "ansible did some trick, there was this line of output but we don't really know which host it belongs to. anyway, here's the line.

@adamruzicka
Copy link
Contributor

Let's change the merge base to 6.3-stable and continue with it here and then forward-port it to smart_proxy_ansible

@ezr-ondrej
Copy link
Member Author

This won't get implemented for 6.3 and later (at least not by me) :)

@ezr-ondrej ezr-ondrej closed this Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants