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

Working /dev/log proxy socket #177

Closed
wants to merge 2 commits into from

Conversation

goochjj
Copy link
Contributor

@goochjj goochjj commented Jun 13, 2020

Has conmon listen on bundle/slproxy, uses a provided OCI hook example to bind-mount that as /dev/log in the container. Conmon reads datagrams and writes them back out to the host /dev/log.

Probably needs some more testing - i.e. if /dev/log is deleted and recreated, making sure we don't end up in a write infinite loop.. Also, organizational comments welcome. Should this be a log-driver option? (one compounded with other such options). Should this be in ctr_logging? It's in conn_sock because that's where most of the code that was similar to this use case was located. Or should it be its own .c?

@goochjj goochjj mentioned this pull request Jun 13, 2020
@@ -0,0 +1,13 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd say let's put this and the hook in contrib/slproxy

@haircommander
Copy link
Collaborator

awesome @goochjj thanks for working on this

the approach works with me. my gut feeling is this more belongs in ctr_logging, but there is a lot of code similarities between this and conn_sock.c, and this does not function as the other log-drivers do (being written to through write_to_logs)

at the very least, I'd like the duplication here to drop a bit. if there was any shared or copied code between the implementations of console_fd and slproxy_fd, can you unify them into a function?

also, we should hide this behind a cli flag (whether it's --log-driver syslog: or --ctr-syslog or something)

finally, what happens if the user does not add the hook but we attempt to proxy to syslog?

@goochjj
Copy link
Contributor Author

goochjj commented Jun 15, 2020

at the very least, I'd like the duplication here to drop a bit. if there was any shared or copied code between the implementations of console_fd and slproxy_fd, can you unify them into a function?

I'll take a look - the main point of divergence was calling the correct callback - it seems that's a lot of the need for multiple functions in the first place... the async nature of the code.

Otherwise we'd need to store state somewhere - perhaps the conn_sock structure itself needs a flag to identify that it's for syslog proxy, instead of actual connection traffic.

also, we should hide this behind a cli flag (whether it's --log-driver syslog: or --ctr-syslog or something)

Agreed.. I'm just unsure right now if podman lets us pass multiple log drivers... I'm assuming it doesn't. Similarly it seems like there's no generic --conmon-arg to podman, unless I missed it - that would solve these sorts of things in an infinitely expandable way.

finally, what happens if the user does not add the hook but we attempt to proxy to syslog?

It'll create a slproxy socket in their userdata folder, but no bind-mount will be created, so it'll just... never receive any data. The hook bind mounts /run/libpod/socket/CID/slproxy to containerroot/dev/log

As an alternative - I modified the hook to spawn a busybox syslogd that gets bind mounted into the container, and forwards to the host's /dev/log. With that, I just use the hook without the need to modify conmon at all. It means my service ends up with two processes instead of one, and I have to switch the KillMode back on in the unit so the process gets cleaned up... I think... Because the oci hooks are subprocesses of conmon, conmon might actually be cleaning it up and reaping it.

Given this approach I'm comfortable retracting this PoC as it'll support more possibilities in the long run and keep conmon lean.

Systemctl status shows this:

   CGroup: /system.slice/rwhod.service
           ├─17116 /opt/podman/libexec/podman/conmon --api-version 1 -c d534e3ea8a830ca6f7c203c9812772cc3eb7346a4416bd86c184e297e1eceace -u d534e3ea8a830ca6f7c203c9812772cc3eb7346a4416bd86c184e297e1>
           └─17149 /opt/podman/bin/busybox syslogd -u /run/libpod/socket/d534e3ea8a830ca6f7c203c9812772cc3eb7346a4416bd86c184e297e1eceace/syslogproxy -R local:/run/systemd/journal/dev-log

@haircommander
Copy link
Collaborator

that sounds good to me. I think something like this is something we will eventually want. We've talked about having a generic "foward all stdout/err to this socket" feature to allow for log consumers to apply backpressure (slow down the container if it's producing output too quickly). but that's hit the (quite long) backlog 🙂

Glad you found a solution that works 😃

@goochjj goochjj closed this Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants