Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Fix bug 1735684: Don't watch manual machines from the firewaller. #8244
Conversation
|
Can one of the admins verify this patch? |
babbageclunk
approved these changes
Dec 20, 2017
Looks good, one suggestion about moving logging inside startMachine.
| if err != nil { | ||
| return err | ||
| } | ||
| - logger.Debugf("started watching %q", tag) | ||
| + if watching { |
babbageclunk
Dec 20, 2017
Member
If you're only using the bool return for logging, it would be better to log inside startMachine and lose the bool (and then you can have different log messages for the different not watching cases).
|
$$merge$$ |
|
Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju |
jujubot
merged commit c899e00
into
juju:2.3
Dec 20, 2017
1 check passed
continuous-integration/jenkins/pr-merge
This commit looks good
Details
hmlanigan
deleted the
hmlanigan:bug1735684
branch
Dec 20, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
hmlanigan commentedDec 19, 2017
Description of change
Since juju shouldn't be touching ports on a manually provisioned machine, don't start watching them.
QA steps
No provider or firewater error like those in bug should be seen. There should be a debug message: "not watching machine-#".
Documentation changes
N/A
Bug reference
https://bugs.launchpad.net/juju/+bug/1735684