From 3e180eafb49f4705733f3b6afbb94fab581febd1 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Fri, 22 Mar 2024 00:54:31 -0400 Subject: [PATCH] cli: Refactor out log to the top level Remove some cruft at the same time. --- cli/deploy.go | 5 ++-- cli/run.go | 30 +++++++++++------------- cli/util/hello.go | 3 ++- cli/util/util.go | 5 ++-- lib/main.go | 58 ++++++++++++++++++++++------------------------- main.go | 8 ++++--- 6 files changed, 51 insertions(+), 58 deletions(-) diff --git a/cli/deploy.go b/cli/deploy.go index 8df5e161..6066af81 100644 --- a/cli/deploy.go +++ b/cli/deploy.go @@ -32,7 +32,6 @@ package cli import ( "context" "fmt" - "log" "os" "os/signal" @@ -106,7 +105,7 @@ func (obj *DeployArgs) Run(ctx context.Context, data *cliUtil.Data) (bool, error program, version := data.Program, data.Version Logf := func(format string, v ...interface{}) { - log.Printf("deploy: "+format, v...) + data.Flags.Logf("deploy: "+format, v...) } // TODO: consider adding a timeout based on an args.Timeout flag ? @@ -226,7 +225,7 @@ func (obj *DeployArgs) Run(ctx context.Context, data *cliUtil.Data) (bool, error Debug: data.Flags.Debug, Logf: func(format string, v ...interface{}) { // TODO: is this a sane prefix to use here? - log.Printf("cli: "+format, v...) + data.Flags.Logf("cli: "+format, v...) }, } diff --git a/cli/run.go b/cli/run.go index 790bfc6a..58e13e61 100644 --- a/cli/run.go +++ b/cli/run.go @@ -32,7 +32,6 @@ package cli import ( "context" "fmt" - "log" "os" "os/signal" "sync" @@ -111,12 +110,9 @@ func (obj *RunArgs) Run(ctx context.Context, data *cliUtil.Data) (bool, error) { main.Config = &obj.Config // pass in all the parsed data main.Program, main.Version = data.Program, data.Version - main.Flags = lib.Flags{ - Debug: data.Flags.Debug, - Verbose: data.Flags.Verbose, - } + main.Debug, main.Logf = data.Flags.Debug, data.Flags.Logf // no prefix Logf := func(format string, v ...interface{}) { - log.Printf("main: "+format, v...) + data.Flags.Logf("main: "+format, v...) } cliUtil.Hello(main.Program, main.Version, data.Flags) // say hello! @@ -138,9 +134,9 @@ func (obj *RunArgs) Run(ctx context.Context, data *cliUtil.Data) (bool, error) { }, Fs: standaloneFs, - Debug: main.Flags.Debug, + Debug: data.Flags.Debug, Logf: func(format string, v ...interface{}) { - log.Printf("cli: "+format, v...) + data.Flags.Logf("cli: "+format, v...) }, } @@ -159,7 +155,7 @@ func (obj *RunArgs) Run(ctx context.Context, data *cliUtil.Data) (bool, error) { if main.Deploy == nil { // nobody activated, but we'll still watch the etcd deploy chan, // and if there is deployed code that's ready to run, we'll run! - log.Printf("main: no frontend selected (no GAPI activated)") + data.Flags.Logf("main: no frontend selected (no GAPI activated)") } if err := main.Validate(); err != nil { @@ -188,20 +184,20 @@ func (obj *RunArgs) Run(ctx context.Context, data *cliUtil.Data) (bool, error) { select { case sig := <-signals: // any signal will do if sig != os.Interrupt { - log.Printf("interrupted by signal") + data.Flags.Logf("interrupted by signal") main.Interrupt(fmt.Errorf("killed by %v", sig)) return } switch count { case 0: - log.Printf("interrupted by ^C") + data.Flags.Logf("interrupted by ^C") main.Exit(nil) case 1: - log.Printf("interrupted by ^C (fast pause)") + data.Flags.Logf("interrupted by ^C (fast pause)") main.FastExit(nil) case 2: - log.Printf("interrupted by ^C (hard interrupt)") + data.Flags.Logf("interrupted by ^C (hard interrupt)") main.Interrupt(nil) } count++ @@ -215,14 +211,14 @@ func (obj *RunArgs) Run(ctx context.Context, data *cliUtil.Data) (bool, error) { reterr := main.Run() if reterr != nil { // log the error message returned - if main.Flags.Debug { - log.Printf("main: %+v", reterr) + if data.Flags.Debug { + data.Flags.Logf("main: %+v", reterr) } } if err := main.Close(); err != nil { - if main.Flags.Debug { - log.Printf("main: Close: %+v", err) + if data.Flags.Debug { + data.Flags.Logf("main: Close: %+v", err) } if reterr == nil { return false, err diff --git a/cli/util/hello.go b/cli/util/hello.go index 898ee710..002a3a14 100644 --- a/cli/util/hello.go +++ b/cli/util/hello.go @@ -40,6 +40,7 @@ import ( func Hello(program, version string, flags Flags) { var start = time.Now().UnixNano() + // TODO: Move these log package initialization steps to the top main.go? logFlags := log.LstdFlags if flags.Debug { logFlags = logFlags + log.Lshortfile @@ -55,5 +56,5 @@ func Hello(program, version string, flags Flags) { fmt.Println(fmt.Sprintf("This is: %s, version: %s", program, version)) fmt.Println("Copyright (C) 2013-2024+ James Shubin and the project contributors") fmt.Println("Written by James Shubin and the project contributors") - log.Printf("main: start: %v", start) + flags.Logf("main: start: %v", start) } diff --git a/cli/util/util.go b/cli/util/util.go index e2e55391..0bd5bcfb 100644 --- a/cli/util/util.go +++ b/cli/util/util.go @@ -54,10 +54,9 @@ func CliParseError(err error) error { } // Flags are some constant flags which are used throughout the program. -// TODO: Unify this with Debug and Logf ? type Flags struct { - Debug bool // add additional log messages - Verbose bool // add extra log message output + Debug bool // add additional log messages + Logf func(format string, v ...interface{}) } // Data is a struct of values that we usually pass to the main CLI function. diff --git a/lib/main.go b/lib/main.go index 4e70fe8e..eeac9db2 100644 --- a/lib/main.go +++ b/lib/main.go @@ -35,7 +35,6 @@ package lib import ( "context" "fmt" - "log" "os" "os/user" "path" @@ -74,12 +73,6 @@ const ( StoragePrefix = "/storage" ) -// Flags are some constant flags which are used throughout the program. -type Flags struct { - Debug bool // add additional log messages - Verbose bool // add extra log message output -} - // Config is a struct of all the configuration values for the Main struct. By // including this as a separate struct, it can be used as part of the API. This // API is not considered stable at this time, and is subject to change. @@ -90,8 +83,11 @@ type Config struct { // Version is the version of this program, usually set at compile time. Version string `arg:"-"` // cli should ignore - // Flags are some static global flags that are set at compile time. - Flags Flags `arg:"-"` // cli should ignore + // Debug represents if we're running in debug mode or not. + Debug bool `arg:"-"` // cli should ignore + + // Logf is a logger which should be used. + Logf func(format string, v ...interface{}) `arg:"-"` // cli should ignore // Hostname to use; nil if undefined. Useful for testing multiple // instances on same machine or for overriding a bad automatic hostname. @@ -300,7 +296,7 @@ func (obj *Main) Init() error { // Run is the main execution entrypoint to run mgmt. func (obj *Main) Run() error { Logf := func(format string, v ...interface{}) { - log.Printf("main: "+format, v...) + obj.Logf("main: "+format, v...) } exitCtx := obj.exit.Context() // local exit signal @@ -509,9 +505,9 @@ func (obj *Main) Run() error { NS: NS, // namespace Prefix: fmt.Sprintf("%s/", path.Join(prefix, "etcd")), - Debug: obj.Flags.Debug, + Debug: obj.Debug, Logf: func(format string, v ...interface{}) { - log.Printf("etcd: "+format, v...) + obj.Logf("etcd: "+format, v...) }, } if err := obj.embdEtcd.Init(); err != nil { @@ -558,9 +554,9 @@ func (obj *Main) Run() error { } simpleDeploy := &deployer.SimpleDeploy{ Client: etcdClient, - Debug: obj.Flags.Debug, + Debug: obj.Debug, Logf: func(format string, v ...interface{}) { - log.Printf("deploy: "+format, v...) + obj.Logf("deploy: "+format, v...) }, } if err := simpleDeploy.Init(); err != nil { @@ -577,9 +573,9 @@ func (obj *Main) Run() error { // implementation of the Local API (we only expect just this single one) localAPI := (&local.API{ Prefix: fmt.Sprintf("%s/", path.Join(prefix, "local")), - Debug: obj.Flags.Debug, + Debug: obj.Debug, Logf: func(format string, v ...interface{}) { - log.Printf("local: api: "+format, v...) + obj.Logf("local: api: "+format, v...) }, }).Init() @@ -593,9 +589,9 @@ func (obj *Main) Run() error { MetadataPrefix: MetadataPrefix, StoragePrefix: StoragePrefix, StandaloneFs: obj.DeployFs, // used for static deploys - Debug: obj.Flags.Debug, + Debug: obj.Debug, Logf: func(format string, v ...interface{}) { - log.Printf("world: etcd: "+format, v...) + obj.Logf("world: etcd: "+format, v...) }, } @@ -608,9 +604,9 @@ func (obj *Main) Run() error { World: world, Prefix: fmt.Sprintf("%s/", path.Join(prefix, "engine")), //Prometheus: prom, // TODO: implement this via a general Status API - Debug: obj.Flags.Debug, + Debug: obj.Debug, Logf: func(format string, v ...interface{}) { - log.Printf("engine: "+format, v...) + obj.Logf("engine: "+format, v...) }, } @@ -701,19 +697,19 @@ func (obj *Main) Run() error { //NoWatch: obj.NoWatch, NoStreamWatch: obj.NoStreamWatch, Prefix: fmt.Sprintf("%s/", path.Join(prefix, "gapi")), - Debug: obj.Flags.Debug, + Debug: obj.Debug, Logf: func(format string, v ...interface{}) { - log.Printf("gapi: "+format, v...) + obj.Logf("gapi: "+format, v...) }, } - if obj.Flags.Debug { + if obj.Debug { Logf("gapi: init...") } if err := gapiImpl.Init(data); err != nil { Logf("gapi: init failed: %+v", err) // TODO: consider running previous GAPI? } else { - if obj.Flags.Debug { + if obj.Debug { Logf("gapi: next...") } // this must generate at least one event for it to work @@ -723,7 +719,7 @@ func (obj *Main) Run() error { case next, ok := <-gapiChan: if !ok { // channel closed - if obj.Flags.Debug { + if obj.Debug { Logf("gapi exited") } gapiChan = nil // disable it @@ -766,7 +762,7 @@ func (obj *Main) Run() error { continue } Logf("new graph took: %s", time.Since(timing)) - if obj.Flags.Debug { + if obj.Debug { Logf("new graph: %+v", newGraph) } @@ -862,7 +858,7 @@ func (obj *Main) Run() error { continue // we'll catch the error later! } - if obj.Flags.Debug { + if obj.Debug { Logf("SendRecv: %s", res) // receiving here } @@ -1013,7 +1009,7 @@ func (obj *Main) Run() error { select { case deployChan <- deploy: // send - if obj.Flags.Debug { + if obj.Debug { Logf("deploy: sending new gapi") } case <-exitchan: @@ -1071,7 +1067,7 @@ func (obj *Main) Run() error { obj.exit.Done(errwrap.Wrapf(err, "deploy: watch error")) continue } - if obj.Flags.Debug { + if obj.Debug { Logf("deploy: got activity") } @@ -1123,7 +1119,7 @@ func (obj *Main) Run() error { select { case deployChan <- deploy: // send - if obj.Flags.Debug { + if obj.Debug { Logf("deploy: sending empty deploy") } @@ -1145,7 +1141,7 @@ func (obj *Main) Run() error { case deployChan <- deploy: last = latest // update last deployed // send - if obj.Flags.Debug { + if obj.Debug { Logf("deploy: sent new gapi") } diff --git a/main.go b/main.go index 37ecefa4..fd2f6cba 100644 --- a/main.go +++ b/main.go @@ -33,6 +33,7 @@ import ( "context" _ "embed" "fmt" + "log" "os" "github.com/purpleidea/mgmt/cli" @@ -48,7 +49,6 @@ import ( const ( tagline = "next generation config management" debug = false // add additional log messages - verbose = false // add extra log message output ) // set at compile time @@ -80,8 +80,10 @@ func main() { Copying: copying, Tagline: tagline, Flags: cliUtil.Flags{ - Debug: debug, - Verbose: verbose, + Debug: debug, + Logf: func(format string, v ...interface{}) { + log.Printf(format, v...) // the top-level log! + }, }, Args: os.Args, }