Skip to content

Commit

Permalink
fix repo push 'repo not found err' (#191)
Browse files Browse the repository at this point in the history
  • Loading branch information
@roi-codefresh
roi-codefresh authored Oct 7, 2021
1 parent 4d32a61 commit 20b7771
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 29 deletions.
55 changes: 50 additions & 5 deletions pkg/git/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/url"
"os"
"strings"
"time"

"github.com/argoproj-labs/argocd-autopilot/pkg/fs"
"github.com/argoproj-labs/argocd-autopilot/pkg/git/gogit"
Expand Down Expand Up @@ -77,6 +78,12 @@ var (
ErrNoRemotes = errors.New("no remotes in repository")
)

// Defaults
const (
pushRetries = 3
failureBackoffTime = 3 * time.Second
)

// go-git functions (we mock those in tests)
var (
checkoutRef = func(r *repo, ref string) error {
Expand Down Expand Up @@ -217,10 +224,24 @@ func (r *repo) Persist(ctx context.Context, opts *PushOptions) (string, error) {
return "", err
}

return h.String(), r.PushContext(ctx, &gg.PushOptions{
Auth: getAuth(r.auth),
Progress: progress,
})
for try := 0; try < pushRetries; try++ {
err = r.PushContext(ctx, &gg.PushOptions{
Auth: getAuth(r.auth),
Progress: progress,
})
if err == nil || !errors.Is(err, transport.ErrRepositoryNotFound) {
break
}

log.G(ctx).WithFields(log.Fields{
"retry": try,
"err": err.Error(),
}).Warn("Failed to push to repository, trying again in 3 seconds...")

time.Sleep(failureBackoffTime)
}

return h.String(), err
}

func (r *repo) commit(opts *PushOptions) (*plumbing.Hash, error) {
Expand Down Expand Up @@ -261,6 +282,12 @@ func (r *repo) commit(opts *PushOptions) (*plumbing.Hash, error) {
}

var clone = func(ctx context.Context, opts *CloneOptions) (*repo, error) {
var (
err error
r gogit.Repository
curPushRetries = pushRetries
)

if opts == nil {
return nil, ErrNilOpts
}
Expand All @@ -278,7 +305,25 @@ var clone = func(ctx context.Context, opts *CloneOptions) (*repo, error) {
}

log.G(ctx).WithField("url", opts.url).Debug("cloning git repo")
r, err := ggClone(ctx, memory.NewStorage(), opts.FS, cloneOpts)

if opts.createIfNotExist {
curPushRetries = 1 // no retries
}

for try := 0; try < curPushRetries; try++ {
r, err = ggClone(ctx, memory.NewStorage(), opts.FS, cloneOpts)
if err == nil || !errors.Is(err, transport.ErrRepositoryNotFound) {
break
}

log.G(ctx).WithFields(log.Fields{
"retry": try,
"err": err.Error(),
}).Debug("Failed to clone repository, trying again in 3 seconds...")

time.Sleep(failureBackoffTime)
}

if err != nil {
return nil, err
}
Expand Down
102 changes: 78 additions & 24 deletions pkg/git/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,11 @@ func Test_clone(t *testing.T) {
wantErr bool
expectedOpts *gg.CloneOptions
checkoutRef func(t *testing.T, r *repo, ref string) error
assertFn func(t *testing.T, r *repo)
assertFn func(t *testing.T, r *repo, cloneCalls int)
}{
"Should fail when there are no CloneOptions": {
wantErr: true,
assertFn: func(t *testing.T, r *repo) {
assertFn: func(t *testing.T, r *repo, cloneCalls int) {
assert.Nil(t, r)
},
},
Expand All @@ -277,7 +277,7 @@ func Test_clone(t *testing.T) {
Depth: 1,
Progress: os.Stderr,
},
assertFn: func(t *testing.T, r *repo) {
assertFn: func(t *testing.T, r *repo, cloneCalls int) {
assert.NotNil(t, r)
},
},
Expand All @@ -298,7 +298,7 @@ func Test_clone(t *testing.T) {
Depth: 1,
Progress: os.Stderr,
},
assertFn: func(t *testing.T, r *repo) {
assertFn: func(t *testing.T, r *repo, cloneCalls int) {
assert.NotNil(t, r)
},
},
Expand All @@ -313,7 +313,7 @@ func Test_clone(t *testing.T) {
},
retErr: fmt.Errorf("error"),
wantErr: true,
assertFn: func(t *testing.T, r *repo) {
assertFn: func(t *testing.T, r *repo, cloneCalls int) {
assert.Nil(t, r)
},
},
Expand All @@ -330,7 +330,7 @@ func Test_clone(t *testing.T) {
assert.Equal(t, "test", ref)
return nil
},
assertFn: func(t *testing.T, r *repo) {
assertFn: func(t *testing.T, r *repo, cloneCalls int) {
assert.NotNil(t, r)
},
},
Expand All @@ -348,10 +348,45 @@ func Test_clone(t *testing.T) {
assert.Equal(t, "test", ref)
return errors.New("some error")
},
assertFn: func(t *testing.T, r *repo) {
assertFn: func(t *testing.T, r *repo, cloneCalls int) {
assert.Nil(t, r)
},
},
"Should retry if fails with 'repo not found' error": {
opts: &CloneOptions{
Repo: "https://github.com/owner/name",
},
expectedOpts: &gg.CloneOptions{
URL: "https://github.com/owner/name.git",
Auth: nil,
Depth: 1,
Progress: os.Stderr,
},
assertFn: func(t *testing.T, r *repo, cloneCalls int) {
assert.Nil(t, r)
assert.Equal(t, cloneCalls, 3)
},
retErr: transport.ErrRepositoryNotFound,
wantErr: true,
},
"Should not retry if createIfNotExist is true": {
opts: &CloneOptions{
Repo: "https://github.com/owner/name",
createIfNotExist: true,
},
expectedOpts: &gg.CloneOptions{
URL: "https://github.com/owner/name.git",
Auth: nil,
Depth: 1,
Progress: os.Stderr,
},
assertFn: func(t *testing.T, r *repo, cloneCalls int) {
assert.Nil(t, r)
assert.Equal(t, cloneCalls, 1)
},
retErr: transport.ErrRepositoryNotFound,
wantErr: true,
},
}

origCheckoutRef, origClone := checkoutRef, ggClone
Expand All @@ -363,7 +398,9 @@ func Test_clone(t *testing.T) {
for tname, tt := range tests {
t.Run(tname, func(t *testing.T) {
mockRepo := &mocks.Repository{}
cloneCalls := 0
ggClone = func(_ context.Context, _ storage.Storer, _ billy.Filesystem, o *gg.CloneOptions) (gogit.Repository, error) {
cloneCalls++
if tt.expectedOpts != nil {
assert.True(t, reflect.DeepEqual(o, tt.expectedOpts), "opts not equal")
}
Expand Down Expand Up @@ -391,7 +428,7 @@ func Test_clone(t *testing.T) {
return
}

tt.assertFn(t, got)
tt.assertFn(t, got, cloneCalls)
})
}
}
Expand Down Expand Up @@ -584,6 +621,35 @@ func Test_repo_Persist(t *testing.T) {
w.AssertCalled(t, "Commit", "hello", mock.Anything)
},
},
"Retry push on 'repo not found err'": {
opts: &PushOptions{
AddGlobPattern: "test",
CommitMsg: "hello",
},
retErr: transport.ErrRepositoryNotFound,
retRevision: "0dee45f70b37aeb59e6d2efb29855f97df9bccb2",
assertFn: func(t *testing.T, r *mocks.Repository, w *mocks.Worktree, revision string, err error) {
assert.Equal(t, "0dee45f70b37aeb59e6d2efb29855f97df9bccb2", revision)
assert.Error(t, err, transport.ErrRepositoryNotFound)
r.AssertCalled(t, "PushContext", mock.Anything, &gg.PushOptions{
Auth: nil,
Progress: os.Stderr,
})
r.AssertNumberOfCalls(t, "PushContext", 3)
w.AssertCalled(t, "AddGlob", "test")
w.AssertCalled(t, "Commit", "hello", mock.Anything)
},
},
}

gitConfig := &config.Config{
User: struct {
Name string
Email string
}{
Name: "name",
Email: "email",
},
}

worktreeOrg := worktree
Expand All @@ -593,29 +659,17 @@ func Test_repo_Persist(t *testing.T) {
t.Run(tname, func(t *testing.T) {
mockRepo := &mocks.Repository{}
mockRepo.On("PushContext", mock.Anything, mock.Anything).Return(tt.retErr)
mockRepo.On("ConfigScoped", mock.Anything).Return(gitConfig, nil)

mockWt := &mocks.Worktree{}
mockWt.On("AddGlob", mock.Anything).Return(tt.retErr)
mockWt.On("Commit", mock.Anything, mock.Anything).Return(plumbing.NewHash(tt.retRevision), tt.retErr)
mockWt.On("AddGlob", mock.Anything).Return(nil)
mockWt.On("Commit", mock.Anything, mock.Anything).Return(plumbing.NewHash(tt.retRevision), nil)

r := &repo{Repository: mockRepo, progress: os.Stderr}
worktree = func(r gogit.Repository) (gogit.Worktree, error) {
return mockWt, tt.retErr
return mockWt, nil
}

gitConfig := &config.Config{
User: struct {
Name string
Email string
}{
Name: "name",
Email: "email",
},
}

mockRepo.On("ConfigScoped", mock.Anything).Return(gitConfig, nil)
mockWt.On("AddGlob", mock.Anything).Return(tt.retErr)

revision, err := r.Persist(context.Background(), tt.opts)
tt.assertFn(t, mockRepo, mockWt, revision, err)
})
Expand Down

0 comments on commit 20b7771

Please sign in to comment.