From 3737194b9f2875c10f3f2117c1816054ba0ff262 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 11 Jul 2018 15:51:51 +0200 Subject: [PATCH 1/4] daemon/*.go: fix some Wrap[f]/Warn[f] errors In particular, these two: > daemon/daemon_unix.go:1129: Wrapf format %v reads arg #1, but call has 0 args > daemon/kill.go:111: Warn call has possible formatting directive %s and a few more. Signed-off-by: Kir Kolyshkin Signed-off-by: Sebastiaan van Stijn --- daemon/daemon.go | 2 +- daemon/daemon_unix.go | 10 +++++----- daemon/kill.go | 2 +- daemon/monitor.go | 2 +- daemon/reload.go | 2 +- daemon/unpause.go | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 5e5f586ae0..119fac2a0a 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -538,7 +538,7 @@ func (daemon *Daemon) DaemonLeavesCluster() { select { case <-done: case <-time.After(5 * time.Second): - logrus.Warnf("timeout while waiting for ingress network removal") + logrus.Warn("timeout while waiting for ingress network removal") } } else { logrus.Warnf("failed to initiate ingress network removal: %v", err) diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index e2c77610d4..3c11f9d582 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -646,13 +646,13 @@ func (daemon *Daemon) initRuntimes(runtimes map[string]types.Runtime) (err error os.RemoveAll(runtimeDir + "-old") tmpDir, err := ioutils.TempDir(daemon.configStore.Root, "gen-runtimes") if err != nil { - return errors.Wrapf(err, "failed to get temp dir to generate runtime scripts") + return errors.Wrap(err, "failed to get temp dir to generate runtime scripts") } defer func() { if err != nil { if err1 := os.RemoveAll(tmpDir); err1 != nil { logrus.WithError(err1).WithField("dir", tmpDir). - Warnf("failed to remove tmp dir") + Warn("failed to remove tmp dir") } return } @@ -661,12 +661,12 @@ func (daemon *Daemon) initRuntimes(runtimes map[string]types.Runtime) (err error return } if err = os.Rename(tmpDir, runtimeDir); err != nil { - err = errors.Wrapf(err, "failed to setup runtimes dir, new containers may not start") + err = errors.Wrap(err, "failed to setup runtimes dir, new containers may not start") return } if err = os.RemoveAll(runtimeDir + "-old"); err != nil { logrus.WithError(err).WithField("dir", tmpDir). - Warnf("failed to remove old runtimes dir") + Warn("failed to remove old runtimes dir") } }() @@ -1126,7 +1126,7 @@ func setupRemappedRoot(config *config.Config) (*idtools.IDMappings, error) { mappings, err := idtools.NewIDMappings(username, groupname) if err != nil { - return nil, errors.Wrapf(err, "Can't create ID mappings: %v") + return nil, errors.Wrap(err, "Can't create ID mappings") } return mappings, nil } diff --git a/daemon/kill.go b/daemon/kill.go index 5034c4df39..3e6457e952 100644 --- a/daemon/kill.go +++ b/daemon/kill.go @@ -108,7 +108,7 @@ func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int) if unpause { // above kill signal will be sent once resume is finished if err := daemon.containerd.Resume(context.Background(), container.ID); err != nil { - logrus.Warn("Cannot unpause container %s: %s", container.ID, err) + logrus.Warnf("Cannot unpause container %s: %s", container.ID, err) } } diff --git a/daemon/monitor.go b/daemon/monitor.go index 5e740dd4fe..9b4452d7ef 100644 --- a/daemon/monitor.go +++ b/daemon/monitor.go @@ -138,7 +138,7 @@ func (daemon *Daemon) ProcessEvent(id string, e libcontainerd.EventType, ei libc "container": c.ID, "exec-id": ei.ProcessID, "exec-pid": ei.Pid, - }).Warnf("Ignoring Exit Event, no such exec command found") + }).Warn("Ignoring Exit Event, no such exec command found") } case libcontainerd.EventStart: c.Lock() diff --git a/daemon/reload.go b/daemon/reload.go index 210864ff87..452615c3ee 100644 --- a/daemon/reload.go +++ b/daemon/reload.go @@ -186,7 +186,7 @@ func (daemon *Daemon) reloadClusterDiscovery(conf *config.Config, attributes map } netOptions, err := daemon.networkOptions(daemon.configStore, daemon.PluginStore, nil) if err != nil { - logrus.WithError(err).Warnf("failed to get options with network controller") + logrus.WithError(err).Warn("failed to get options with network controller") return nil } err = daemon.netController.ReloadConfiguration(netOptions...) diff --git a/daemon/unpause.go b/daemon/unpause.go index 9061d50a16..27648ae72e 100644 --- a/daemon/unpause.go +++ b/daemon/unpause.go @@ -37,7 +37,7 @@ func (daemon *Daemon) containerUnpause(container *container.Container) error { daemon.LogContainerEvent(container, "unpause") if err := container.CheckpointTo(daemon.containersReplica); err != nil { - logrus.WithError(err).Warnf("could not save container to disk") + logrus.WithError(err).Warn("could not save container to disk") } return nil From a9a136572dc2a2c2ec5da320c4d0a32b5a15c550 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Jul 2018 04:07:45 -0700 Subject: [PATCH 2/4] Compile fix Go 1.11beta1 (rightfully) complains: > 15:38:37 daemon/cluster/controllers/plugin/controller.go:183: > Entry.Debugf format %#T has unrecognized flag # This debug print was added by commit 72c3bcf2a5. Signed-off-by: Kir Kolyshkin Signed-off-by: Sebastiaan van Stijn --- daemon/cluster/controllers/plugin/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/cluster/controllers/plugin/controller.go b/daemon/cluster/controllers/plugin/controller.go index 6d7606aa84..371f918d39 100644 --- a/daemon/cluster/controllers/plugin/controller.go +++ b/daemon/cluster/controllers/plugin/controller.go @@ -180,7 +180,7 @@ func (p *Controller) Wait(ctx context.Context) error { case <-ctx.Done(): return ctx.Err() case e := <-events: - p.logger.Debugf("got event %#T", e) + p.logger.Debugf("got event %T", e) switch e.(type) { case plugin.EventEnable: From 2e30e9e6db42043cb2bd67d25a7152488c834f9f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Jul 2018 11:03:52 -0700 Subject: [PATCH 3/4] aufs: fix Wrapf args Fix the following go-1.11beta1 build error: > daemon/graphdriver/aufs/aufs.go:376: Wrapf format %s reads arg #1, but call has 0 args While at it, change '%s' to %q. Signed-off-by: Kir Kolyshkin Signed-off-by: Sebastiaan van Stijn --- daemon/graphdriver/aufs/aufs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/graphdriver/aufs/aufs.go b/daemon/graphdriver/aufs/aufs.go index 9152252770..ac2fe26ecd 100644 --- a/daemon/graphdriver/aufs/aufs.go +++ b/daemon/graphdriver/aufs/aufs.go @@ -373,7 +373,7 @@ func atomicRemove(source string) error { case os.IsExist(err): // Got error saying the target dir already exists, maybe the source doesn't exist due to a previous (failed) remove if _, e := os.Stat(source); !os.IsNotExist(e) { - return errors.Wrapf(err, "target rename dir '%s' exists but should not, this needs to be manually cleaned up") + return errors.Wrapf(err, "target rename dir %q exists but should not, this needs to be manually cleaned up", target) } default: return errors.Wrapf(err, "error preparing atomic delete") From 09ad434f10cff48741322854a3003686b28295b5 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Jul 2018 11:51:50 -0700 Subject: [PATCH 4/4] loggerutils: build fixes, improve errors There are two build errors when using go-1.11beta1: > daemon/logger/loggerutils/logfile.go:367: Warningf format %q arg f.Name is a func value, not called > daemon/logger/loggerutils/logfile.go:564: Debug call has possible formatting directive %v In the first place, the file name is actually not required as error message already includes it. While at it, fix a couple of other places for more correct messages, and make sure to not add a file name if an error already has it. Fixes: f69f09f44c Signed-off-by: Kir Kolyshkin Signed-off-by: Sebastiaan van Stijn --- daemon/logger/loggerutils/logfile.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/daemon/logger/loggerutils/logfile.go b/daemon/logger/loggerutils/logfile.go index 6e3cda8648..d13db5ec48 100644 --- a/daemon/logger/loggerutils/logfile.go +++ b/daemon/logger/loggerutils/logfile.go @@ -332,7 +332,7 @@ func (w *LogFile) ReadLogs(config logger.ReadConfig, watcher *logger.LogWatcher) if strings.HasSuffix(fileName, tmpLogfileSuffix) { err := w.filesRefCounter.Dereference(fileName) if err != nil { - logrus.Errorf("Failed to dereference the log file %q: %v", fileName, err) + logrus.Errorf("Failed to dereference log file %q: %v", fileName, err) } } } @@ -364,7 +364,7 @@ func (w *LogFile) openRotatedFiles(config logger.ReadConfig) (files []*os.File, if strings.HasSuffix(f.Name(), tmpLogfileSuffix) { err := os.Remove(f.Name()) if err != nil && !os.IsNotExist(err) { - logrus.Warningf("Failed to remove the logfile %q: %v", f.Name, err) + logrus.Warnf("Failed to remove logfile: %v", err) } } } @@ -436,7 +436,7 @@ func decompressfile(fileName, destFileName string, since time.Time) (*os.File, e rs.Close() rErr := os.Remove(rs.Name()) if rErr != nil && !os.IsNotExist(rErr) { - logrus.Errorf("Failed to remove the logfile %q: %v", rs.Name(), rErr) + logrus.Errorf("Failed to remove logfile: %v", rErr) } return nil, errors.Wrap(err, "error while copying decompressed log stream to file") } @@ -561,7 +561,7 @@ func followLogs(f *os.File, logWatcher *logger.LogWatcher, notifyRotate chan int } return errRetry case err := <-fileWatcher.Errors(): - logrus.Debug("logger got error watching file: %v", err) + logrus.Debugf("logger got error watching file: %v", err) // Something happened, let's try and stay alive and create a new watcher if retries <= 5 { fileWatcher.Close()