fluxcd
/
source-controller
mirror of https://github.com/fluxcd/source-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Optimize libgit2 checkout tag strategy through condition no-ops 

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
This commit is contained in:
Sanskar Jaiswal 2022-04-28 17:34:57 +05:30 committed by Paulo Gomes
parent 860d7051da
commit 6a793a55f6
No known key found for this signature in database
GPG Key ID: 9995233870E99BEE
3 changed files with 145 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
controllers/gitrepository_controller_test.go
View File

@ -359,7 +359,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
},
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "failed to checkout and determine revision: unable to clone '<url>': PEM CA bundle could not be appended to x509 certificate pool"),
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "failed to checkout and determine revision: unable to fetch-connect to remote '<url>': PEM CA bundle could not be appended to x509 certificate pool"),
},
},
{

94
pkg/git/libgit2/checkout.go
View File

@ -65,23 +65,13 @@ type CheckoutBranch struct {
}
func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) {
repo, err := git2go.InitRepository(path, false)
repo, remote, err := getBlankRepoAndRemote(ctx, path, url, opts)
if err != nil {
return nil, fmt.Errorf("unable to init repository for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err))
return nil, err
}
defer repo.Free()
remote, err := repo.Remotes.Create("origin", url)
if err != nil {
return nil, fmt.Errorf("unable to create remote for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err))
}
defer remote.Free()
callBacks := RemoteCallbacks(ctx, opts)
err = remote.ConnectFetch(&callBacks, &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, nil)
if err != nil {
return nil, fmt.Errorf("unable to fetch-connect to remote '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err))
}
defer remote.Disconnect()
// When the last observed revision is set, check whether it is still
@ -95,7 +85,7 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g
currentRevision := fmt.Sprintf("%s/%s", c.Branch, heads[0].Id.String())
if currentRevision == c.LastRevision {
return nil, git.NoChangesError{
Message: "no changes since last reconcilation",
Message: "no changes since last reconciliation",
ObservedRevision: currentRevision,
}
}
@ -155,21 +145,59 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g
}
type CheckoutTag struct {
Tag string
Tag string
LastRevision string
}
func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) {
repo, err := safeClone(url, path, &git2go.CloneOptions{
FetchOptions: git2go.FetchOptions{
DownloadTags: git2go.DownloadTagsAll,
repo, remote, err := getBlankRepoAndRemote(ctx, path, url, opts)
if err != nil {
return nil, err
}
defer repo.Free()
defer remote.Free()
defer remote.Disconnect()
if c.LastRevision != "" {
heads, err := remote.Ls(c.Tag)
if err != nil {
return nil, fmt.Errorf("unable to remote ls for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err))
}
if len(heads) > 0 {
currentRevision := fmt.Sprintf("%s/%s", c.Tag, heads[0].Id.String())
var same bool
if currentRevision == c.LastRevision {
same = true
} else if len(heads) > 1 {
currentAnnotatedRevision := fmt.Sprintf("%s/%s", c.Tag, heads[1].Id.String())
if currentAnnotatedRevision == c.LastRevision {
same = true
}
}
if same {
return nil, git.NoChangesError{
Message: "no changes since last reconciliation",
ObservedRevision: currentRevision,
}
}
}
}
err = remote.Fetch([]string{c.Tag},
&git2go.FetchOptions{
DownloadTags: git2go.DownloadTagsAuto,
RemoteCallbacks: RemoteCallbacks(ctx, opts),
ProxyOptions: git2go.ProxyOptions{Type: git2go.ProxyTypeAuto},
},
})
"")
if err != nil {
return nil, fmt.Errorf("unable to clone '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err))
return nil, fmt.Errorf("unable to fetch remote '%s': %w",
managed.EffectiveURL(url), gitutil.LibGit2Error(err))
}
defer repo.Free()
cc, err := checkoutDetachedDwim(repo, c.Tag)
if err != nil {
return nil, err
@ -391,3 +419,27 @@ func buildSignature(s *git2go.Signature) git.Signature {
When: s.When,
}
}
// getBlankRepoAndRemote returns a newly initialized repository, and a remote connected to the provided url.
// Callers must make sure to call the below defer statements:
// defer repo.Free()
// defer remote.Free()
// defer remote.Disconnect()
func getBlankRepoAndRemote(ctx context.Context, path, url string, opts *git.AuthOptions) (*git2go.Repository, *git2go.Remote, error) {
repo, err := git2go.InitRepository(path, false)
if err != nil {
return nil, nil, fmt.Errorf("unable to init repository for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err))
}
remote, err := repo.Remotes.Create("origin", url)
if err != nil {
return nil, nil, fmt.Errorf("unable to create remote for '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err))
}
callBacks := RemoteCallbacks(ctx, opts)
err = remote.ConnectFetch(&callBacks, &git2go.ProxyOptions{Type: git2go.ProxyTypeAuto}, nil)
if err != nil {
return nil, nil, fmt.Errorf("unable to fetch-connect to remote '%s': %w", managed.EffectiveURL(url), gitutil.LibGit2Error(err))
}
return repo, remote, nil
}

84
pkg/git/libgit2/checkout_test.go
View File

@ -51,8 +51,19 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
// ignores the error here because it can be defaulted
// https://github.blog/2020-07-27-highlights-from-git-2-28/#introducing-init-defaultbranch
defaultBranch := "master"
if v, err := cfg.LookupString("init.defaultBranch"); err != nil && v != "" {
defaultBranch = v
iter, err := cfg.NewIterator()
if err != nil {
t.Fatal(err)
}
for {
val, e := iter.Next()
if e != nil {
break
}
if val.Name == "init.defaultbranch" {
defaultBranch = val.Value
break
}
}
firstCommit, err := commitFile(repo, "branch", "init", time.Now())
@ -102,7 +113,7 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
filesCreated: map[string]string{"branch": "second"},
expectedCommit: secondCommit.String(),
lastRevision: fmt.Sprintf("%s/%s", defaultBranch, secondCommit.String()),
expectedErr: fmt.Sprintf("no changes since last reconcilation: observed revision '%s/%s'", defaultBranch, secondCommit.String()),
expectedErr: fmt.Sprintf("no changes since last reconciliation: observed revision '%s/%s'", defaultBranch, secondCommit.String()),
},
{
name: "lastRevision is different",
@ -143,12 +154,13 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
func TestCheckoutTag_Checkout(t *testing.T) {
tests := []struct {
name string
tag string
annotated bool
checkoutTag string
expectTag string
expectErr string
name string
tag string
annotated bool
checkoutTag string
expectTag string
expectErr string
lastRevision bool
}{
{
name: "Tag",
@ -168,6 +180,21 @@ func TestCheckoutTag_Checkout(t *testing.T) {
checkoutTag: "invalid",
expectErr: "unable to find 'invalid': no reference found for shorthand 'invalid'",
},
{
name: "skip clone - last revision is unchanged",
tag: "tag-1",
checkoutTag: "tag-1",
expectTag: "tag-1",
lastRevision: true,
expectErr: "no changes since last reconciliation",
},
{
name: "last revision changed",
tag: "tag-1",
checkoutTag: "tag-1",
expectTag: "tag-2",
lastRevision: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -188,29 +215,60 @@ func TestCheckoutTag_Checkout(t *testing.T) {
if commit, err = repo.LookupCommit(c); err != nil {
t.Fatal(err)
}
_, err = tag(repo, c, !tt.annotated, tt.tag, time.Now())
_, err = tag(repo, commit.Id(), !tt.annotated, tt.tag, time.Now())
if err != nil {
t.Fatal(err)
}
}
tag := CheckoutTag{
checkoutTag := CheckoutTag{
Tag: tt.checkoutTag,
}
tmpDir := t.TempDir()
cc, err := tag.Checkout(context.TODO(), tmpDir, repo.Path(), nil)
cc, err := checkoutTag.Checkout(context.TODO(), tmpDir, repo.Path(), nil)
if tt.expectErr != "" {
if tt.lastRevision {
tmpDir, _ = os.MkdirTemp("", "test")
defer os.RemoveAll(tmpDir)
checkoutTag.LastRevision = cc.String()
cc, err = checkoutTag.Checkout(context.TODO(), tmpDir, repo.Path(), nil)
}
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(ContainSubstring(tt.expectErr))
g.Expect(cc).To(BeNil())
return
}
if tt.lastRevision {
checkoutTag.LastRevision = fmt.Sprintf("%s/%s", tt.tag, commit.Id().String())
checkoutTag.Tag = tt.expectTag
if tt.tag != "" {
c, err := commitFile(repo, "tag", "changed tag", time.Now())
if err != nil {
t.Fatal(err)
}
if commit, err = repo.LookupCommit(c); err != nil {
t.Fatal(err)
}
_, err = tag(repo, commit.Id(), !tt.annotated, tt.expectTag, time.Now())
if err != nil {
t.Fatal(err)
}
tmpDir, _ = os.MkdirTemp("", "test")
defer os.RemoveAll(tmpDir)
cc, err = checkoutTag.Checkout(context.TODO(), tmpDir, repo.Path(), nil)
}
}
g.Expect(err).ToNot(HaveOccurred())
g.Expect(cc.String()).To(Equal(tt.expectTag + "/" + commit.Id().String()))
g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile())
g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.tag))
if tt.lastRevision {
g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo("changed tag"))
} else {
g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.tag))
}
})
}
}