From 69c55ee003a3553d4e8b50d9bf98d2c31c8832e4 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Mon, 5 Jun 2023 13:11:23 +0000 Subject: [PATCH] refactor: daemon, config, and logging for better clarity (#225) - Import "path", "runtime", "strconv", and "strings" packages in daemon.go - Move "Starting runner daemon" log message to a different location - Refactor log formatter initialization and add debug level caller information - Split Config struct into separate Log, Runner, Cache, and Container structs with comments in config.go Signed-off-by: Bo-Yi Wu Reviewed-on: https://gitea.com/gitea/act_runner/pulls/225 Reviewed-by: Jason Song Co-authored-by: Bo-Yi Wu Co-committed-by: Bo-Yi Wu --- internal/app/cmd/daemon.go | 28 ++++++++++++--- internal/pkg/config/config.go | 65 +++++++++++++++++++++-------------- 2 files changed, 63 insertions(+), 30 deletions(-) diff --git a/internal/app/cmd/daemon.go b/internal/app/cmd/daemon.go index a648d64..9cd66e2 100644 --- a/internal/app/cmd/daemon.go +++ b/internal/app/cmd/daemon.go @@ -7,6 +7,10 @@ import ( "context" "fmt" "os" + "path" + "runtime" + "strconv" + "strings" "github.com/mattn/go-isatty" log "github.com/sirupsen/logrus" @@ -23,14 +27,13 @@ import ( func runDaemon(ctx context.Context, configFile *string) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { - log.Infoln("Starting runner daemon") - cfg, err := config.LoadDefault(*configFile) if err != nil { return fmt.Errorf("invalid configuration: %w", err) } initLogging(cfg) + log.Infoln("Starting runner daemon") reg, err := config.LoadRegistration(cfg.Runner.File) if os.IsNotExist(err) { @@ -79,10 +82,11 @@ func runDaemon(ctx context.Context, configFile *string) func(cmd *cobra.Command, // initLogging setup the global logrus logger. func initLogging(cfg *config.Config) { isTerm := isatty.IsTerminal(os.Stdout.Fd()) - log.SetFormatter(&log.TextFormatter{ + format := &log.TextFormatter{ DisableColors: !isTerm, FullTimestamp: true, - }) + } + log.SetFormatter(format) if l := cfg.Log.Level; l != "" { level, err := log.ParseLevel(l) @@ -90,6 +94,22 @@ func initLogging(cfg *config.Config) { log.WithError(err). Errorf("invalid log level: %q", l) } + + // debug level + if level == log.DebugLevel { + log.SetReportCaller(true) + format.CallerPrettyfier = func(f *runtime.Frame) (string, string) { + // get function name + s := strings.Split(f.Function, ".") + funcname := "[" + s[len(s)-1] + "]" + // get file name and line number + _, filename := path.Split(f.File) + filename = "[" + filename + ":" + strconv.Itoa(f.Line) + "]" + return funcname, filename + } + log.SetFormatter(format) + } + if log.GetLevel() != level { log.Infof("log level changed to %v", level) log.SetLevel(level) diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 4edf1d6..4819687 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -14,33 +14,46 @@ import ( "gopkg.in/yaml.v3" ) +// Log represents the configuration for logging. +type Log struct { + Level string `yaml:"level"` // Level indicates the logging level. +} + +// Runner represents the configuration for the runner. +type Runner struct { + File string `yaml:"file"` // File specifies the file path for the runner. + Capacity int `yaml:"capacity"` // Capacity specifies the capacity of the runner. + Envs map[string]string `yaml:"envs"` // Envs stores environment variables for the runner. + EnvFile string `yaml:"env_file"` // EnvFile specifies the path to the file containing environment variables for the runner. + Timeout time.Duration `yaml:"timeout"` // Timeout specifies the duration for runner timeout. + Insecure bool `yaml:"insecure"` // Insecure indicates whether the runner operates in an insecure mode. + FetchTimeout time.Duration `yaml:"fetch_timeout"` // FetchTimeout specifies the timeout duration for fetching resources. + FetchInterval time.Duration `yaml:"fetch_interval"` // FetchInterval specifies the interval duration for fetching resources. +} + +// Cache represents the configuration for caching. +type Cache struct { + Enabled *bool `yaml:"enabled"` // Enabled indicates whether caching is enabled. It is a pointer to distinguish between false and not set. If not set, it will be true. + Dir string `yaml:"dir"` // Dir specifies the directory path for caching. + Host string `yaml:"host"` // Host specifies the caching host. + Port uint16 `yaml:"port"` // Port specifies the caching port. +} + +// Container represents the configuration for the container. +type Container struct { + Network string `yaml:"network"` // Network specifies the network for the container. + NetworkMode string `yaml:"network_mode"` // Deprecated: use Network instead. Could be removed after Gitea 1.20 + Privileged bool `yaml:"privileged"` // Privileged indicates whether the container runs in privileged mode. + Options string `yaml:"options"` // Options specifies additional options for the container. + WorkdirParent string `yaml:"workdir_parent"` // WorkdirParent specifies the parent directory for the container's working directory. +} + +// Config represents the overall configuration. type Config struct { - Log struct { - Level string `yaml:"level"` - } `yaml:"log"` - Runner struct { - File string `yaml:"file"` - Capacity int `yaml:"capacity"` - Envs map[string]string `yaml:"envs"` - EnvFile string `yaml:"env_file"` - Timeout time.Duration `yaml:"timeout"` - Insecure bool `yaml:"insecure"` - FetchTimeout time.Duration `yaml:"fetch_timeout"` - FetchInterval time.Duration `yaml:"fetch_interval"` - } `yaml:"runner"` - Cache struct { - Enabled *bool `yaml:"enabled"` // pointer to distinguish between false and not set, and it will be true if not set - Dir string `yaml:"dir"` - Host string `yaml:"host"` - Port uint16 `yaml:"port"` - } `yaml:"cache"` - Container struct { - Network string `yaml:"network"` - NetworkMode string `yaml:"network_mode"` // Deprecated: use Network instead. Could be removed after Gitea 1.20 - Privileged bool `yaml:"privileged"` - Options string `yaml:"options"` - WorkdirParent string `yaml:"workdir_parent"` - } `yaml:"container"` + Log Log `yaml:"log"` // Log represents the configuration for logging. + Runner Runner `yaml:"runner"` // Runner represents the configuration for the runner. + Cache Cache `yaml:"cache"` // Cache represents the configuration for caching. + Container Container `yaml:"container"` // Container represents the configuration for the container. } // LoadDefault returns the default configuration.