Skip to content
This repository has been archived by the owner on Feb 5, 2021. It is now read-only.

systemd poc #21

Merged
merged 5 commits into from Jul 17, 2017
Merged

systemd poc #21

merged 5 commits into from Jul 17, 2017

Conversation

flah00
Copy link
Contributor

@flah00 flah00 commented Apr 13, 2017

This is a very rough cut, for #19. I wanted to offer a simple POC, and see what folks thought. Many of the systemd units created by kube-aws are not included. I'd be happy to work on that.

Some questions:

  • How is the Dockerfile maintained?
    • The fluentd folks are using a single file, but it's an erb template
  • How are the separate configs, for systemd and in_file maintained?
    • I have no idea what that might look like
  • Is this image built using a bot?
    • It would be essential for both to be updated at the same time
  • Would latest point at the current image and a new tag? What would the new tag be?
  • Docker can log to systemd, but it requires DOCKER_OPTS=--log-driver=journald, thus container logs remain in_file
    • This should be accounted for, somehow

Thanks for this awesome project! Please let me know what you think!

@frankreno
Copy link
Contributor

@flah00 : thank you again for this! I will review it ASAP.

@andrewrynhard
Copy link

Any timeline for this?

@frankreno
Copy link
Contributor

@flah00 @andrewrynhard : I have not had any cycles. Plan to review this week.

@flah00
Copy link
Contributor Author

flah00 commented May 8, 2017

Hey, @frankreno, any thoughts on this?

@frankreno
Copy link
Contributor

frankreno commented May 10, 2017

@flah00 : sorry for the delay in getting back to you on this. I think this is a great first start. A few ideas, let me know your thoughts.

Can we do this from a single Docker file and modify the entry point? I like how you have separated the conf files and I think that its ok to have those all ported into the same container. What I am thinking though is that we change the entry point to a script that detects (perhaps using an environment variable in the Daemonset? ) if you want to use the systemd configs or not. This might be nice to make it more extensible, suporting more than just SystemD down the line. Basically, wrap up the logic for what plugins fluentD starts in the script and have the docker file start that way. Thoughts?

@flah00
Copy link
Contributor Author

flah00 commented May 15, 2017

@frankreno

If I understand, you're suggesting that we use fluent/fluentd:stable-debian, for both systemd and in_tail? I'm all for that, nice and simple. I believe that files currently residing in /fluentd/conf.d should be moved to /fluentd/conf.d/file.

Would you like me to clean this POC up and move forward, with a new entrypoint? Perhaps add the other kube-aws services?

I think users might also find it helpful if they could project their own configs, via a configmap, into their containers.

New env vars

  • FLUENTD_SOURCE file or systemd, defaults to file
  • FLUENTD_CONFIG_PATH A directory which contains additional optional user generated fluentd configs

Entrypoint behavior

  • Exit non-zero, if FLUENTD_SOURCE is not file or systemd
  • Recursively copy from FLUENTD_CONFIG_PATH to /fluentd/conf.d/file or /fluentd/conf.d/systemd
  • Start fluentd

Do you know, off the top of your head, whether @include /fluentd/conf.d/systemd/source.*.conf could be made to use an env var? It would be so much nicer if we could use FLUENTD_SOURCE, in the fluentd config.

@flah00
Copy link
Contributor Author

flah00 commented Jun 13, 2017

@frankreno what are you thoughts on the changes?

Quite odd, and I'm not able to explain why I see 128 of these over four days

2017-06-13 19:39:01 +0000 [info]: Worker 0 finished unexpectedly with signal SIGKILL
$ kubectl get po
NAME                         READY     STATUS    RESTARTS   AGE
sl-sumologic-fluentd-1rscd   1/1       Running   0          4d

@frankreno
Copy link
Contributor

@flah00 : I am going to have time to review this next week

@frankreno
Copy link
Contributor

@flah00 I have reviewed the files and this look good, thanks for this and sorry for the delay. Before I merge, I wanna just test a few things on my end.

@frankreno frankreno self-requested a review July 17, 2017 22:03
@frankreno frankreno self-assigned this Jul 17, 2017
@frankreno frankreno merged commit b73b4ed into SumoLogic:master Jul 17, 2017
@frankreno
Copy link
Contributor

testing went well and all LGTM, thanks @flah00 for this!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants