refactor: use standard logger instead of logrus

This commit is contained in:
Lukas Zapletal 2025-04-29 15:17:22 +02:00 committed by Achilleas Koutsou
parent 48cd004bed
commit 392a04fd12
9 changed files with 152 additions and 29 deletions

View file

@ -6,6 +6,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"log"
"os" "os"
"os/signal" "os/signal"
"path/filepath" "path/filepath"
@ -13,7 +14,6 @@ import (
"strings" "strings"
"syscall" "syscall"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"github.com/osbuild/image-builder-cli/pkg/progress" "github.com/osbuild/image-builder-cli/pkg/progress"
@ -24,6 +24,7 @@ import (
"github.com/osbuild/images/pkg/ostree" "github.com/osbuild/images/pkg/ostree"
"github.com/osbuild/image-builder-cli/internal/blueprintload" "github.com/osbuild/image-builder-cli/internal/blueprintload"
"github.com/osbuild/image-builder-cli/internal/olog"
) )
var ( var (
@ -409,13 +410,8 @@ func cmdDescribeImg(cmd *cobra.Command, args []string) error {
} }
func run() error { func run() error {
// images generates a lot of noisy logs a bunch of stuff to // Initialize console logger (stderr, no prefix)
// Debug/Info that is distracting the user (at least by log.SetFlags(0)
// default, like what repos being loaded)
//
// Disable for now until we can filter out the usless log
// messages.
logrus.SetOutput(io.Discard)
rootCmd := &cobra.Command{ rootCmd := &cobra.Command{
Use: "image-builder", Use: "image-builder",
@ -435,7 +431,7 @@ operating systems like Fedora, CentOS and RHEL with easy customizations support.
rootCmd.PersistentFlags().StringArray("extra-repo", nil, `Add an extra repository during build (will *not* be gpg checked and not be part of the final image)`) rootCmd.PersistentFlags().StringArray("extra-repo", nil, `Add an extra repository during build (will *not* be gpg checked and not be part of the final image)`)
rootCmd.PersistentFlags().StringArray("force-repo", nil, `Override the base repositories during build (these will not be part of the final image)`) rootCmd.PersistentFlags().StringArray("force-repo", nil, `Override the base repositories during build (these will not be part of the final image)`)
rootCmd.PersistentFlags().String("output-dir", "", `Put output into the specified directory`) rootCmd.PersistentFlags().String("output-dir", "", `Put output into the specified directory`)
rootCmd.PersistentFlags().BoolP("verbose", "v", false, `Switch to verbose mode`) rootCmd.PersistentFlags().BoolP("verbose", "v", false, `Switch to verbose mode (more logging on stderr and verbose progress)`)
rootCmd.SetOut(osStdout) rootCmd.SetOut(osStdout)
rootCmd.SetErr(osStderr) rootCmd.SetErr(osStderr)
@ -520,12 +516,21 @@ operating systems like Fedora, CentOS and RHEL with easy customizations support.
rootCmd.AddCommand(describeImgCmd) rootCmd.AddCommand(describeImgCmd)
verbose, err := rootCmd.PersistentFlags().GetBool("verbose")
if err != nil {
return err
}
if verbose {
olog.SetDefault(log.New(os.Stderr, "", 0))
// XXX: add once images has olog support
//images_log.SetDefault(log.New(os.Stderr, "", 0))
}
return rootCmd.Execute() return rootCmd.Execute()
} }
func main() { func main() {
if err := run(); err != nil { if err := run(); err != nil {
fmt.Fprintf(osStderr, "error: %s\n", err) log.Fatalf("error: %s\n", err)
os.Exit(1)
} }
} }

View file

@ -14,7 +14,6 @@ import (
"strings" "strings"
"testing" "testing"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -30,11 +29,6 @@ import (
"github.com/osbuild/images/pkg/arch" "github.com/osbuild/images/pkg/arch"
) )
func init() {
// silence logrus by default, it is quite verbose
logrus.SetLevel(logrus.WarnLevel)
}
func TestListImagesNoArguments(t *testing.T) { func TestListImagesNoArguments(t *testing.T) {
restore := main.MockNewRepoRegistry(testrepos.New) restore := main.MockNewRepoRegistry(testrepos.New)
defer restore() defer restore()

8
internal/olog/package.go Normal file
View file

@ -0,0 +1,8 @@
// Package olog provides a simple wrapper around the standard log package
// variable so logging statements are shorter. Instead of pkg.Logger.Printf one
// can simply write olog.Printf.
//
// The name stands for "optional logger" because the output can be disabled by
// setting the logger to nil or providing io.Discard as the output. This is
// useful for user-configurable logging (e.g. --verbose flags for CLI apps).
package olog

83
internal/olog/proxy.go Normal file
View file

@ -0,0 +1,83 @@
package olog
import "io"
// Print is a proxy for log.Print.
func Print(v ...any) {
Default().Print(v...)
}
// Printf is a proxy for log.Printf.
func Printf(format string, v ...any) {
Default().Printf(format, v...)
}
// Println is a proxy for log.Println.
func Println(v ...any) {
Default().Println(v...)
}
// Fatal is a proxy for log.Fatal.
func Fatal(v ...any) {
Default().Fatal(v...)
}
// Fatalf is a proxy for log.Fatalf.
func Fatalf(format string, v ...any) {
Default().Fatalf(format, v...)
}
// Fatalln is a proxy for log.Fatalln.
func Fatalln(v ...any) {
Default().Fatalln(v...)
}
// Panic is a proxy for log.Panic.
func Panic(v ...any) {
Default().Panic(v...)
}
// Panicf is a proxy for log.Panicf.
func Panicf(format string, v ...any) {
Default().Panicf(format, v...)
}
// Panicln is a proxy for log.Panicln.
func Panicln(v ...any) {
Default().Panicln(v...)
}
// Writer is a proxy for log.Writer.
func Writer() io.Writer {
return Default().Writer()
}
// Output is a proxy for log.Output.
func Output(calldepth int, s string) error {
return Default().Output(calldepth+1, s)
}
// SetOutput is a proxy for log.SetOutput.
func SetOutput(w io.Writer) {
Default().SetOutput(w)
}
// Flags is a proxy for log.Flags.
func Flags() int {
return Default().Flags()
}
// SetFlags is a proxy for log.SetFlags.
func SetFlags(flag int) {
Default().SetFlags(flag)
}
// Prefix is a proxy for log.Prefix.
func Prefix() string {
return Default().Prefix()
}
// SetPrefix is a proxy for log.SetPrefix.
func SetPrefix(prefix string) {
Default().SetPrefix(prefix)
}

30
internal/olog/var.go Normal file
View file

@ -0,0 +1,30 @@
package olog
import (
"io"
"log"
"sync/atomic"
)
var logger atomic.Pointer[log.Logger]
func init() {
SetDefault(nil)
}
// SetDefault sets the default logger to the provided logger. When nil is passed,
// the default logger is set to a no-op logger that discards all log messages.
func SetDefault(l *log.Logger) {
if l == nil {
logger.Store(log.New(io.Discard, "", 0))
return
}
logger.Store(l)
}
// Default returns the default logger. If no logger has been set, it returns a
// no-op logger that discards all log messages.
func Default() *log.Logger {
return logger.Load()
}

View file

@ -5,6 +5,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"log"
"os" "os"
"os/exec" "os/exec"
"strings" "strings"
@ -14,7 +15,6 @@ import (
"github.com/cheggaaa/pb/v3" "github.com/cheggaaa/pb/v3"
"github.com/mattn/go-isatty" "github.com/mattn/go-isatty"
"github.com/sirupsen/logrus"
"github.com/osbuild/images/pkg/datasizes" "github.com/osbuild/images/pkg/datasizes"
"github.com/osbuild/images/pkg/osbuild" "github.com/osbuild/images/pkg/osbuild"
@ -241,7 +241,7 @@ func (b *terminalProgressBar) Stop() {
// I cannot think of how this could happen, i.e. why // I cannot think of how this could happen, i.e. why
// closing would not work but lets be conservative - // closing would not work but lets be conservative -
// without a timeout we hang here forever // without a timeout we hang here forever
logrus.Warnf("no progress channel shutdown after 1sec") log.Printf("WARNING: no progress channel shutdown after 1sec")
} }
b.shutdownCh = nil b.shutdownCh = nil
// This should never happen but be paranoid, this should // This should never happen but be paranoid, this should
@ -479,7 +479,7 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, o
i := 0 i := 0
for p := st.Progress; p != nil; p = p.SubProgress { for p := st.Progress; p != nil; p = p.SubProgress {
if err := pb.SetProgress(i, p.Message, p.Done, p.Total); err != nil { if err := pb.SetProgress(i, p.Message, p.Done, p.Total); err != nil {
logrus.Warnf("cannot set progress: %v", err) log.Printf("WARNING: cannot set progress: %v", err)
} }
i++ i++
} }

View file

@ -2,6 +2,7 @@ package setup
import ( import (
"fmt" "fmt"
"log"
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
@ -10,8 +11,6 @@ import (
"golang.org/x/sys/unix" "golang.org/x/sys/unix"
"github.com/sirupsen/logrus"
"github.com/osbuild/image-builder-cli/pkg/podmanutil" "github.com/osbuild/image-builder-cli/pkg/podmanutil"
"github.com/osbuild/image-builder-cli/pkg/util" "github.com/osbuild/image-builder-cli/pkg/util"
) )
@ -136,7 +135,7 @@ func validateCanRunTargetArch(targetArch string) error {
// we could error here but in principle with a working qemu-user // we could error here but in principle with a working qemu-user
// any arch should work so let's just warn. the common case // any arch should work so let's just warn. the common case
// (arm64/amd64) is covered properly // (arm64/amd64) is covered properly
logrus.Warningf("cannot check architecture support for %v: no canary binary found", targetArch) log.Printf("WARNING: cannot check architecture support for %v: no canary binary found", targetArch)
return nil return nil
} }
output, err := exec.Command(canaryCmd).CombinedOutput() output, err := exec.Command(canaryCmd).CombinedOutput()

View file

@ -3,12 +3,12 @@ package setup_test
import ( import (
"bytes" "bytes"
"fmt" "fmt"
"log"
"os" "os"
"path/filepath" "path/filepath"
"runtime" "runtime"
"testing" "testing"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/osbuild/image-builder-cli/pkg/setup" "github.com/osbuild/image-builder-cli/pkg/setup"
@ -23,11 +23,13 @@ func TestValidateCanRunTargetArchTrivial(t *testing.T) {
func TestValidateCanRunTargetArchUnsupportedCanary(t *testing.T) { func TestValidateCanRunTargetArchUnsupportedCanary(t *testing.T) {
var logbuf bytes.Buffer var logbuf bytes.Buffer
logrus.SetOutput(&logbuf) ow := log.Writer()
log.SetOutput(&logbuf)
defer log.SetOutput(ow)
err := setup.ValidateCanRunTargetArch("unsupported-arch") err := setup.ValidateCanRunTargetArch("unsupported-arch")
assert.NoError(t, err) assert.NoError(t, err)
assert.Contains(t, logbuf.String(), `level=warning msg="cannot check architecture support for unsupported-arch: no canary binary found"`) assert.Contains(t, logbuf.String(), `cannot check architecture support for unsupported-arch: no canary binary found`)
} }
func makeFakeBinary(t *testing.T, binary, content string) { func makeFakeBinary(t *testing.T, binary, content string) {
@ -43,7 +45,9 @@ func makeFakeCanary(t *testing.T, content string) {
func TestValidateCanRunTargetArchHappy(t *testing.T) { func TestValidateCanRunTargetArchHappy(t *testing.T) {
var logbuf bytes.Buffer var logbuf bytes.Buffer
logrus.SetOutput(&logbuf) ow := log.Writer()
log.SetOutput(&logbuf)
defer log.SetOutput(ow)
makeFakeCanary(t, "#!/bin/sh\necho ok") makeFakeCanary(t, "#!/bin/sh\necho ok")

View file

@ -6,7 +6,7 @@ import (
"os/exec" "os/exec"
"strings" "strings"
"github.com/sirupsen/logrus" "github.com/osbuild/image-builder-cli/internal/olog"
) )
// IsMountpoint checks if the target path is a mount point // IsMountpoint checks if the target path is a mount point
@ -17,7 +17,7 @@ func IsMountpoint(path string) bool {
// Synchronously invoke a command, propagating stdout and stderr // Synchronously invoke a command, propagating stdout and stderr
// to the current process's stdout and stderr // to the current process's stdout and stderr
func RunCmdSync(cmdName string, args ...string) error { func RunCmdSync(cmdName string, args ...string) error {
logrus.Debugf("Running: %s %s", cmdName, strings.Join(args, " ")) olog.Printf("Running: %s %s", cmdName, strings.Join(args, " "))
cmd := exec.Command(cmdName, args...) cmd := exec.Command(cmdName, args...)
cmd.Stdout = os.Stdout cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr cmd.Stderr = os.Stderr