More logging and refactor (#457)

* only use "context"
* enable 'h2' support at server
* trace log  remote and database config
* log loglevel on start

cmd/agent/agent.go

@@ -25,7 +25,6 @@ import (
 	"github.com/rs/zerolog/log"
 	"github.com/tevino/abool"
 	"github.com/urfave/cli"
-	oldcontext "golang.org/x/net/context"
 	"google.golang.org/grpc"
 	grpccredentials "google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/keepalive"
@@ -160,7 +159,7 @@ type credentials struct {
 	password string
 }
 
-func (c *credentials) GetRequestMetadata(oldcontext.Context, ...string) (map[string]string, error) {
+func (c *credentials) GetRequestMetadata(context.Context, ...string) (map[string]string, error) {
 	return map[string]string{
 		"username": c.username,
 		"password": c.password,

cmd/server/server.go

@@ -31,7 +31,6 @@ import (
 	"github.com/rs/zerolog/log"
 	"github.com/urfave/cli"
 	"golang.org/x/crypto/acme/autocert"
-	oldcontext "golang.org/x/net/context"
 	"golang.org/x/sync/errgroup"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/keepalive"
@@ -68,7 +67,6 @@ func loop(c *cli.Context) error {
 		log.Warn().Msg("--debug is deprecated, use --log-level instead")
 		zerolog.SetGlobalLevel(zerolog.DebugLevel)
 	}
-
 	if c.IsSet("log-level") {
 		logLevelFlag := c.String("log-level")
 		lvl, err := zerolog.ParseLevel(logLevelFlag)
@@ -77,6 +75,7 @@ func loop(c *cli.Context) error {
 		}
 		zerolog.SetGlobalLevel(lvl)
 	}
+	log.Log().Msgf("LogLevel = %s", zerolog.GlobalLevel().String())
 
 	if c.String("server-host") == "" {
 		log.Fatal().Msg("WOODPECKER_HOST is not properly configured")
@@ -105,7 +104,11 @@ func loop(c *cli.Context) error {
 		log.Fatal().Err(err).Msg("")
 	}
 
-	store_ := setupStore(c)
+	store_, err := setupStore(c)
+	if err != nil {
+		log.Fatal().Err(err).Msg("")
+	}
+
 	setupEvilGlobals(c, store_, remote_)
 
 	proxyWebUI := c.String("www-proxy")
@@ -189,7 +192,7 @@ func loop(c *cli.Context) error {
 				Addr:    ":https",
 				Handler: handler,
 				TLSConfig: &tls.Config{
-					NextProtos: []string{"http/1.1"}, // disable h2 because Safari :(
+					NextProtos: []string{"h2", "http/1.1"},
 				},
 			}
 			return serve.ListenAndServeTLS(
@@ -232,7 +235,7 @@ func loop(c *cli.Context) error {
 			Handler: handler,
 			TLSConfig: &tls.Config{
 				GetCertificate: manager.GetCertificate,
-				NextProtos:     []string{"http/1.1"}, // disable h2 because Safari :(
+				NextProtos:     []string{"h2", "http/1.1"},
 			},
 		}
 		return serve.ListenAndServeTLS("", "")
@@ -296,7 +299,7 @@ func (a *authorizer) streamInterceptor(srv interface{}, stream grpc.ServerStream
 	return handler(srv, stream)
 }
 
-func (a *authorizer) unaryIntercaptor(ctx oldcontext.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) {
+func (a *authorizer) unaryIntercaptor(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) {
 	if err := a.authorize(ctx); err != nil {
 		return nil, err
 	}

cmd/server/setup.go

@@ -44,11 +44,13 @@ import (
 	"github.com/woodpecker-ci/woodpecker/server/web"
 )
 
-func setupStore(c *cli.Context) store.Store {
+func setupStore(c *cli.Context) (store.Store, error) {
-	return datastore.New(
+	opts := &datastore.Opts{
-		c.String("driver"),
+		Driver: c.String("driver"),
-		c.String("datasource"),
+		Config: c.String("datasource"),
-	)
+	}
+	log.Trace().Msgf("setup datastore: %#v", opts)
+	return datastore.New(opts)
 }
 
 func setupQueue(c *cli.Context, s store.Store) queue.Queue {
@@ -98,21 +100,25 @@ func SetupRemote(c *cli.Context) (remote.Remote, error) {
 
 // helper function to setup the Bitbucket remote from the CLI arguments.
 func setupBitbucket(c *cli.Context) (remote.Remote, error) {
-	return bitbucket.New(
+	opts := &bitbucket.Opts{
-		c.String("bitbucket-client"),
+		Client: c.String("bitbucket-client"),
-		c.String("bitbucket-secret"),
+		Secret: c.String("bitbucket-secret"),
-	), nil
+	}
+	log.Trace().Msgf("Remote (bitbucket) opts: %#v", opts)
+	return bitbucket.New(opts)
 }
 
 // helper function to setup the Gogs remote from the CLI arguments.
 func setupGogs(c *cli.Context) (remote.Remote, error) {
-	return gogs.New(gogs.Opts{
+	opts := gogs.Opts{
 		URL:         c.String("gogs-server"),
 		Username:    c.String("gogs-git-username"),
 		Password:    c.String("gogs-git-password"),
 		PrivateMode: c.Bool("gogs-private-mode"),
 		SkipVerify:  c.Bool("gogs-skip-verify"),
-	})
+	}
+	log.Trace().Msgf("Remote (gogs) opts: %#v", opts)
+	return gogs.New(opts)
 }
 
 // helper function to setup the Gitea remote from the CLI arguments.
@@ -130,12 +136,13 @@ func setupGitea(c *cli.Context) (remote.Remote, error) {
 	if len(opts.URL) == 0 {
 		log.Fatal().Msg("WOODPECKER_GITEA_URL must be set")
 	}
+	log.Trace().Msgf("Remote (gitea) opts: %#v", opts)
 	return gitea.New(opts)
 }
 
 // helper function to setup the Stash remote from the CLI arguments.
 func setupStash(c *cli.Context) (remote.Remote, error) {
-	return bitbucketserver.New(bitbucketserver.Opts{
+	opts := bitbucketserver.Opts{
 		URL:               c.String("stash-server"),
 		Username:          c.String("stash-git-username"),
 		Password:          c.String("stash-git-password"),
@@ -143,7 +150,9 @@ func setupStash(c *cli.Context) (remote.Remote, error) {
 		ConsumerRSA:       c.String("stash-consumer-rsa"),
 		ConsumerRSAString: c.String("stash-consumer-rsa-string"),
 		SkipVerify:        c.Bool("stash-skip-verify"),
-	})
+	}
+	log.Trace().Msgf("Remote (bitbucketserver) opts: %#v", opts)
+	return bitbucketserver.New(opts)
 }
 
 // helper function to setup the Gitlab remote from the CLI arguments.
@@ -161,7 +170,7 @@ func setupGitlab(c *cli.Context) (remote.Remote, error) {
 
 // helper function to setup the GitHub remote from the CLI arguments.
 func setupGithub(c *cli.Context) (remote.Remote, error) {
-	return github.New(github.Opts{
+	opts := github.Opts{
 		URL:         c.String("github-server"),
 		Context:     c.String("github-context"),
 		Client:      c.String("github-client"),
@@ -172,12 +181,14 @@ func setupGithub(c *cli.Context) (remote.Remote, error) {
 		PrivateMode: c.Bool("github-private-mode"),
 		SkipVerify:  c.Bool("github-skip-verify"),
 		MergeRef:    c.BoolT("github-merge-ref"),
-	})
+	}
+	log.Trace().Msgf("Remote (github) opts: %#v", opts)
+	return github.New(opts)
 }
 
 // helper function to setup the Coding remote from the CLI arguments.
 func setupCoding(c *cli.Context) (remote.Remote, error) {
-	return coding.New(coding.Opts{
+	opts := coding.Opts{
 		URL:        c.String("coding-server"),
 		Client:     c.String("coding-client"),
 		Secret:     c.String("coding-secret"),
@@ -186,7 +197,9 @@ func setupCoding(c *cli.Context) (remote.Remote, error) {
 		Username:   c.String("coding-git-username"),
 		Password:   c.String("coding-git-password"),
 		SkipVerify: c.Bool("coding-skip-verify"),
-	})
+	}
+	log.Trace().Msgf("Remote (coding) opts: %#v", opts)
+	return coding.New(opts)
 }
 
 func setupTree(c *cli.Context) *gin.Engine {

server/grpc/rpc.go

@@ -28,7 +28,6 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
-	oldcontext "golang.org/x/net/context"
 	grpcMetadata "google.golang.org/grpc/metadata"
 
 	"github.com/woodpecker-ci/expr"
@@ -505,7 +504,7 @@ func NewWoodpeckerServer(remote remote.Remote, queue queue.Queue, logger logging
 	return &WoodpeckerServer{peer: peer}
 }
 
-func (s *WoodpeckerServer) Next(c oldcontext.Context, req *proto.NextRequest) (*proto.NextReply, error) {
+func (s *WoodpeckerServer) Next(c context.Context, req *proto.NextRequest) (*proto.NextReply, error) {
 	filter := rpc.Filter{
 		Labels: req.GetFilter().GetLabels(),
 		Expr:   req.GetFilter().GetExpr(),
@@ -528,7 +527,7 @@ func (s *WoodpeckerServer) Next(c oldcontext.Context, req *proto.NextRequest) (*
 	return res, err
 }
 
-func (s *WoodpeckerServer) Init(c oldcontext.Context, req *proto.InitRequest) (*proto.Empty, error) {
+func (s *WoodpeckerServer) Init(c context.Context, req *proto.InitRequest) (*proto.Empty, error) {
 	state := rpc.State{
 		Error:    req.GetState().GetError(),
 		ExitCode: int(req.GetState().GetExitCode()),
@@ -542,7 +541,7 @@ func (s *WoodpeckerServer) Init(c oldcontext.Context, req *proto.InitRequest) (*
 	return res, err
 }
 
-func (s *WoodpeckerServer) Update(c oldcontext.Context, req *proto.UpdateRequest) (*proto.Empty, error) {
+func (s *WoodpeckerServer) Update(c context.Context, req *proto.UpdateRequest) (*proto.Empty, error) {
 	state := rpc.State{
 		Error:    req.GetState().GetError(),
 		ExitCode: int(req.GetState().GetExitCode()),
@@ -556,7 +555,7 @@ func (s *WoodpeckerServer) Update(c oldcontext.Context, req *proto.UpdateRequest
 	return res, err
 }
 
-func (s *WoodpeckerServer) Upload(c oldcontext.Context, req *proto.UploadRequest) (*proto.Empty, error) {
+func (s *WoodpeckerServer) Upload(c context.Context, req *proto.UploadRequest) (*proto.Empty, error) {
 	file := &rpc.File{
 		Data: req.GetFile().GetData(),
 		Mime: req.GetFile().GetMime(),
@@ -572,7 +571,7 @@ func (s *WoodpeckerServer) Upload(c oldcontext.Context, req *proto.UploadRequest
 	return res, err
 }
 
-func (s *WoodpeckerServer) Done(c oldcontext.Context, req *proto.DoneRequest) (*proto.Empty, error) {
+func (s *WoodpeckerServer) Done(c context.Context, req *proto.DoneRequest) (*proto.Empty, error) {
 	state := rpc.State{
 		Error:    req.GetState().GetError(),
 		ExitCode: int(req.GetState().GetExitCode()),
@@ -586,19 +585,19 @@ func (s *WoodpeckerServer) Done(c oldcontext.Context, req *proto.DoneRequest) (*
 	return res, err
 }
 
-func (s *WoodpeckerServer) Wait(c oldcontext.Context, req *proto.WaitRequest) (*proto.Empty, error) {
+func (s *WoodpeckerServer) Wait(c context.Context, req *proto.WaitRequest) (*proto.Empty, error) {
 	res := new(proto.Empty)
 	err := s.peer.Wait(c, req.GetId())
 	return res, err
 }
 
-func (s *WoodpeckerServer) Extend(c oldcontext.Context, req *proto.ExtendRequest) (*proto.Empty, error) {
+func (s *WoodpeckerServer) Extend(c context.Context, req *proto.ExtendRequest) (*proto.Empty, error) {
 	res := new(proto.Empty)
 	err := s.peer.Extend(c, req.GetId())
 	return res, err
 }
 
-func (s *WoodpeckerServer) Log(c oldcontext.Context, req *proto.LogRequest) (*proto.Empty, error) {
+func (s *WoodpeckerServer) Log(c context.Context, req *proto.LogRequest) (*proto.Empty, error) {
 	line := &rpc.Line{
 		Out:  req.GetLine().GetOut(),
 		Pos:  int(req.GetLine().GetPos()),

server/remote/bitbucket/bitbucket.go

@@ -34,6 +34,12 @@ const (
 	DefaultURL = "https://bitbucket.org"
 )
 
+// Opts are remote options for bitbucket
+type Opts struct {
+	Client string
+	Secret string
+}
+
 type config struct {
 	API    string
 	URL    string
@@ -43,13 +49,14 @@ type config struct {
 
 // New returns a new remote Configuration for integrating with the Bitbucket
 // repository hosting service at https://bitbucket.org
-func New(client, secret string) remote.Remote {
+func New(opts *Opts) (remote.Remote, error) {
 	return &config{
 		API:    DefaultAPI,
 		URL:    DefaultURL,
-		Client: client,
+		Client: opts.Client,
-		Secret: secret,
+		Secret: opts.Secret,
-	}
+	}, nil
+	// TODO: add checks
 }
 
 // Login authenticates an account with Bitbucket using the oauth2 protocol. The

server/remote/bitbucket/bitbucket_test.go

@@ -44,7 +44,7 @@ func Test_bitbucket(t *testing.T) {
 		})
 
 		g.It("Should return client with default endpoint", func() {
-			remote := New("4vyW6b49Z", "a5012f6c6")
+			remote, _ := New(&Opts{Client: "4vyW6b49Z", Secret: "a5012f6c6"})
 			g.Assert(remote.(*config).URL).Equal(DefaultURL)
 			g.Assert(remote.(*config).API).Equal(DefaultAPI)
 			g.Assert(remote.(*config).Client).Equal("4vyW6b49Z")
@@ -52,7 +52,7 @@ func Test_bitbucket(t *testing.T) {
 		})
 
 		g.It("Should return the netrc file", func() {
-			remote := New("", "")
+			remote, _ := New(&Opts{})
 			netrc, _ := remote.Netrc(fakeUser, nil)
 			g.Assert(netrc.Machine).Equal("bitbucket.org")
 			g.Assert(netrc.Login).Equal("x-token-auth")

server/remote/context.go

@@ -15,7 +15,7 @@
 package remote
 
 import (
-	"golang.org/x/net/context"
+	"context"
 )
 
 const key = "remote"

server/shared/configFetcher.go

@@ -39,6 +39,7 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*remote.FileMeta, e
 	// try to fetch 3 times, timeout is one second longer each time
 	for i := 0; i < 3; i++ {
 		files, err = cf.fetch(ctx, time.Second*time.Duration(configFetchTimeout), strings.TrimSpace(cf.repo.Config))
+		log.Trace().Msgf("%d try failed: %v", i, err)
 		if errors.Is(err, context.DeadlineExceeded) {
 			continue
 		}

server/store/context.go

@@ -15,7 +15,7 @@
 package store
 
 import (
-	"golang.org/x/net/context"
+	"context"
 )
 
 const key = "store"

server/store/datastore/store.go

@@ -16,7 +16,7 @@ package datastore
 
 import (
 	"database/sql"
-	"os"
+	"fmt"
 	"time"
 
 	"github.com/rs/zerolog/log"
@@ -35,14 +35,21 @@ type datastore struct {
 	config string
 }
 
+// Opts are options for a new database connection
+type Opts struct {
+	Driver string
+	Config string
+}
+
 // New creates a database connection for the given driver and datasource
 // and returns a new Store.
-func New(driver, config string) store.Store {
+func New(opts *Opts) (store.Store, error) {
+	db, err := open(opts.Driver, opts.Config)
 	return &datastore{
-		DB:     open(driver, config),
+		DB:     db,
-		driver: driver,
+		driver: opts.Driver,
-		config: config,
+		config: opts.Config,
-	}
+	}, err
 }
 
 // From returns a Store using an existing database connection.
@@ -52,10 +59,10 @@ func From(db *sql.DB) store.Store {
 
 // open opens a new database connection with the specified
 // driver and connection string and returns a store.
-func open(driver, config string) *sql.DB {
+func open(driver, config string) (*sql.DB, error) {
 	db, err := sql.Open(driver, config)
 	if err != nil {
-		log.Fatal().Err(err).Msg("database connection failed")
+		return nil, fmt.Errorf("database connection failed: %v", err)
 	}
 	if driver == "mysql" {
 		// per issue https://github.com/go-sql-driver/mysql/issues/257
@@ -65,47 +72,13 @@ func open(driver, config string) *sql.DB {
 	setupMeddler(driver)
 
 	if err := pingDatabase(db); err != nil {
-		log.Fatal().Err(err).Msg("database ping attempts failed")
+		return nil, fmt.Errorf("database ping attempts failed: %v", err)
 	}
 
 	if err := setupDatabase(driver, db); err != nil {
-		log.Fatal().Err(err).Msg("migration failed")
+		return nil, fmt.Errorf("database migration failed: %v", err)
-	}
-	return db
-}
-
-// openTest opens a new database connection for testing purposes.
-// The database driver and connection string are provided by
-// environment variables, with fallback to in-memory sqlite.
-func openTest() *sql.DB {
-	var (
-		driver = "sqlite3"
-		config = ":memory:"
-	)
-	if os.Getenv("WOODPECKER_DATABASE_DRIVER") != "" {
-		driver = os.Getenv("WOODPECKER_DATABASE_DRIVER")
-		config = os.Getenv("WOODPECKER_DATABASE_CONFIG")
-	}
-	return open(driver, config)
-}
-
-// newTest creates a new database connection for testing purposes.
-// The database driver and connection string are provided by
-// environment variables, with fallback to in-memory sqlite.
-func newTest() *datastore {
-	var (
-		driver = "sqlite3"
-		config = ":memory:"
-	)
-	if os.Getenv("WOODPECKER_DATABASE_DRIVER") != "" {
-		driver = os.Getenv("WOODPECKER_DATABASE_DRIVER")
-		config = os.Getenv("WOODPECKER_DATABASE_CONFIG")
-	}
-	return &datastore{
-		DB:     open(driver, config),
-		driver: driver,
-		config: config,
 	}
+	return db, nil
 }
 
 // helper function to ping the database with backoff to ensure

server/store/datastore/store_test.go

@@ -0,0 +1,52 @@
+// Copyright 2021 Woodpecker Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package datastore
+
+import (
+	"database/sql"
+	"os"
+)
+
+func testDriverConfig() (driver, config string) {
+	driver = "sqlite3"
+	config = ":memory:"
+
+	if os.Getenv("WOODPECKER_DATABASE_DRIVER") != "" {
+		driver = os.Getenv("WOODPECKER_DATABASE_DRIVER")
+		config = os.Getenv("WOODPECKER_DATABASE_CONFIG")
+	}
+
+	return
+}
+
+// openTest opens a new database connection for testing purposes.
+// The database driver and connection string are provided by
+// environment variables, with fallback to in-memory sqlite.
+func openTest() *sql.DB {
+	db, _ := open(testDriverConfig())
+	return db
+}
+
+// newTest creates a new database connection for testing purposes.
+// The database driver and connection string are provided by
+// environment variables, with fallback to in-memory sqlite.
+func newTest() *datastore {
+	driver, config := testDriverConfig()
+	return &datastore{
+		DB:     openTest(),
+		driver: driver,
+		config: config,
+	}
+}

server/store/store.go

@@ -15,10 +15,9 @@
 package store
 
 import (
+	"context"
 	"io"
 
-	"golang.org/x/net/context"
-
 	"github.com/woodpecker-ci/woodpecker/server/model"
 )