Improve error messages in Git repository opening to make debugging easier (#286)

Signed-off-by: liamfallon <liam.fallon@est.tech>

Author: liamfallon

pkg/externalrepo/git/git.go

@@ -143,27 +143,27 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi
 
 		r, err := initEmptyRepository(dir)
 		if err != nil {
-			return nil, fmt.Errorf("error cloning git repository %q: %w", spec.Repo, err)
+			return nil, pkgerrors.Wrapf(err, "error cloning git repository %+v, could not initialize empty repository", spec.Repo)
 		}
 
 		repo = r
 	} else if !fi.IsDir() {
 		// Internal error - corrupted cache. We will cleanup on the way out.
-		return nil, fmt.Errorf("cannot clone git repository %q: %w", spec.Repo, err)
+		return nil, pkgerrors.Wrapf(err, "error cloning git repository %+v, local cache location %q is not a directory", spec.Repo, dir)
 	} else {
 		cleanup = "" // Existing directory; do not delete it.
 
 		r, err := openRepository(dir)
 		if err != nil {
-			return nil, err
+			return nil, pkgerrors.Wrapf(err, "error cloning git repository %+v, open of repository failed in gogit", spec.Repo)
 		}
 
 		repo = r
 	}
 
 	// Create Remote
 	if err := initializeOrigin(repo, spec.Repo); err != nil {
-		return nil, fmt.Errorf("error cloning git repository %q, cannot create remote: %v", spec.Repo, err)
+		return nil, pkgerrors.Wrapf(err, "error cloning git repository %+v, cannot create remote", spec.Repo)
 	}
 
 	// NOTE: the spec.git.branch field in the Repository CRD (OpenAPI schema) is defined with
@@ -175,7 +175,7 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi
 	}
 
 	if err := util.ValidateDirectoryName(string(branch), false); err != nil {
-		return nil, fmt.Errorf("branch name %s invalid: %v", branch, err)
+		return nil, pkgerrors.Wrapf(err, "error cloning git repository %+v, branch name %q invalid", spec.Repo, branch)
 	}
 
 	repository := &gitRepository{
@@ -203,11 +203,11 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi
 	}
 
 	if err := repository.fetchRemoteRepositoryWithRetry(ctx); err != nil {
-		return nil, err
+		return nil, pkgerrors.Wrapf(err, "error cloning git repository %+v, fetch of remote repository failed", spec.Repo)
 	}
 
 	if err := repository.verifyRepository(ctx, &opts); err != nil {
-		return nil, err
+		return nil, pkgerrors.Wrapf(err, "error cloning git repository %+v, fetch of remote repository failed", spec.Repo)
 	}
 
 	cleanup = "" // Success. Keep the git directory.
@@ -654,19 +654,19 @@ func (r *gitRepository) fetchRemoteRepositoryWithRetry(ctx context.Context) erro
 			if retryNumber >= 0 {
 				err := r.fetchRemoteRepository(ctx)
 				if err != nil {
-					klog.Errorf("Fetching Remote Repository %s failed - try number %d", r.Key().Name, retryNumber)
+					klog.Errorf("Fetching Remote Repository %+v failed - try number %d", r.Key(), retryNumber)
 					time.Sleep(1 * time.Second)
-					return err
+					return pkgerrors.Wrapf(err, "fetch of remote repository %+v with retry failed on try number %d", r.Key(), retryNumber)
 				}
 				if retryNumber > 1 {
-					klog.Infof("Successfully Fetched Remote Repository %s after %d retries", r.Key(), retryNumber)
+					klog.Infof("Successfully Fetched Remote Repository %+v after %d retries", r.Key(), retryNumber)
 				}
 				return nil
 			}
 			return nil
 		},
 	); err != nil {
-		return err
+		return pkgerrors.Wrapf(err, "fetch of remote repository %+v with retry failed", r.Key())
 	}
 	return nil
 }

pkg/externalrepo/git/gogit.go

@@ -22,6 +22,7 @@ import (
 	"github.com/go-git/go-git/v5/plumbing/cache"
 	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/go-git/go-git/v5/storage/filesystem"
+	pkgerrors "github.com/pkg/errors"
 )
 
 // This file contains helpers for interacting with gogit.
@@ -30,23 +31,23 @@ func initEmptyRepository(path string) (*git.Repository, error) {
 	isBare := true // Porch only uses bare repositories
 	repo, err := git.PlainInit(path, isBare)
 	if err != nil {
-		return nil, err
+		return nil, pkgerrors.Wrapf(err, "gogit: repo plain init failed on path %q", path)
 	}
 	if err := initializeDefaultBranches(repo); err != nil {
-		return nil, err
+		return nil, pkgerrors.Wrapf(err, "gogit: default branch initialize failed on repo for path %q", path)
 	}
 	return repo, nil
 }
 
 func initializeDefaultBranches(repo *git.Repository) error {
 	// Adjust default references
 	if err := repo.Storer.RemoveReference(plumbing.Master); err != nil {
-		return err
+		return pkgerrors.Wrapf(err, "gogit: failed to remove reference %+v on repo %+v", plumbing.Master, repo)
 	}
 	// gogit points HEAD at a wrong branch; point it at main
 	main := plumbing.NewSymbolicReference(plumbing.HEAD, DefaultMainReferenceName)
 	if err := repo.Storer.SetReference(main); err != nil {
-		return err
+		return pkgerrors.Wrapf(err, "gogit: failed to set reference %+v on repo %+v", main, repo)
 	}
 	return nil
 }
@@ -60,7 +61,7 @@ func openRepository(path string) (*git.Repository, error) {
 func initializeOrigin(repo *git.Repository, address string) error {
 	cfg, err := repo.Config()
 	if err != nil {
-		return err
+		return pkgerrors.Wrapf(err, "gogit: failed to get configuration for repo %+v", repo)
 	}
 
 	cfg.Remotes[OriginName] = &config.RemoteConfig{
@@ -70,7 +71,7 @@ func initializeOrigin(repo *git.Repository, address string) error {
 	}
 
 	if err := repo.SetConfig(cfg); err != nil {
-		return err
+		return pkgerrors.Wrapf(err, "gogit: failed to set configuration for repo %+v", repo)
 	}
 
 	return nil
@@ -79,15 +80,15 @@ func initializeOrigin(repo *git.Repository, address string) error {
 func storeCommit(repo *git.Repository, commit *object.Commit) (plumbing.Hash, error) {
 	eo := repo.Storer.NewEncodedObject()
 	if err := commit.Encode(eo); err != nil {
-		return plumbing.Hash{}, err
+		return plumbing.Hash{}, pkgerrors.Wrapf(err, "gogit: failed to store commit %+v on repo %+v", commit, repo)
 	}
 	return repo.Storer.SetEncodedObject(eo)
 }
 
 func storeTree(repo *git.Repository, tree *object.Tree) (plumbing.Hash, error) {
 	eo := repo.Storer.NewEncodedObject()
 	if err := tree.Encode(eo); err != nil {
-		return plumbing.Hash{}, err
+		return plumbing.Hash{}, pkgerrors.Wrapf(err, "gogit: failed to store tree %+v on repo %+v", tree, repo)
 	}
 	return repo.Storer.SetEncodedObject(eo)
 }
@@ -100,16 +101,16 @@ func storeBlob(repo *git.Repository, value string) (plumbing.Hash, error) {
 
 	w, err := eo.Writer()
 	if err != nil {
-		return plumbing.Hash{}, err
+		return plumbing.Hash{}, pkgerrors.Wrapf(err, "gogit: failed to get writer for repo %+v", repo)
 	}
 
 	if _, err := w.Write(data); err != nil {
 		w.Close()
-		return plumbing.Hash{}, err
+		return plumbing.Hash{}, pkgerrors.Wrapf(err, "gogit: failed to write blob %+v to repo %+v", value, repo)
 	}
 
 	if err := w.Close(); err != nil {
-		return plumbing.Hash{}, err
+		return plumbing.Hash{}, pkgerrors.Wrapf(err, "gogit: failed to close repo %+v", repo)
 	}
 
 	return repo.Storer.SetEncodedObject(eo)