all: don't log errors at warning level

- Don't log errors at warning level
- Don't log ignored errors at warning level, make them debug as there is
nothing the user is required to do.
- Don't log errors and return them, just return the error
- Don't log informational errors at warning level.
- Don't log multiline messages.

agent/tools/toolsdir.go

@@ -152,7 +152,7 @@ func removeAll(dir string) {
 	if err == nil || os.IsNotExist(err) {
 		return
 	}
-	logger.Warningf("cannot remove %q: %v", dir, err)
+	logger.Errorf("cannot remove %q: %v", dir, err)
 }
 
 func writeFile(name string, mode os.FileMode, r io.Reader) error {

apiserver/apiserver.go

@@ -481,7 +481,7 @@ func (srv *Server) newHandlerArgs(spec apihttp.HandlerConstraints) apihttp.NewHa
 			return st, nil, err
 		}
 	default:
-		logger.Warningf(`unrecognized access level %q; proceeding with "unauthenticated"`, spec.AuthKind)
+		logger.Infof(`unrecognized access level %q; proceeding with "unauthenticated"`, spec.AuthKind)
 		args.Connect = func(req *http.Request) (*state.State, state.Entity, error) {
 			st, err := ctxt.stateForRequestUnauthenticated(req)
 			return st, nil, err

cloudconfig/containerinit/container_userdata.go

@@ -226,7 +226,7 @@ func TemplateUserData(
 	// we need to enable apt-get update regardless of the environ
 	// setting, otherwise provisioning will fail.
 	if series == "precise" && !enablePackageUpdates {
-		logger.Warningf("series %q requires cloud-tools archive: enabling updates", series)
+		logger.Infof("series %q requires cloud-tools archive: enabling updates", series)
 		enablePackageUpdates = true
 	}
 

cmd/juju/cloud/detectcredentials.go

@@ -151,13 +151,13 @@ func (c *detectCredentialsCommand) Run(ctxt *cmd.Context) error {
 		if err != nil {
 			// Should never happen but it will on go 1.2
 			// because lxd provider is not built.
-			logger.Warningf("provider %q not available on this platform", providerName)
+			logger.Errorf("provider %q not available on this platform", providerName)
 			continue
 		}
 		if detectCredentials, ok := provider.(environs.ProviderCredentials); ok {
 			detected, err := detectCredentials.DetectCredentials()
 			if err != nil && !errors.IsNotFound(err) {
-				logger.Warningf("could not detect credentials for provider %q: %v", providerName, err)
+				logger.Errorf("could not detect credentials for provider %q: %v", providerName, err)
 				continue
 			}
 			if errors.IsNotFound(err) || len(detected.AuthCredentials) == 0 {
@@ -167,7 +167,7 @@ func (c *detectCredentialsCommand) Run(ctxt *cmd.Context) error {
 			// For each credential, construct meta info for which cloud it may pertain to etc.
 			for credName, newCred := range detected.AuthCredentials {
 				if credName == "" {
-					logger.Warningf("ignoring unnamed credential for provider %s", providerName)
+					logger.Debugf("ignoring unnamed credential for provider %s", providerName)
 					continue
 				}
 				// Ignore empty credentials.

cmd/juju/commands/bootstrap.go

@@ -366,14 +366,14 @@ func (c *bootstrapCommand) Run(ctx *cmd.Context) (resultErr error) {
 		}
 		if oldCurrentController != "" {
 			if err := modelcmd.WriteCurrentController(oldCurrentController); err != nil {
-				logger.Warningf(
+				logger.Errorf(
 					"cannot reset current controller to %q: %v",
 					oldCurrentController, err,
 				)
 			}
 		}
 		if err := store.RemoveController(c.controllerName); err != nil {
-			logger.Warningf(
+			logger.Errorf(
 				"cannot destroy newly created controller %q details: %v",
 				c.controllerName, err,
 			)
@@ -430,7 +430,7 @@ func (c *bootstrapCommand) Run(ctx *cmd.Context) (resultErr error) {
 	defer func() {
 		if resultErr != nil {
 			if c.KeepBrokenEnvironment {
-				logger.Warningf(`
+				logger.Infof(`
 bootstrap failed but --keep-broken was specified so model is not being destroyed.
 When you are finished diagnosing the problem, remember to run juju destroy-model --force
 to clean up the model.`[1:])

container/interface.go

@@ -84,6 +84,6 @@ func (m ManagerConfig) PopValue(key string) string {
 // WarnAboutUnused emits a warning about each value in the map.
 func (m ManagerConfig) WarnAboutUnused() {
 	for key, value := range m {
-		logger.Warningf("unused config option: %q -> %q", key, value)
+		logger.Infof("unused config option: %q -> %q", key, value)
 	}
 }

container/kvm/kvm.go

@@ -159,7 +159,7 @@ func (manager *containerManager) CreateContainer(
 		fmt.Sprintf("arch=%s mem=%vM root-disk=%vG cpu-cores=%v",
 			startParams.Arch, startParams.Memory, startParams.RootDisk, startParams.CpuCores))
 	if err != nil {
-		logger.Warningf("failed to parse hardware: %v", err)
+		return nil, nil, errors.Annotate(err, "failed to parse hardware")
 	}
 
 	callback(status.StatusAllocating, "Creating container; it might take some time", nil)

container/kvm/kvm_test.go

@@ -53,7 +53,7 @@ func (*KVMSuite) TestManagerWarnsAboutUnknownOption(c *gc.C) {
 		"shazam":             "Captain Marvel",
 	})
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(c.GetTestLog(), jc.Contains, `WARNING juju.container unused config option: "shazam" -> "Captain Marvel"`)
+	c.Assert(c.GetTestLog(), jc.Contains, `INFO juju.container unused config option: "shazam" -> "Captain Marvel"`)
 }
 
 func (s *KVMSuite) TestListInitiallyEmpty(c *gc.C) {

container/lxc/lxc.go

@@ -253,9 +253,8 @@ func (manager *containerManager) CreateContainer(
 			return nil, nil, errors.Annotate(err, "failed to reorder network settings")
 		}
 
-		err = callback(status.StatusProvisioning, "Cloning template container", nil)
-		if err != nil {
-			logger.Warningf("Cannot set instance status for container: %v", err)
+		if err := callback(status.StatusProvisioning, "Cloning template container", nil); err != nil {
+			return nil, nil, errors.Annotate(err, "Cannot set instance status for container")
 		}
 		lxcContainer, err = templateContainer.Clone(name, extraCloneArgs, templateParams)
 		if err != nil {
@@ -891,7 +890,7 @@ func networkConfigTemplate(config container.NetworkConfig) string {
 	case container.BridgeNetwork:
 		data.Type = "veth"
 	default:
-		logger.Warningf(
+		logger.Infof(
 			"unknown network type %q, using the default %q config",
 			config.NetworkType, container.BridgeNetwork,
 		)
@@ -917,7 +916,7 @@ func networkConfigTemplate(config container.NetworkConfig) string {
 			nic.IPv4Gateway = iface.GatewayAddress.Value
 		}
 		if iface.MACAddress == "" || nic.MACAddress == "" {
-			logger.Warningf(
+			logger.Infof(
 				"empty MAC address %q from config for %q (rendered as %q)",
 				iface.MACAddress, iface.InterfaceName, nic.MACAddress,
 			)
@@ -949,7 +948,7 @@ func networkConfigTemplate(config container.NetworkConfig) string {
 func generateNetworkConfig(config *container.NetworkConfig) string {
 	if config == nil {
 		config = DefaultNetworkConfig()
-		logger.Warningf("network type missing, using the default %q config", config.NetworkType)
+		logger.Infof("network type missing, using the default %q config", config.NetworkType)
 	}
 	return networkConfigTemplate(*config)
 }

container/lxc/lxc_test.go

@@ -609,7 +609,7 @@ func (*LxcSuite) TestManagerWarnsAboutUnknownOption(c *gc.C) {
 		"shazam":             "Captain Marvel",
 	}, &containertesting.MockURLGetter{})
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(c.GetTestLog(), jc.Contains, `WARNING juju.container unused config option: "shazam" -> "Captain Marvel"`)
+	c.Assert(c.GetTestLog(), jc.Contains, `INFO juju.container unused config option: "shazam" -> "Captain Marvel"`)
 }
 
 func (s *LxcSuite) TestCreateContainer(c *gc.C) {
@@ -1110,7 +1110,7 @@ func (*NetworkSuite) TestGenerateNetworkConfig(c *gc.C) {
 			"lxc.network.link = lxcbr0",
 			"lxc.network.flags = up",
 		},
-		logContains:       `WARNING juju.container.lxc network type missing, using the default "bridge" config`,
+		logContains:       `INFO juju.container.lxc network type missing, using the default "bridge" config`,
 		logDoesNotContain: `INFO juju.container.lxc setting MTU to 0 for LXC network interfaces`,
 	}, {
 		about:  "default config",
@@ -1249,7 +1249,7 @@ func (*NetworkSuite) TestNetworkConfigTemplate(c *gc.C) {
 	c.Assert(obtained, jc.DeepEquals, expected)
 	log := c.GetTestLog()
 	c.Assert(log, jc.Contains,
-		`WARNING juju.container.lxc unknown network type "foo", using the default "bridge" config`,
+		`INFO juju.container.lxc unknown network type "foo", using the default "bridge" config`,
 	)
 }
 

environs/bootstrap/bootstrap.go

@@ -420,7 +420,7 @@ func setBootstrapTools(environ environs.Environ, possibleTools coretools.List) (
 	if !isCompatibleVersion(newVersion, jujuversion.Current) {
 		compatibleVersion, compatibleTools := findCompatibleTools(possibleTools, jujuversion.Current)
 		if len(compatibleTools) == 0 {
-			logger.Debugf(
+			logger.Infof(
 				"failed to find %s tools, will attempt to use %s",
 				jujuversion.Current, newVersion,
 			)
@@ -493,10 +493,7 @@ func validateConstraints(env environs.Environ, cons constraints.Value) error {
 		return errors.Trace(err)
 	}
 	unsupported, err := validator.Validate(cons)
-	if len(unsupported) > 0 {
-		err = errors.Annotatef(err, "unsupported constraints: %v", unsupported)
-	}
-	return err
+	return errors.Annotatef(err, "unsupported constraints: %v", unsupported)
 }
 
 // guiArchive returns information on the GUI archive that will be uploaded

environs/config/config.go

@@ -768,14 +768,17 @@ func (c *Config) ControllerUUID() string {
 // DefaultSeries returns the configured default Ubuntu series for the environment,
 // and whether the default series was explicitly configured on the environment.
 func (c *Config) DefaultSeries() (string, bool) {
-	if s, ok := c.defined["default-series"]; ok {
-		if series, ok := s.(string); ok && series != "" {
-			return series, true
-		} else if !ok {
-			logger.Warningf("invalid default-series: %q", s)
-		}
+	s, ok := c.defined["default-series"]
+	if !ok {
+		return "", false
+	}
+	switch s := s.(type) {
+	case string:
+		return s, s != ""
+	default:
+		logger.Errorf("invalid default-series: %q", s)
+		return "", false
 	}
-	return "", false
 }
 
 // StatePort returns the controller port for the environment.
@@ -1448,11 +1451,11 @@ func (cfg *Config) ValidateUnknownAttrs(fields schema.Fields, defaults schema.De
 func (cfg *Config) GenerateControllerCertAndKey(hostAddresses []string) (string, string, error) {
 	caCert, hasCACert := cfg.CACert()
 	if !hasCACert {
-		return "", "", fmt.Errorf("model configuration has no ca-cert")
+		return "", "", errors.New("model configuration has no ca-cert")
 	}
 	caKey, hasCAKey := cfg.CAPrivateKey()
 	if !hasCAKey {
-		return "", "", fmt.Errorf("model configuration has no ca-private-key")
+		return "", "", errors.New("model configuration has no ca-private-key")
 	}
 	return cert.NewDefaultServer(caCert, caKey, hostAddresses)
 }

environs/manual/provisioner.go

@@ -76,7 +76,7 @@ func ProvisionMachine(args ProvisionMachineArgs) (machineId string, err error) {
 		if machineId != "" && err != nil {
 			logger.Errorf("provisioning failed, removing machine %v: %v", machineId, err)
 			if cleanupErr := args.Client.ForceDestroyMachines(machineId); cleanupErr != nil {
-				logger.Warningf("error cleaning up machine: %s", cleanupErr)
+				logger.Errorf("error cleaning up machine: %s", cleanupErr)
 			}
 			machineId = ""
 		}

juju/api.go

@@ -107,7 +107,7 @@ func newAPIFromStore(args NewAPIConnectionParams, apiOpen api.OpenFunc) (api.Con
 		if errorImportance(err0) < errorImportance(err1) {
 			err0, err1 = err1, err0
 		}
-		logger.Infof("discarding API open error: %v", err1)
+		logger.Errorf("discarding API open error: %v", err1)
 		return err0
 	}
 	try := parallel.NewTry(0, chooseError)
@@ -159,20 +159,19 @@ func newAPIFromStore(args NewAPIConnectionParams, apiOpen api.OpenFunc) (api.Con
 			// lose error encapsulation:
 			err = ierr.error
 		}
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 
 	st := val0.(api.Connection)
 	addrConnectedTo, err := serverAddress(st.Addr())
 	if err != nil {
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 	// Update API addresses if they've changed. Error is non-fatal.
 	hostPorts := st.APIHostPorts()
-	if localerr := updateControllerAddresses(
-		args.Store, args.ControllerName, controllerDetails, hostPorts, addrConnectedTo,
-	); localerr != nil {
-		logger.Debugf("cannot cache API addresses: %v", localerr)
+	err = updateControllerAddresses(args.Store, args.ControllerName, controllerDetails, hostPorts, addrConnectedTo)
+	if err != nil {
+		logger.Errorf("cannot cache API addresses: %v", err)
 	}
 	return st, nil
 }

provider/azure/instancetype.go

@@ -97,7 +97,7 @@ func newInstanceType(size compute.VirtualMachineSize) instances.InstanceType {
 		}
 	}
 	if cost == len(machineSizeCost) {
-		logger.Warningf("found unknown VM size %q", sizeName)
+		logger.Errorf("found unknown VM size %q", sizeName)
 	}
 
 	vtype := "Hyper-V"

provider/azure/storage.go

@@ -616,7 +616,7 @@ func nextAvailableLUN(vm *compute.VirtualMachine) (int, error) {
 		for _, disk := range *vm.Properties.StorageProfile.DataDisks {
 			lun := to.Int(disk.Lun)
 			if lun < 0 || lun > 31 {
-				logger.Warningf("ignore disk with invalid LUN: %+v", disk)
+				logger.Debugf("ignore disk with invalid LUN: %+v", disk)
 				continue
 			}
 			inUse[lun] = true

state/cleanup.go

@@ -108,14 +108,14 @@ func (st *State) Cleanup() (err error) {
 		default:
 			handler, ok := cleanupHandlers[doc.Kind]
 			if !ok {
-				err = fmt.Errorf("unknown cleanup kind %q", doc.Kind)
+				err = errors.Errorf("unknown cleanup kind %q", doc.Kind)
 			} else {
 				persist := st.newPersistence()
 				err = handler(st, persist, doc.Prefix)
 			}
 		}
 		if err != nil {
-			logger.Warningf("cleanup failed: %v", err)
+			logger.Errorf("cleanup failed: %v", err)
 			continue
 		}
 		ops := []txn.Op{{
@@ -124,7 +124,7 @@ func (st *State) Cleanup() (err error) {
 			Remove: true,
 		}}
 		if err := st.runTransaction(ops); err != nil {
-			logger.Warningf("cannot remove empty cleanup document: %v", err)
+			return errors.Annotate(err, "cannot remove empty cleanup document")
 		}
 	}
 	return nil

storage/provider/loop.go

@@ -229,7 +229,7 @@ func (lvs *loopVolumeSource) detachVolume(tag names.VolumeTag) error {
 		return errors.Annotate(err, "locating loop device")
 	}
 	if len(deviceNames) > 1 {
-		logger.Warningf("expected 1 loop device, got %d", len(deviceNames))
+		logger.Errorf("expected 1 loop device, got %d", len(deviceNames))
 	}
 	for _, deviceName := range deviceNames {
 		if err := detachLoopDevice(lvs.run, deviceName); err != nil {

worker/apicaller/connect.go

@@ -199,7 +199,10 @@ func ScaryConnect(a agent.Agent, apiOpen api.OpenFunc) (_ api.Connection, err er
 	// Update the agent config if necessary; this should just read the
 	// conn's properties, rather than making api calls, so we don't
 	// need to think about facades yet.
-	maybeSetAgentModelTag(a, conn)
+	if err := maybeSetAgentModelTag(a, conn); err != nil {
+		// apperently it's fine for this to fail
+		logger.Errorf("maybeSetAgentModelTag failed: %v", err)
+	}
 
 	// newConnFacade is patched out in export_test, because exhaustion.
 	// proper config/params struct would be better.
@@ -255,7 +258,7 @@ func ScaryConnect(a agent.Agent, apiOpen api.OpenFunc) (_ api.Connection, err er
 // it's missing a model tag. It doesn't *really* matter if it fails,
 // because we can demonstrably connect without it, so we log any
 // errors encountered and never return any to the client.
-func maybeSetAgentModelTag(a agent.Agent, conn api.Connection) {
+func maybeSetAgentModelTag(a agent.Agent, conn api.Connection) error {
 	if a.CurrentConfig().Model().Id() == "" {
 		err := a.ChangeConfig(func(setter agent.ConfigSetter) error {
 			modelTag, err := conn.ModelTag()
@@ -266,11 +269,9 @@ func maybeSetAgentModelTag(a agent.Agent, conn api.Connection) {
 				Model: modelTag,
 			})
 		})
-		if err != nil {
-			logger.Warningf("unable to save model uuid: %v", err)
-			// Not really fatal, just annoying.
-		}
+		return errors.Annotate(err, "unable to save model uuid")
 	}
+	return nil
 }
 
 // changePassword generates a new random password and records it in

worker/certupdater/certupdater.go

@@ -122,12 +122,11 @@ func (c *CertificateUpdater) updateCertificate(addresses []network.Address, done
 	// available.
 	stateInfo, ok := c.getter.StateServingInfo()
 	if !ok {
-		logger.Warningf("no state serving info, cannot regenerate server certificate")
-		return nil
+		return errors.New("no state serving info, cannot regenerate server certificate")
 	}
 	caPrivateKey := stateInfo.CAPrivateKey
 	if caPrivateKey == "" {
-		logger.Warningf("no CA cert private key, cannot regenerate server certificate")
+		logger.Errorf("no CA cert private key, cannot regenerate server certificate")
 		return nil
 	}
 	// Grab the env config and update a copy with ca cert private key.