Code review comment for lp://staging/~axwalk/juju-core/lp1281071-rsyslog-worker-tls

Revision history for this message
Tim Penhey (thumper) wrote :

LGTM

Initially thought this would clash with Ian's work, but appears not.

https://codereview.appspot.com/68930045/diff/20001/cmd/jujud/unit.go
File cmd/jujud/unit.go (right):

https://codereview.appspot.com/68930045/diff/20001/cmd/jujud/unit.go#newcode98
cmd/jujud/unit.go:98: runner.StartWorker("rsyslog", func()
(worker.Worker, error) {
Hmm... the sooner we can merge the unit agent and the machine agent the
better I think.

https://codereview.appspot.com/68930045/diff/20001/log/syslog/config.go
File log/syslog/config.go (right):

https://codereview.appspot.com/68930045/diff/20001/log/syslog/config.go#newcode57
log/syslog/config.go:57: $DefaultNetstreamDriverCAFile {{tlsCACertPath}}
I know all this is going to conflict with Ian's branch we he changes the
behaviour to use a struct.

https://codereview.appspot.com/68930045/diff/20001/log/syslog/testing/syslogconf.go
File log/syslog/testing/syslogconf.go (right):

https://codereview.appspot.com/68930045/diff/20001/log/syslog/testing/syslogconf.go#newcode18
log/syslog/testing/syslogconf.go:18: $InputFileName
/var/log/juju{{.Namespace}}/{{.MachineTag}}.log
Ah... maybe not then.

https://codereview.appspot.com/68930045/diff/20001/worker/rsyslog/worker.go
File worker/rsyslog/worker.go (right):

https://codereview.appspot.com/68930045/diff/20001/worker/rsyslog/worker.go#newcode175
worker/rsyslog/worker.go:175: var lookupUser = func(username string)
(uid, gid int, err error) {
useful elsewhere?

https://codereview.appspot.com/68930045/

« Back to merge proposal