Avoid skipping reconciliation

- Attempt short-circuiting clone only when the artifact is already in the
  storage.
- A successful no-op clone need not return an error, but a partial
  commit which contains only a hash + reference.
- On no-op clone, reconcileSource() populates the source build dir by
  copying the existing artifact and lets the reconciliation continue.
- Reconciliation is not skipped to allow other subreconcilers to operate
  on other parts of GitRepo object like include, ignore, etc, when
  attributes associated with them change but the remote repo has not
  changed.
- Add a function IsConcreteCommit() to differentiate between partial and
  concrete commit.
- Update and simplify go-git and libgit2 no-op clone tests.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
This commit is contained in:
Sunny 2022-05-14 22:58:11 +05:30
parent aea21cdde7
commit 71d3870e0a
No known key found for this signature in database
GPG Key ID: 9F3D25DDFF7FA3CF
11 changed files with 370 additions and 259 deletions

View File

@ -100,4 +100,7 @@ const (
// CacheOperationFailedReason signals a failure in cache operation.
CacheOperationFailedReason string = "CacheOperationFailed"
// CopyOperationFailedReason signals a failure in copying files.
CopyOperationFailedReason string = "CopyOperationFailed"
)

View File

@ -425,9 +425,10 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
}
if val, ok := r.features[features.OptimizedGitClones]; ok && val {
// Only if the object is ready, use the last revision to attempt
// short-circuiting clone operation.
if conditions.IsTrue(obj, meta.ReadyCondition) {
// Only if the object has an existing artifact in storage, attempt to
// short-circuit clone operation. reconcileStorage has already verified
// that the artifact exists.
if conditions.IsTrue(obj, sourcev1.ArtifactInStorageCondition) {
if artifact := obj.GetArtifact(); artifact != nil {
checkoutOpts.LastRevision = artifact.Revision
}
@ -478,17 +479,6 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
defer cancel()
c, err := checkoutStrategy.Checkout(gitCtx, dir, repositoryURL, authOpts)
if err != nil {
var v git.NoChangesError
if errors.As(err, &v) {
// Create generic error without notification. Since it's a no-op
// error, ignore (no runtime error), normal event.
ge := serror.NewGeneric(v, sourcev1.GitOperationSucceedReason)
ge.Notification = false
ge.Ignore = true
ge.Event = corev1.EventTypeNormal
return sreconcile.ResultEmpty, ge
}
e := serror.NewGeneric(
fmt.Errorf("failed to checkout and determine revision: %w", err),
sourcev1.GitOperationFailedReason,
@ -499,8 +489,38 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
}
// Assign the commit to the shared commit reference.
*commit = *c
// If it's a partial commit obtained from an existing artifact, check if the
// reconciliation can be skipped if other configurations have not changed.
if !git.IsConcreteCommit(*commit) {
ctrl.LoggerFrom(ctx).V(logger.DebugLevel).Info(fmt.Sprintf(
"no changes since last reconciliation, observed revision '%s'", commit.String()))
// Remove the target directory, as CopyToPath() renames another
// directory to which the artifact is unpacked into the target
// directory. At this point, the target directory is empty, safe to
// remove.
os.RemoveAll(dir)
if err := r.Storage.CopyToPath(obj.GetArtifact(), "/", dir); err != nil {
e := serror.NewGeneric(
fmt.Errorf("failed to copy existing artifact to source dir: %w", err),
sourcev1.CopyOperationFailedReason,
)
conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error())
return sreconcile.ResultEmpty, e
}
conditions.Delete(obj, sourcev1.FetchFailedCondition)
conditions.Delete(obj, sourcev1.StorageOperationFailedCondition)
return sreconcile.ResultSuccess, nil
}
ctrl.LoggerFrom(ctx).V(logger.DebugLevel).Info("git repository checked out", "url", obj.Spec.URL, "revision", commit.String())
conditions.Delete(obj, sourcev1.FetchFailedCondition)
// In case no-op clone resulted in a failure and in the subsequent
// reconciliation a new remote revision was observed, delete any stale
// StorageOperationFailedCondition.
conditions.Delete(obj, sourcev1.StorageOperationFailedCondition)
// Verify commit signature
if result, err := r.verifyCommitSignature(ctx, obj, *commit); err != nil || result == sreconcile.ResultEmpty {

View File

@ -551,27 +551,31 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
want sreconcile.Result
wantErr bool
wantRevision string
wantArtifactOutdated bool
}{
{
name: "Nil reference (default branch)",
want: sreconcile.ResultSuccess,
wantRevision: "master/<commit>",
name: "Nil reference (default branch)",
want: sreconcile.ResultSuccess,
wantRevision: "master/<commit>",
wantArtifactOutdated: true,
},
{
name: "Branch",
reference: &sourcev1.GitRepositoryRef{
Branch: "staging",
},
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantArtifactOutdated: true,
},
{
name: "Tag",
reference: &sourcev1.GitRepositoryRef{
Tag: "v0.1.0",
},
want: sreconcile.ResultSuccess,
wantRevision: "v0.1.0/<commit>",
want: sreconcile.ResultSuccess,
wantRevision: "v0.1.0/<commit>",
wantArtifactOutdated: true,
},
{
name: "Branch commit",
@ -580,8 +584,9 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
Branch: "staging",
Commit: "<commit>",
},
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantArtifactOutdated: true,
},
{
name: "Branch commit",
@ -590,60 +595,56 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
Branch: "staging",
Commit: "<commit>",
},
want: sreconcile.ResultSuccess,
wantRevision: "HEAD/<commit>",
want: sreconcile.ResultSuccess,
wantRevision: "HEAD/<commit>",
wantArtifactOutdated: true,
},
{
name: "SemVer",
reference: &sourcev1.GitRepositoryRef{
SemVer: "*",
},
want: sreconcile.ResultSuccess,
wantRevision: "v2.0.0/<commit>",
want: sreconcile.ResultSuccess,
wantRevision: "v2.0.0/<commit>",
wantArtifactOutdated: true,
},
{
name: "SemVer range",
reference: &sourcev1.GitRepositoryRef{
SemVer: "<v0.2.1",
},
want: sreconcile.ResultSuccess,
wantRevision: "0.2.0/<commit>",
want: sreconcile.ResultSuccess,
wantRevision: "0.2.0/<commit>",
wantArtifactOutdated: true,
},
{
name: "SemVer prerelease",
reference: &sourcev1.GitRepositoryRef{
SemVer: ">=1.0.0-0 <1.1.0-0",
},
wantRevision: "v1.0.0-alpha/<commit>",
want: sreconcile.ResultSuccess,
wantRevision: "v1.0.0-alpha/<commit>",
want: sreconcile.ResultSuccess,
wantArtifactOutdated: true,
},
{
name: "Optimized clone, Ready=True",
name: "Optimized clone",
reference: &sourcev1.GitRepositoryRef{
Branch: "staging",
},
beforeFunc: func(obj *sourcev1.GitRepository, latestRev string) {
// Add existing artifact on the object and storage.
obj.Status = sourcev1.GitRepositoryStatus{
Artifact: &sourcev1.Artifact{
Revision: "staging/" + latestRev,
Path: randStringRunes(10),
},
}
conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "ready")
testStorage.Archive(obj.GetArtifact(), "testdata/git/repository", nil)
conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
},
want: sreconcile.ResultEmpty,
wantErr: true,
wantRevision: "staging/<commit>",
},
{
name: "Optimized clone, Ready=False",
reference: &sourcev1.GitRepositoryRef{
Branch: "staging",
},
beforeFunc: func(obj *sourcev1.GitRepository, latestRev string) {
conditions.MarkFalse(obj, meta.ReadyCondition, meta.FailedReason, "not ready")
},
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantArtifactOutdated: false,
},
}
@ -721,7 +722,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
if tt.wantRevision != "" && !tt.wantErr {
revision := strings.ReplaceAll(tt.wantRevision, "<commit>", headRef.Hash().String())
g.Expect(commit.String()).To(Equal(revision))
g.Expect(conditions.IsTrue(obj, sourcev1.ArtifactOutdatedCondition)).To(BeTrue())
g.Expect(conditions.IsTrue(obj, sourcev1.ArtifactOutdatedCondition)).To(Equal(tt.wantArtifactOutdated))
}
})
}

View File

@ -406,8 +406,8 @@ reconciliations. It supports both `go-git` and `libgit2` implementations
when cloning repositories using branches or tags.
When enabled, avoids full clone operations by first checking whether
the last revision is still the same at the target repository,
and if that is so, skips the reconciliation.
the revision of the last stored artifact is still the head of the remote
repository, and if that is so, the local artifact is reused.
This feature is enabled by default. It can be disabled by starting the
controller with the argument `--feature-gates=OptimizedGitClones=false`.

View File

@ -107,14 +107,12 @@ type CheckoutStrategy interface {
Checkout(ctx context.Context, path, url string, config *AuthOptions) (*Commit, error)
}
// NoChangesError represents the case in which a Git clone operation
// is attempted, but cancelled as the revision is still the same as
// the one observed on the last successful reconciliation.
type NoChangesError struct {
Message string
ObservedRevision string
}
func (e NoChangesError) Error() string {
return fmt.Sprintf("%s: observed revision '%s'", e.Message, e.ObservedRevision)
// IsConcreteCommit returns if a given commit is a concrete commit. Concrete
// commits have most of commit metadata and commit content. In contrast, a
// partial commit may only have some metadata and no commit content.
func IsConcreteCommit(c Commit) bool {
if c.Hash != nil && c.Encoded != nil {
return true
}
return false
}

View File

@ -18,6 +18,7 @@ package git
import (
"testing"
"time"
. "github.com/onsi/gomega"
)
@ -263,3 +264,41 @@ of the commit`,
})
}
}
func TestIsConcreteCommit(t *testing.T) {
tests := []struct {
name string
commit Commit
result bool
}{
{
name: "concrete commit",
commit: Commit{
Hash: Hash("foo"),
Reference: "refs/tags/main",
Author: Signature{
Name: "user", Email: "user@example.com", When: time.Now(),
},
Committer: Signature{
Name: "user", Email: "user@example.com", When: time.Now(),
},
Signature: "signature",
Encoded: []byte("commit-content"),
Message: "commit-message",
},
result: true,
},
{
name: "partial commit",
commit: Commit{Hash: Hash("foo")},
result: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
g.Expect(IsConcreteCommit(tt.commit)).To(Equal(tt.result))
})
}
}

View File

@ -22,6 +22,7 @@ import (
"fmt"
"io"
"sort"
"strings"
"time"
"github.com/Masterminds/semver/v3"
@ -78,10 +79,21 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g
}
if currentRevision != "" && currentRevision == c.LastRevision {
return nil, git.NoChangesError{
Message: "no changes since last reconciliation",
ObservedRevision: currentRevision,
// Construct a partial commit with the existing information.
// Split the revision and take the last part as the hash.
// Example revision: main/43d7eb9c49cdd49b2494efd481aea1166fc22b67
var hash git.Hash
ss := strings.Split(currentRevision, "/")
if len(ss) > 1 {
hash = git.Hash(ss[len(ss)-1])
} else {
hash = git.Hash(ss[0])
}
c := &git.Commit{
Hash: hash,
Reference: plumbing.NewBranchReferenceName(c.Branch).String(),
}
return c, nil
}
}
@ -153,10 +165,21 @@ func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.
}
if currentRevision != "" && currentRevision == c.LastRevision {
return nil, git.NoChangesError{
Message: "no changes since last reconciliation",
ObservedRevision: currentRevision,
// Construct a partial commit with the existing information.
// Split the revision and take the last part as the hash.
// Example revision: 6.1.4/bf09377bfd5d3bcac1e895fa8ce52dc76695c060
var hash git.Hash
ss := strings.Split(currentRevision, "/")
if len(ss) > 1 {
hash = git.Hash(ss[len(ss)-1])
} else {
hash = git.Hash(ss[0])
}
c := &git.Commit{
Hash: hash,
Reference: ref.String(),
}
return c, nil
}
}
repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{

View File

@ -67,32 +67,36 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
}
tests := []struct {
name string
branch string
filesCreated map[string]string
expectedCommit string
expectedErr string
lastRevision string
name string
branch string
filesCreated map[string]string
lastRevision string
expectedCommit string
expectedConcreteCommit bool
expectedErr string
}{
{
name: "Default branch",
branch: "master",
filesCreated: map[string]string{"branch": "init"},
expectedCommit: firstCommit.String(),
name: "Default branch",
branch: "master",
filesCreated: map[string]string{"branch": "init"},
expectedCommit: firstCommit.String(),
expectedConcreteCommit: true,
},
{
name: "skip clone if LastRevision hasn't changed",
branch: "master",
filesCreated: map[string]string{"branch": "init"},
expectedErr: fmt.Sprintf("no changes since last reconciliation: observed revision 'master/%s'", firstCommit.String()),
lastRevision: fmt.Sprintf("master/%s", firstCommit.String()),
name: "skip clone if LastRevision hasn't changed",
branch: "master",
filesCreated: map[string]string{"branch": "init"},
lastRevision: fmt.Sprintf("master/%s", firstCommit.String()),
expectedCommit: firstCommit.String(),
expectedConcreteCommit: false,
},
{
name: "Other branch - revision has changed",
branch: "test",
filesCreated: map[string]string{"branch": "second"},
expectedCommit: secondCommit.String(),
lastRevision: fmt.Sprintf("master/%s", firstCommit.String()),
name: "Other branch - revision has changed",
branch: "test",
filesCreated: map[string]string{"branch": "second"},
lastRevision: fmt.Sprintf("master/%s", firstCommit.String()),
expectedCommit: secondCommit.String(),
expectedConcreteCommit: true,
},
{
name: "Non existing branch",
@ -120,59 +124,65 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
}
g.Expect(err).ToNot(HaveOccurred())
g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit))
g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectedConcreteCommit))
for k, v := range tt.filesCreated {
g.Expect(filepath.Join(tmpDir, k)).To(BeARegularFile())
g.Expect(os.ReadFile(filepath.Join(tmpDir, k))).To(BeEquivalentTo(v))
if tt.expectedConcreteCommit {
for k, v := range tt.filesCreated {
g.Expect(filepath.Join(tmpDir, k)).To(BeARegularFile())
g.Expect(os.ReadFile(filepath.Join(tmpDir, k))).To(BeEquivalentTo(v))
}
}
})
}
}
func TestCheckoutTag_Checkout(t *testing.T) {
type testTag struct {
name string
annotated bool
}
tests := []struct {
name string
tag string
annotated bool
checkoutTag string
expectTag string
expectErr string
lastRev string
setLastRev bool
name string
tagsInRepo []testTag
checkoutTag string
lastRevTag string
expectConcreteCommit bool
expectErr string
}{
{
name: "Tag",
tag: "tag-1",
checkoutTag: "tag-1",
expectTag: "tag-1",
name: "Tag",
tagsInRepo: []testTag{{"tag-1", false}},
checkoutTag: "tag-1",
expectConcreteCommit: true,
},
{
name: "Skip Tag if last revision hasn't changed",
tag: "tag-2",
checkoutTag: "tag-2",
setLastRev: true,
expectErr: "no changes since last reconciliation",
name: "Annotated",
tagsInRepo: []testTag{{"annotated", true}},
checkoutTag: "annotated",
expectConcreteCommit: true,
},
{
name: "Last revision changed",
tag: "tag-3",
checkoutTag: "tag-3",
expectTag: "tag-3",
lastRev: "tag-3/<fake-hash>",
},
{
name: "Annotated",
tag: "annotated",
annotated: true,
checkoutTag: "annotated",
expectTag: "annotated",
},
{
name: "Non existing tag",
tag: "tag-1",
name: "Non existing tag",
// Without this go-git returns error "remote repository is empty".
tagsInRepo: []testTag{{"tag-1", false}},
checkoutTag: "invalid",
expectErr: "couldn't find remote ref \"refs/tags/invalid\"",
},
{
name: "Skip clone - last revision unchanged",
tagsInRepo: []testTag{{"tag-1", false}},
checkoutTag: "tag-1",
lastRevTag: "tag-1",
expectConcreteCommit: false,
},
{
name: "Last revision changed",
tagsInRepo: []testTag{{"tag-1", false}, {"tag-2", false}},
checkoutTag: "tag-2",
lastRevTag: "tag-1",
expectConcreteCommit: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -183,32 +193,37 @@ func TestCheckoutTag_Checkout(t *testing.T) {
t.Fatal(err)
}
var h plumbing.Hash
var tagHash *plumbing.Reference
if tt.tag != "" {
h, err = commitFile(repo, "tag", tt.tag, time.Now())
if err != nil {
t.Fatal(err)
}
tagHash, err = tag(repo, h, !tt.annotated, tt.tag, time.Now())
if err != nil {
t.Fatal(err)
// Collect tags and their associated commit hash for later
// reference.
tagCommits := map[string]string{}
// Populate the repo with commits and tags.
if tt.tagsInRepo != nil {
for _, tr := range tt.tagsInRepo {
h, err := commitFile(repo, "tag", tr.name, time.Now())
if err != nil {
t.Fatal(err)
}
_, err = tag(repo, h, tr.annotated, tr.name, time.Now())
if err != nil {
t.Fatal(err)
}
tagCommits[tr.name] = h.String()
}
}
tag := CheckoutTag{
checkoutTag := CheckoutTag{
Tag: tt.checkoutTag,
}
if tt.setLastRev {
tag.LastRevision = fmt.Sprintf("%s/%s", tt.tag, tagHash.Hash().String())
// If last revision is provided, configure it.
if tt.lastRevTag != "" {
lc := tagCommits[tt.lastRevTag]
checkoutTag.LastRevision = fmt.Sprintf("%s/%s", tt.lastRevTag, lc)
}
if tt.lastRev != "" {
tag.LastRevision = tt.lastRev
}
tmpDir := t.TempDir()
cc, err := tag.Checkout(context.TODO(), tmpDir, path, nil)
cc, err := checkoutTag.Checkout(context.TODO(), tmpDir, path, nil)
if tt.expectErr != "" {
g.Expect(err).ToNot(BeNil())
g.Expect(err.Error()).To(ContainSubstring(tt.expectErr))
@ -216,10 +231,17 @@ func TestCheckoutTag_Checkout(t *testing.T) {
return
}
// Check successful checkout results.
g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectConcreteCommit))
targetTagHash := tagCommits[tt.checkoutTag]
g.Expect(err).ToNot(HaveOccurred())
g.Expect(cc.String()).To(Equal(tt.expectTag + "/" + h.String()))
g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile())
g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.tag))
g.Expect(cc.String()).To(Equal(tt.checkoutTag + "/" + targetTagHash))
// Check file content only when there's an actual checkout.
if tt.lastRevTag != tt.checkoutTag {
g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile())
g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.checkoutTag))
}
})
}
}

View File

@ -81,12 +81,15 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g
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.Branch, heads[0].Id.String())
hash := heads[0].Id.String()
currentRevision := fmt.Sprintf("%s/%s", c.Branch, hash)
if currentRevision == c.LastRevision {
return nil, git.NoChangesError{
Message: "no changes since last reconciliation",
ObservedRevision: currentRevision,
// Construct a partial commit with the existing information.
c := &git.Commit{
Hash: git.Hash(hash),
Reference: "refs/heads/" + c.Branch,
}
return c, nil
}
}
}
@ -163,21 +166,25 @@ func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.
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())
hash := heads[0].Id.String()
currentRevision := fmt.Sprintf("%s/%s", c.Tag, hash)
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())
hash = heads[1].Id.String()
currentAnnotatedRevision := fmt.Sprintf("%s/%s", c.Tag, hash)
if currentAnnotatedRevision == c.LastRevision {
same = true
}
}
if same {
return nil, git.NoChangesError{
Message: "no changes since last reconciliation",
ObservedRevision: currentRevision,
// Construct a partial commit with the existing information.
c := &git.Commit{
Hash: git.Hash(hash),
Reference: "refs/tags/" + c.Tag,
}
return c, nil
}
}
}

View File

@ -83,44 +83,49 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
}
tests := []struct {
name string
branch string
filesCreated map[string]string
expectedCommit string
expectedErr string
lastRevision string
name string
branch string
filesCreated map[string]string
lastRevision string
expectedCommit string
expectedConcreteCommit bool
expectedErr string
}{
{
name: "Default branch",
branch: defaultBranch,
filesCreated: map[string]string{"branch": "second"},
expectedCommit: secondCommit.String(),
name: "Default branch",
branch: defaultBranch,
filesCreated: map[string]string{"branch": "second"},
expectedCommit: secondCommit.String(),
expectedConcreteCommit: true,
},
{
name: "Other branch",
branch: "test",
filesCreated: map[string]string{"branch": "init"},
expectedCommit: firstCommit.String(),
name: "Other branch",
branch: "test",
filesCreated: map[string]string{"branch": "init"},
expectedCommit: firstCommit.String(),
expectedConcreteCommit: true,
},
{
name: "Non existing branch",
branch: "invalid",
expectedErr: "reference 'refs/remotes/origin/invalid' not found",
name: "Non existing branch",
branch: "invalid",
expectedErr: "reference 'refs/remotes/origin/invalid' not found",
expectedConcreteCommit: true,
},
{
name: "skip clone - lastRevision hasn't changed",
branch: defaultBranch,
filesCreated: map[string]string{"branch": "second"},
expectedCommit: secondCommit.String(),
lastRevision: fmt.Sprintf("%s/%s", defaultBranch, secondCommit.String()),
expectedErr: fmt.Sprintf("no changes since last reconciliation: observed revision '%s/%s'", defaultBranch, secondCommit.String()),
name: "skip clone - lastRevision hasn't changed",
branch: defaultBranch,
filesCreated: map[string]string{"branch": "second"},
lastRevision: fmt.Sprintf("%s/%s", defaultBranch, secondCommit.String()),
expectedCommit: secondCommit.String(),
expectedConcreteCommit: false,
},
{
name: "lastRevision is different",
branch: defaultBranch,
filesCreated: map[string]string{"branch": "second"},
expectedCommit: secondCommit.String(),
lastRevision: fmt.Sprintf("%s/%s", defaultBranch, firstCommit.String()),
name: "lastRevision is different",
branch: defaultBranch,
filesCreated: map[string]string{"branch": "second"},
lastRevision: fmt.Sprintf("%s/%s", defaultBranch, firstCommit.String()),
expectedCommit: secondCommit.String(),
expectedConcreteCommit: true,
},
}
@ -143,37 +148,43 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
}
g.Expect(err).ToNot(HaveOccurred())
g.Expect(cc.String()).To(Equal(tt.branch + "/" + tt.expectedCommit))
g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectedConcreteCommit))
for k, v := range tt.filesCreated {
g.Expect(filepath.Join(tmpDir, k)).To(BeARegularFile())
g.Expect(os.ReadFile(filepath.Join(tmpDir, k))).To(BeEquivalentTo(v))
if tt.expectedConcreteCommit {
for k, v := range tt.filesCreated {
g.Expect(filepath.Join(tmpDir, k)).To(BeARegularFile())
g.Expect(os.ReadFile(filepath.Join(tmpDir, k))).To(BeEquivalentTo(v))
}
}
})
}
}
func TestCheckoutTag_Checkout(t *testing.T) {
type testTag struct {
name string
annotated bool
}
tests := []struct {
name string
tag string
annotated bool
checkoutTag string
expectTag string
expectErr string
lastRevision bool
name string
tagsInRepo []testTag
checkoutTag string
lastRevTag string
expectErr string
expectConcreteCommit bool
}{
{
name: "Tag",
tag: "tag-1",
checkoutTag: "tag-1",
expectTag: "tag-1",
name: "Tag",
tagsInRepo: []testTag{{"tag-1", false}},
checkoutTag: "tag-1",
expectConcreteCommit: true,
},
{
name: "Annotated",
tag: "annotated",
annotated: true,
checkoutTag: "annotated",
expectTag: "annotated",
name: "Annotated",
tagsInRepo: []testTag{{"annotated", true}},
checkoutTag: "annotated",
expectConcreteCommit: true,
},
{
name: "Non existing tag",
@ -181,19 +192,18 @@ func TestCheckoutTag_Checkout(t *testing.T) {
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: "Skip clone - last revision unchanged",
tagsInRepo: []testTag{{"tag-1", false}},
checkoutTag: "tag-1",
lastRevTag: "tag-1",
expectConcreteCommit: false,
},
{
name: "last revision changed",
tag: "tag-1",
checkoutTag: "tag-1",
expectTag: "tag-2",
lastRevision: true,
name: "Last revision changed",
tagsInRepo: []testTag{{"tag-1", false}, {"tag-2", false}},
checkoutTag: "tag-2",
lastRevTag: "tag-1",
expectConcreteCommit: true,
},
}
for _, tt := range tests {
@ -206,68 +216,57 @@ func TestCheckoutTag_Checkout(t *testing.T) {
}
defer repo.Free()
var commit *git2go.Commit
if tt.tag != "" {
c, err := commitFile(repo, "tag", tt.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.tag, time.Now())
if err != nil {
t.Fatal(err)
}
}
// Collect tags and their associated commit for later reference.
tagCommits := map[string]*git2go.Commit{}
checkoutTag := CheckoutTag{
Tag: tt.checkoutTag,
}
tmpDir := t.TempDir()
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())
// Populate the repo with commits and tags.
if tt.tagsInRepo != nil {
for _, tr := range tt.tagsInRepo {
var commit *git2go.Commit
c, err := commitFile(repo, "tag", tr.name, 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())
_, err = tag(repo, commit.Id(), tr.annotated, tr.name, 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)
tagCommits[tr.name] = commit
}
}
checkoutTag := CheckoutTag{
Tag: tt.checkoutTag,
}
// If last revision is provided, configure it.
if tt.lastRevTag != "" {
lc := tagCommits[tt.lastRevTag]
checkoutTag.LastRevision = fmt.Sprintf("%s/%s", tt.lastRevTag, lc.Id().String())
}
tmpDir := t.TempDir()
cc, err := checkoutTag.Checkout(context.TODO(), tmpDir, repo.Path(), nil)
if tt.expectErr != "" {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(ContainSubstring(tt.expectErr))
g.Expect(cc).To(BeNil())
return
}
// Check successful checkout results.
g.Expect(git.IsConcreteCommit(*cc)).To(Equal(tt.expectConcreteCommit))
targetTagCommit := tagCommits[tt.checkoutTag]
g.Expect(err).ToNot(HaveOccurred())
g.Expect(cc.String()).To(Equal(tt.expectTag + "/" + commit.Id().String()))
g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile())
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))
g.Expect(cc.String()).To(Equal(tt.checkoutTag + "/" + targetTagCommit.Id().String()))
// Check file content only when there's an actual checkout.
if tt.lastRevTag != tt.checkoutTag {
g.Expect(filepath.Join(tmpDir, "tag")).To(BeARegularFile())
g.Expect(os.ReadFile(filepath.Join(tmpDir, "tag"))).To(BeEquivalentTo(tt.checkoutTag))
}
})
}

View File

@ -49,8 +49,7 @@ type CheckoutOptions struct {
// not supported by all Implementations.
RecurseSubmodules bool
// LastRevision holds the revision observed on the last successful
// reconciliation.
// LastRevision holds the last observed revision of the local repository.
// It is used to skip clone operations when no changes were detected.
LastRevision string
}