Merge lp://staging/~axwalk/juju-core/lp1281071-rsyslog-worker-tls into lp://staging/~go-bot/juju-core/trunk

Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2364
Proposed branch: lp://staging/~axwalk/juju-core/lp1281071-rsyslog-worker-tls
Merge into: lp://staging/~go-bot/juju-core/trunk
Prerequisite: lp://staging/~axwalk/juju-core/remove-syslog-cloudinit-config
Diff against target: 1452 lines (+1008/-32)
27 files modified
cmd/jujud/agent.go (+18/-0)
cmd/jujud/machine.go (+12/-0)
cmd/jujud/machine_test.go (+27/-0)
cmd/jujud/unit.go (+4/-0)
cmd/jujud/unit_test.go (+22/-0)
environs/cloudinit/cloudinit.go (+1/-0)
environs/config/config.go (+14/-2)
log/syslog/config.go (+73/-19)
log/syslog/config_test.go (+1/-0)
log/syslog/testing/syslogconf.go (+21/-4)
provider/local/config.go (+0/-1)
provider/local/environ.go (+9/-5)
state/api/params/params.go (+5/-0)
state/api/rsyslog/package_test.go (+14/-0)
state/api/rsyslog/rsyslog.go (+44/-0)
state/api/rsyslog/rsyslog_test.go (+35/-0)
state/api/state.go (+6/-0)
state/apiserver/root.go (+12/-0)
state/apiserver/rsyslog/package_test.go (+14/-0)
state/apiserver/rsyslog/rsyslog.go (+56/-0)
state/apiserver/rsyslog/rsyslog_test.go (+82/-0)
upgrades/rsysloggnutls.go (+14/-0)
upgrades/steps118.go (+5/-0)
upgrades/steps118_test.go (+2/-1)
worker/rsyslog/export_test.go (+12/-0)
worker/rsyslog/rsyslog_test.go (+248/-0)
worker/rsyslog/worker.go (+257/-0)
To merge this branch: bzr merge lp://staging/~axwalk/juju-core/lp1281071-rsyslog-worker-tls
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+208531@code.staging.launchpad.net

Commit message

Implement rsyslog TLS support

This changes the rsyslog configuration
we generate to use TLS. We install the
rsyslog-gnutls package and generate a
new CA cert, server cert and key
specifically for rsyslog.

This completely changes the way rsyslog
configuration is managed. Now, instead
of writing at bootstrap time and having
an upgrade step, we have a worker that
writes the syslog config whenever syslog
parameters change. The state server will
generate certificates and propagate the
CA cert to other agents via environ config.

There are some other miscellaneous changes:
  - we now use reliable forwarding, as
    otherwise the machine agent and unit
    agent will restart rsyslog while
    log messages are buffered causing
    message loss
  - dedicated upgrades for rsyslog are
    redundant and removed. The new worker
    upgrades machine and unit agent rsyslog
    automatically.
  - syslog-port can now be changed, and must
    be changed to work around the privilege
    drop race in rsyslog 5.x (this is the
    sole motivation)
  - the local provider symlinks machine-0.log
    into /var/log/juju$namespace so that we
    do not need any configuration other than
    the existing namespace to determine log
    location

Fixes lp:1281071
Fixes lp:1284020

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

Description of the change

Implement rsyslog TLS support

This changes the rsyslog configuration
we generate to use TLS. We install the
rsyslog-gnutls package and generate a
new CA cert, server cert and key
specifically for rsyslog.

This completely changes the way rsyslog
configuration is managed. Now, instead
of writing at bootstrap time and having
an upgrade step, we have a worker that
writes the syslog config whenever syslog
parameters change. The state server will
generate certificates and propagate the
CA cert to other agents via environ config.

There are some other miscellaneous changes:
  - we now use reliable forwarding, as
    otherwise the machine agent and unit
    agent will restart rsyslog while
    log messages are buffered causing
    message loss
  - dedicated upgrades for rsyslog are
    redundant and removed. The new worker
    upgrades machine and unit agent rsyslog
    automatically.
  - syslog-port can now be changed, and must
    be changed to work around the privilege
    drop race in rsyslog 5.x (this is the
    sole motivation)
  - the local provider symlinks machine-0.log
    into /var/log/juju$namespace so that we
    do not need any configuration other than
    the existing namespace to determine log
    location

Fixes lp:1281071
Fixes lp:1284020

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

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+208531_code.launchpad.net,

Message:
Please take a look.

Description:
Implement rsyslog TLS support

This changes the rsyslog configuration
we generate to use TLS. We install the
rsyslog-gnutls package and generate a
new CA cert, server cert and key
specifically for rsyslog.

This completely changes the way rsyslog
configuration is managed. Now, instead
of writing at bootstrap time and having
an upgrade step, we have a worker that
writes the syslog config whenever syslog
parameters change. The state server will
generate certificates and propagate the
CA cert to other agents via environ config.

There are some other miscellaneous changes:
   - we now use reliable forwarding, as
     otherwise the machine agent and unit
     agent will restart rsyslog while
     log messages are buffered causing
     message loss
   - dedicated upgrades for rsyslog are
     redundant and removed. The new worker
     upgrades machine and unit agent rsyslog
     automatically.
   - syslog-port can now be changed, and must
     be changed to work around the privilege
     drop race in rsyslog 5.x (this is the
     sole motivation)
   - the local provider symlinks machine-0.log
     into /var/log/juju$namespace so that we
     do not need any configuration other than
     the existing namespace to determine log
     location

Fixes lp:1281071
Fixes lp:1284020

https://code.launchpad.net/~axwalk/juju-core/lp1281071-rsyslog-worker-tls/+merge/208531

Requires:
https://code.launchpad.net/~axwalk/juju-core/remove-syslog-cloudinit-config/+merge/208278

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/68930045/

Affected files (+1011, -32 lines):
   A [revision details]
   M cmd/jujud/agent.go
   M cmd/jujud/machine.go
   M cmd/jujud/machine_test.go
   M cmd/jujud/unit.go
   M cmd/jujud/unit_test.go
   M environs/cloudinit/cloudinit.go
   M environs/config/config.go
   M log/syslog/config.go
   M log/syslog/config_test.go
   M log/syslog/testing/syslogconf.go
   M provider/local/config.go
   M provider/local/environ.go
   M state/api/params/params.go
   A state/api/rsyslog/package_test.go
   A state/api/rsyslog/rsyslog.go
   A state/api/rsyslog/rsyslog_test.go
   M state/api/state.go
   M state/apiserver/root.go
   A state/apiserver/rsyslog/package_test.go
   A state/apiserver/rsyslog/rsyslog.go
   A state/apiserver/rsyslog/rsyslog_test.go
   A upgrades/rsysloggnutls.go
   M upgrades/steps118.go
   M upgrades/steps118_test.go
   A worker/rsyslog/export_test.go
   A worker/rsyslog/rsyslog_test.go
   A worker/rsyslog/worker.go

Revision history for this message
Andrew Wilkins (axwalk) wrote :
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/

Revision history for this message
Andrew Wilkins (axwalk) wrote :

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}}
On 2014/02/27 06:22:38, thumper wrote:
> I know all this is going to conflict with Ian's branch we he changes
the
> behaviour to use a struct.

Nah, Ian's stuff just changed the tests. We should probably do the same
thing here, but really it's not that important.

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) {
On 2014/02/27 06:22:38, thumper wrote:
> useful elsewhere?

I'd rather not institutionalise it, because this will only work on *Nix
(other OSes, such as Windows, may have non-integer Uid/Gid). I'll keep
an eye out for people doing the same thing.

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

Revision history for this message
Go Bot (go-bot) wrote :
Revision history for this message
Go Bot (go-bot) wrote :

Attempt to merge into lp:juju-core failed due to conflicts:

text conflict in upgrades/steps118.go
text conflict in upgrades/steps118_test.go

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (16.1 KiB)

The attempt to merge lp:~axwalk/juju-core/lp1281071-rsyslog-worker-tls into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.017s
ok launchpad.net/juju-core/agent 1.265s
ok launchpad.net/juju-core/agent/tools 0.286s
ok launchpad.net/juju-core/bzr 7.759s
ok launchpad.net/juju-core/cert 3.152s
ok launchpad.net/juju-core/charm 0.619s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.035s
ok launchpad.net/juju-core/cloudinit/sshinit 1.122s
ok launchpad.net/juju-core/cmd 0.240s
ok launchpad.net/juju-core/cmd/charm-admin 0.835s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 257.647s
ok launchpad.net/juju-core/cmd/jujud 73.631s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 10.713s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/constraints 0.029s
ok launchpad.net/juju-core/container 0.040s
ok launchpad.net/juju-core/container/factory 0.062s
ok launchpad.net/juju-core/container/kvm 0.259s
ok launchpad.net/juju-core/container/kvm/mock 0.052s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 0.336s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.290s
ok launchpad.net/juju-core/environs 3.245s
ok launchpad.net/juju-core/environs/bootstrap 4.726s
ok launchpad.net/juju-core/environs/cloudinit 0.638s
ok launchpad.net/juju-core/environs/config 2.695s
ok launchpad.net/juju-core/environs/configstore 0.043s
ok launchpad.net/juju-core/environs/filestorage 0.032s
ok launchpad.net/juju-core/environs/httpstorage 0.946s
ok launchpad.net/juju-core/environs/imagemetadata 0.677s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.060s
ok launchpad.net/juju-core/environs/jujutest 0.245s
ok launchpad.net/juju-core/environs/manual 15.203s
ok launchpad.net/juju-core/environs/simplestreams 0.355s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.268s
ok launchpad.net/juju-core/environs/storage 1.312s
ok launchpad.net/juju-core/environs/sync 38.294s
ok launchpad.net/juju-core/environs/testing 0.271s
ok launchpad.net/juju-core/environs/tools 7.124s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.017s
ok launchpad.net/juju-core/instance 0.024s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 26.100s
ok launchpad.net/juju-core/juju/osenv 0.023s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.015s
ok launchpad.net/juju-core/log/syslog 0.026s
? launchpad.net/juju...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: