Many: factor out logger implementation from upload/koji

The upload/koji package functions were creating a logger and then were
using it. This is not ideal for a library implementation.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>

cmd/osbuild-koji-tests/main_test.go

@@ -53,7 +53,7 @@ func TestKojiRefund(t *testing.T) {
 		Principal: "osbuild-krb@LOCAL",
 		KeyTab:    shareDir + "/client.keytab",
 	}
-	k, err := koji.NewFromGSSAPI(server, credentials, transport)
+	k, err := koji.NewFromGSSAPI(server, credentials, transport, nil)
 	require.NoError(t, err)
 
 	defer func() {
@@ -114,7 +114,7 @@ func TestKojiImport(t *testing.T) {
 		Principal: "osbuild-krb@LOCAL",
 		KeyTab:    shareDir + "/client.keytab",
 	}
-	k, err := koji.NewFromGSSAPI(server, credentials, transport)
+	k, err := koji.NewFromGSSAPI(server, credentials, transport, nil)
 	require.NoError(t, err)
 
 	defer func() {

cmd/osbuild-koji/main.go

@@ -42,8 +42,8 @@ func main() {
 	}
 	defer file.Close()
 
-	transport := koji.CreateRetryableTransport()
+	transport := koji.CreateRetryableTransport(nil)
-	k, err := koji.NewFromPlain(server, "osbuild", "osbuildpass", transport)
+	k, err := koji.NewFromPlain(server, "osbuild", "osbuildpass", transport, nil)
 	if err != nil {
 		println(err.Error())
 		return

cmd/osbuild-worker/jobimpl-koji-finalize.go

@@ -37,8 +37,8 @@ func (impl *KojiFinalizeJobImpl) kojiImport(
 		return fmt.Errorf("Koji server has not been configured: %s", serverURL.Hostname())
 	}
 
-	transport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor)
+	transport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor, NewRHLeveledLogger(nil))
-	k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport)
+	k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport, NewRHLeveledLogger(nil))
 	if err != nil {
 		return err
 	}
@@ -69,8 +69,8 @@ func (impl *KojiFinalizeJobImpl) kojiFail(server string, buildID int, token stri
 		return fmt.Errorf("Koji server has not been configured: %s", serverURL.Hostname())
 	}
 
-	transport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor)
+	transport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor, NewRHLeveledLogger(nil))
-	k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport)
+	k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport, NewRHLeveledLogger(nil))
 	if err != nil {
 		return err
 	}

cmd/osbuild-worker/jobimpl-koji-init.go

@@ -27,8 +27,8 @@ func (impl *KojiInitJobImpl) kojiInit(server, name, version, release string) (st
 		return "", 0, fmt.Errorf("Koji server has not been configured: %s", serverURL.Hostname())
 	}
 
-	transport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor)
+	transport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor, NewRHLeveledLogger(nil))
-	k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport)
+	k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport, NewRHLeveledLogger(nil))
 	if err != nil {
 		return "", 0, err
 	}

cmd/osbuild-worker/jobimpl-osbuild.go

@@ -1012,9 +1012,9 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error {
 				break
 			}
 
-			kojiTransport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor)
+			kojiTransport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor, NewRHLeveledLogger(nil))
 
-			kojiAPI, err := koji.NewFromGSSAPI(targetOptions.Server, &kojiServer.creds, kojiTransport)
+			kojiAPI, err := koji.NewFromGSSAPI(targetOptions.Server, &kojiServer.creds, kojiTransport, NewRHLeveledLogger(nil))
 			if err != nil {
 				logWithId.Warnf("[Koji] 🔑 login failed: %v", err) // DON'T EDIT: Used for Splunk dashboard
 				targetResult.TargetError = clienterrors.New(clienterrors.ErrorInvalidTargetConfig, fmt.Sprintf("failed to authenticate with Koji server %q: %v", kojiServerURL.Hostname(), err), nil)

cmd/osbuild-worker/rh-logrus-adapter.go

@@ -1,8 +1,9 @@
-package koji
+package main
 
 import (
 	"strings"
 
+	rh "github.com/hashicorp/go-retryablehttp"
 	"github.com/sirupsen/logrus"
 )
 
@@ -10,6 +11,13 @@ type LeveledLogrus struct {
 	*logrus.Logger
 }
 
+func NewRHLeveledLogger(logger *logrus.Logger) rh.LeveledLogger {
+	if logger == nil {
+		logger = logrus.StandardLogger()
+	}
+	return rh.LeveledLogger(&LeveledLogrus{logger})
+}
+
 const monitoringKeyword = "retrying"
 
 func fields(keysAndValues ...interface{}) map[string]interface{} {

internal/upload/koji/koji.go

@@ -23,7 +23,6 @@ import (
 
 	rh "github.com/hashicorp/go-retryablehttp"
 	"github.com/kolo/xmlrpc"
-	"github.com/sirupsen/logrus"
 	"github.com/ubccr/kerby/khttp"
 
 	"github.com/osbuild/images/pkg/rpmmd"
@@ -34,6 +33,7 @@ type Koji struct {
 	xmlrpc    *xmlrpc.Client
 	server    string
 	transport http.RoundTripper
+	logger    rh.LeveledLogger
 }
 
 // BUILD METADATA
@@ -242,7 +242,7 @@ type loginReply struct {
 	SessionKey string `xmlrpc:"session-key"`
 }
 
-func newKoji(server string, transport http.RoundTripper, reply loginReply) (*Koji, error) {
+func newKoji(server string, transport http.RoundTripper, reply loginReply, logger rh.LeveledLogger) (*Koji, error) {
 	// Create the final xmlrpc client with our custom RoundTripper handling
 	// sessionID, sessionKey and callnum
 	kojiTransport := &Transport{
@@ -261,13 +261,14 @@ func newKoji(server string, transport http.RoundTripper, reply loginReply) (*Koj
 		xmlrpc:    client,
 		server:    server,
 		transport: kojiTransport,
+		logger:    logger,
 	}, nil
 }
 
 // NewFromPlain creates a new Koji sessions  =authenticated using the plain
 // username/password method. If you want to speak to a public koji instance,
 // you probably cannot use this method.
-func NewFromPlain(server, user, password string, transport http.RoundTripper) (*Koji, error) {
+func NewFromPlain(server, user, password string, transport http.RoundTripper, logger rh.LeveledLogger) (*Koji, error) {
 	// Create a temporary xmlrpc client.
 	// The API doesn't require sessionID, sessionKey and callnum yet,
 	// so there's no need to use the custom Koji RoundTripper,
@@ -284,13 +285,17 @@ func NewFromPlain(server, user, password string, transport http.RoundTripper) (*
 		return nil, err
 	}
 
-	return newKoji(server, transport, reply)
+	return newKoji(server, transport, reply, logger)
 }
 
 // NewFromGSSAPI creates a new Koji session authenticated using GSSAPI.
 // Principal and keytab used for the session is passed using credentials
 // parameter.
-func NewFromGSSAPI(server string, credentials *GSSAPICredentials, transport http.RoundTripper) (*Koji, error) {
+func NewFromGSSAPI(
+	server string,
+	credentials *GSSAPICredentials,
+	transport http.RoundTripper,
+	logger rh.LeveledLogger) (*Koji, error) {
 	// Create a temporary xmlrpc client with kerberos transport.
 	// The API doesn't require sessionID, sessionKey and callnum yet,
 	// so there's no need to use the custom Koji RoundTripper,
@@ -310,7 +315,7 @@ func NewFromGSSAPI(server string, credentials *GSSAPICredentials, transport http
 		return nil, err
 	}
 
-	return newKoji(server, transport, reply)
+	return newKoji(server, transport, reply, logger)
 }
 
 // GetAPIVersion gets the version of the API of the remote Koji instance
@@ -418,7 +423,9 @@ func (k *Koji) CGImport(build Build, buildRoots []BuildRoot, outputs []BuildOutp
 			return nil, err
 		}
 
-		logrus.Infof("CGImport succeeded after %d attempts", attempt+1)
+		if k.logger != nil {
+			k.logger.Info(fmt.Sprintf("CGImport succeeded after %d attempts", attempt+1))
+		}
 
 		return &result, nil
 	}
@@ -444,7 +451,7 @@ func (k *Koji) uploadChunk(chunk []byte, filepath, filename string, offset uint6
 	q.Add("overwrite", "true")
 	u.RawQuery = q.Encode()
 
-	client := createCustomRetryableClient()
+	client := createCustomRetryableClient(k.logger)
 
 	client.HTTPClient = &http.Client{
 		Transport: k.transport,
@@ -573,10 +580,10 @@ func GSSAPICredentialsFromEnv() (*GSSAPICredentials, error) {
 	}, nil
 }
 
-func CreateKojiTransport(relaxTimeout time.Duration) http.RoundTripper {
+func CreateKojiTransport(relaxTimeout time.Duration, logger rh.LeveledLogger) http.RoundTripper {
 	// Koji for some reason needs TLS renegotiation enabled.
 	// Clone the default http rt and enable renegotiation.
-	rt := CreateRetryableTransport()
+	rt := CreateRetryableTransport(logger)
 
 	transport := rt.Client.HTTPClient.Transport.(*http.Transport)
 
@@ -597,36 +604,35 @@ func CreateKojiTransport(relaxTimeout time.Duration) http.RoundTripper {
 	return rt
 }
 
-func customCheckRetry(ctx context.Context, resp *http.Response, err error) (bool, error) {
+func createCustomRetryableClient(logger rh.LeveledLogger) *rh.Client {
-	shouldRetry, retErr := rh.DefaultRetryPolicy(ctx, resp, err)
+	client := rh.NewClient()
+	client.Logger = logger
 
-	// DefaultRetryPolicy denies retrying for any certificate related error.
+	client.CheckRetry = func(ctx context.Context, resp *http.Response, err error) (bool, error) {
-	// Override it in case the error is a timeout.
+		shouldRetry, retErr := rh.DefaultRetryPolicy(ctx, resp, err)
-	if !shouldRetry && err != nil {
+
-		if v, ok := err.(*url.Error); ok {
+		// DefaultRetryPolicy denies retrying for any certificate related error.
-			if _, ok := v.Err.(x509.UnknownAuthorityError); ok {
+		// Override it in case the error is a timeout.
-				// retry if it's a timeout
+		if !shouldRetry && err != nil {
-				return strings.Contains(strings.ToLower(v.Error()), "timeout"), v
+			if v, ok := err.(*url.Error); ok {
+				if _, ok := v.Err.(x509.UnknownAuthorityError); ok {
+					// retry if it's a timeout
+					return strings.Contains(strings.ToLower(v.Error()), "timeout"), v
+				}
 			}
 		}
+
+		if logger != nil && (!shouldRetry && !(resp.StatusCode >= 200 && resp.StatusCode < 300)) {
+			logger.Info(fmt.Sprintf("Not retrying: %v", resp.Status))
+		}
+
+		return shouldRetry, retErr
 	}
-
-	if !shouldRetry && !(resp.StatusCode >= 200 && resp.StatusCode < 300) {
-		logrus.Info("Not retrying: ", resp.Status)
-	}
-
-	return shouldRetry, retErr
-}
-
-func createCustomRetryableClient() *rh.Client {
-	client := rh.NewClient()
-	client.Logger = rh.LeveledLogger(&LeveledLogrus{logrus.StandardLogger()})
-	client.CheckRetry = customCheckRetry
 	return client
 }
 
-func CreateRetryableTransport() *rh.RoundTripper {
+func CreateRetryableTransport(logger rh.LeveledLogger) *rh.RoundTripper {
 	rt := rh.RoundTripper{}
-	rt.Client = createCustomRetryableClient()
+	rt.Client = createCustomRetryableClient(logger)
 	return &rt
 }