Code review comment for lp:~wwitzel3/juju-core/009-ha-rsyslog-api

Revision history for this message
Michael Foord (mfoord) wrote :

On 2014/05/19 10:02:47, mfoord wrote:

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

https://codereview.appspot.com/94510043/diff/20001/worker/rsyslog/worker.go#newcode152
> worker/rsyslog/worker.go:152: if cfg.Port == h.syslogPort &&
rsyslogCACert ==
> h.rsyslogCACert {
> This will actually screw us over. If APIHosts changes, but not the
port nor the
> cert, then the conf won't get rewritten. So this needs to change.

Note that I've removed this in a separate branch, along with an obsolete
test that now fails, in the following branch:

https://code.launchpad.net/~mfoord/juju-core/ha-rsyslog-shortcut-removal/+merge/220105

Andrew Wilkins (axw) is fairly certain that by the time the
rsyslogworker is created the port is in the environment, and starting
the rsyslog worker already ensures the ca-cert will be available, so
just watching APIHostPorts should be safe (regarding rsyslog config
writing and restarting).

https://codereview.appspot.com/94510043/

« Back to merge proposal