Remember the last logging config in the agent.conf file. #7709

Merged
merged 3 commits into from Aug 13, 2017

Conversation

Projects
None yet
3 participants
Owner

howbazaar commented Aug 7, 2017

Description of change

When agents currently restart they all start with --debug. This has two problems, one is that there is significant noise whenever an agent restarts and you are only interested in INFO or greater, and it is impossible to trace the dependency engine startup.

QA steps

Bootstrap a new controller, and update the logging-config for the controller model.
SSH into one of the machines and look at the agent.conf file. It will have a logging-config value.
Restart the agent and observe it using that config to set up the logging when the agent starts.

Really good to have this in place

agent/format.go
+//
+// When a new format version is introduced there is going to need to be some
+// refactoring around the config writing when provisioning a machine as the
+// controller may well understand a config format that the model does not. So
@mjs

mjs Aug 8, 2017

Contributor

Two spaces before "not"

cmd/jujud/agent/agent.go
+func setupAgentLogging(config agent.Config) {
+
+ if loggingOverride := config.Value(agent.LoggingOverride); loggingOverride != "" {
+ logger.Infof("setting logging override to %q", loggingOverride)
@mjs

mjs Aug 8, 2017

Contributor

"logging override set for this agent: %q"

cmd/jujud/agent/agent.go
+ loggo.DefaultContext().ResetLoggerLevels()
+ err := loggo.ConfigureLoggers(loggingConfig)
+ if err != nil {
+ logger.Errorf("setting logging config %v", err)
@mjs

mjs Aug 8, 2017

Contributor

"problem setting logging config: %v" might read better

@@ -502,10 +491,8 @@ func (a *MachineAgent) Run(*cmd.Context) error {
return errors.Errorf("cannot read agent configuration: %v", err)
}
- logger.Infof("machine agent %v start (%s [%s])", a.Tag(), jujuversion.Current, runtime.Compiler)
@mjs

mjs Aug 8, 2017

Contributor

This line appears to have been lost

@howbazaar

howbazaar Aug 10, 2017

Owner

Removed on purpose due to duplication.

@@ -148,10 +133,7 @@ func (a *UnitAgent) Run(ctx *cmd.Context) error {
if err := a.ReadConfig(a.Tag().String()); err != nil {
return err
}
- agentLogger.Infof("unit agent %v start (%s [%s])", a.Tag().String(), jujuversion.Current, runtime.Compiler)
@mjs

mjs Aug 8, 2017

Contributor

Same for this one

@howbazaar

howbazaar Aug 10, 2017

Owner

Same reason.

worker/logger/logger.go
lastConfig string
configOverride string
}
// NewLogger returns a worker.Worker that uses the notify watcher returned
// from the setup.
-func NewLogger(api *logger.State, agentConfig agent.Config) (worker.Worker, error) {
+func NewLogger(api *logger.State, agentConfig agent.Config, updateCallback func(string) error) (worker.Worker, error) {
logger := &Logger{
api: api,
agentConfig: agentConfig,
@mjs

mjs Aug 8, 2017

Contributor

Not your change but... the only thing this appears to get used for is the agent tag, so perhaps just store that instead of holding on to the config.

@howbazaar

howbazaar Aug 10, 2017

Owner

Fixed too.

mjs approved these changes Aug 10, 2017

Owner

howbazaar commented Aug 10, 2017

$$merge$$

Contributor

jujubot commented Aug 10, 2017

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

Contributor

jujubot commented Aug 10, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/114

Owner

howbazaar commented Aug 13, 2017

$$try-again$$

Contributor

jujubot commented Aug 13, 2017

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

@jujubot jujubot merged commit c175ad1 into juju:2.2 Aug 13, 2017

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details

@howbazaar howbazaar deleted the howbazaar:2.2-save-logging-config branch Aug 13, 2017

jujubot added a commit that referenced this pull request Aug 15, 2017

Merge pull request #7742 from howbazaar/2.2-logger-worker-race
Fix test race, and remove JujuConnSuite dependency.

This branch fixes a test race that was introduced in the recent PR #7709.

As a drive by I removed the JujuConnSuite and introduced an interface for the API calls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment