apiserver: add certificate changing test #6331

Merged
merged 1 commit into from Sep 27, 2016

Conversation

Projects
None yet
4 participants
Owner

rogpeppe commented Sep 27, 2016

Currently the certificate changing behaviour seems to be
untested in the apiserver package.

We also make the cert change listener a little more
like the other worker goroutines started by the
API server - it now listens to the API server tomb
and is waited for when the API server stops.

Also we move the changeCertListener code towards end of file
so it doesn't clutter the higher level code.

👍 with a small nit

apiserver/apiserver.go
+ cl.mu.Lock()
+ defer cl.mu.Unlock()
+ tlsConfig := utils.SecureTLSConfig()
+ //tlsConfig.GetCertificate = cl.getCertificate
@mhilton

mhilton Sep 27, 2016

Member

I always find commented out code disturbing, if it shouldn't be here could you remove it properly please?

apiserver/cert_test.go
+import (
+ "time"
+
+ coretesting "github.com/juju/juju/testing"
@dimitern

dimitern Sep 27, 2016

Contributor

Please move this to the group below.

apiserver: add certificate changing test
Currently the certificate changing behaviour seems to be
untested in the apiserver package.

We also make the cert change listener a little more
like the other worker goroutines started by the
API server - it now listens to the API server tomb
and is waited for when the API server stops.

Also we move the changeCertListener code towards end of file
so it doesn't clutter the higher level code.
Owner

rogpeppe commented Sep 27, 2016

$$merge$$

Contributor

jujubot commented Sep 27, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 42d0c9c into juju:master Sep 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment