/juju

cmd/jujud/agent/machine: Preserve key too! #6413

Merged
merged 2 commits into from Oct 10, 2016

Conversation

Reviewers

@axw axw

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@mjs @jujubot @axw
@mjs
Contributor

mjs commented Oct 10, 2016 •
Edited 1 time

When the stateservinginfo setter preserves an already set certificate, the key that goes with it needs to be copied with it. Failing to do this results in key mismatches when the apiserver is restarted.

Also make the agent config updates safer by performing the whole update inside a ChangeConfig func.

Fixes https://bugs.launchpad.net/juju/+bug/1631145

QA

Bootstrapped and restarted the controller machine agent multiple times without being able to trigger the problem. Previously it was fairly easy to trigger the problem.
@mjs
cmd/jujud/agent/machine: Preserve key too! 
When preserving the already set certificate, the key that goes with it
needs to be copied with it. Failing to do this results in key mismatches
when the apiserver is restarted.

Fixes https://bugs.launchpad.net/juju/+bug/1631145
4b22f35
@axw

axw approved these changes Oct 10, 2016
View changes

LGTM with QA

cmd/jujud/agent/machine/servinginfo_setter.go
+
+ err := agent.ChangeConfig(func(config coreagent.ConfigSetter) error {
+ existing, hasInfo := config.StateServingInfo()
+ info, err := apiState.StateServingInfo()
@axw

axw Oct 10, 2016

Member

do this bit outside the ChangeConfig to avoid holding others up?

@axw

axw Oct 10, 2016

Member

(by which I mean just the API call)

@mjs

mjs Oct 10, 2016

Contributor

Yeah, that would be better. I'll update.

@mjs
cmd/jujud/agent/machine: Make SSI setter safe 
The config update by the ServingInfoSetterManifold is now not racy. The
agent config reads and updates are performed inside a ChangeConfig
operation.

Part of the fix for https://bugs.launchpad.net/juju/+bug/1631145
17a593f
@mjs
Contributor

mjs commented Oct 10, 2016

$$merge$$
@jujubot
Contributor

jujubot commented Oct 10, 2016

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

@jujubot jujubot merged commit c243ac2 into juju:master Oct 10, 2016

@mjs mjs deleted the mjs:1631145-key-mistmatch branch Oct 10, 2016

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