diff --git a/cmd/osbuild-koji-tests/main_test.go b/cmd/osbuild-koji-tests/main_test.go index 3fd0fed0b..f097ad64f 100644 --- a/cmd/osbuild-koji-tests/main_test.go +++ b/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() { diff --git a/cmd/osbuild-koji/main.go b/cmd/osbuild-koji/main.go index 08b080410..0e682a036 100644 --- a/cmd/osbuild-koji/main.go +++ b/cmd/osbuild-koji/main.go @@ -42,8 +42,8 @@ func main() { } defer file.Close() - transport := koji.CreateRetryableTransport() - k, err := koji.NewFromPlain(server, "osbuild", "osbuildpass", transport) + transport := koji.CreateRetryableTransport(nil) + k, err := koji.NewFromPlain(server, "osbuild", "osbuildpass", transport, nil) if err != nil { println(err.Error()) return diff --git a/cmd/osbuild-worker/jobimpl-koji-finalize.go b/cmd/osbuild-worker/jobimpl-koji-finalize.go index c3e00c693..5f9295e50 100644 --- a/cmd/osbuild-worker/jobimpl-koji-finalize.go +++ b/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) - k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport) + transport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor, NewRHLeveledLogger(nil)) + 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) - k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport) + transport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor, NewRHLeveledLogger(nil)) + k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport, NewRHLeveledLogger(nil)) if err != nil { return err } diff --git a/cmd/osbuild-worker/jobimpl-koji-init.go b/cmd/osbuild-worker/jobimpl-koji-init.go index 08a1173af..9ffb1c495 100644 --- a/cmd/osbuild-worker/jobimpl-koji-init.go +++ b/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) - k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport) + transport := koji.CreateKojiTransport(kojiServer.relaxTimeoutFactor, NewRHLeveledLogger(nil)) + k, err := koji.NewFromGSSAPI(server, &kojiServer.creds, transport, NewRHLeveledLogger(nil)) if err != nil { return "", 0, err } diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index cf258f759..2850aeaad 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/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) diff --git a/internal/upload/koji/rh-logrus-adapter.go b/cmd/osbuild-worker/rh-logrus-adapter.go similarity index 80% rename from internal/upload/koji/rh-logrus-adapter.go rename to cmd/osbuild-worker/rh-logrus-adapter.go index 5a5c482c1..06c3392de 100644 --- a/internal/upload/koji/rh-logrus-adapter.go +++ b/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{} { diff --git a/internal/upload/koji/koji.go b/internal/upload/koji/koji.go index 656f10ae3..d4f0fb6d9 100644 --- a/internal/upload/koji/koji.go +++ b/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) { - shouldRetry, retErr := rh.DefaultRetryPolicy(ctx, resp, err) +func createCustomRetryableClient(logger rh.LeveledLogger) *rh.Client { + client := rh.NewClient() + client.Logger = logger - // DefaultRetryPolicy denies retrying for any certificate related error. - // Override it in case the error is a timeout. - if !shouldRetry && err != nil { - 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 + client.CheckRetry = func(ctx context.Context, resp *http.Response, err error) (bool, error) { + shouldRetry, retErr := rh.DefaultRetryPolicy(ctx, resp, err) + + // DefaultRetryPolicy denies retrying for any certificate related error. + // Override it in case the error is a timeout. + if !shouldRetry && err != nil { + 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 }