diff --git a/api/v1beta2/condition_types.go b/api/v1beta2/condition_types.go
index 711469eb8..4338b2378 100644
--- a/api/v1beta2/condition_types.go
+++ b/api/v1beta2/condition_types.go
@@ -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"
)
diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go
index 8f7dc84d9..ed9938af0 100644
--- a/controllers/gitrepository_controller.go
+++ b/controllers/gitrepository_controller.go
@@ -115,6 +115,7 @@ type GitRepositoryReconciler struct {
ControllerName string
requeueDependency time.Duration
+ features map[string]bool
}
type GitRepositoryReconcilerOptions struct {
@@ -134,6 +135,15 @@ func (r *GitRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *GitRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts GitRepositoryReconcilerOptions) error {
r.requeueDependency = opts.DependencyRequeueInterval
+ if r.features == nil {
+ r.features = map[string]bool{}
+ }
+
+ // Check and enable gated features.
+ if oc, _ := features.Enabled(features.OptimizedGitClones); oc {
+ r.features[features.OptimizedGitClones] = true
+ }
+
return ctrl.NewControllerManagedBy(mgr).
For(&sourcev1.GitRepository{}, builder.WithPredicates(
predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}),
@@ -294,7 +304,14 @@ func (r *GitRepositoryReconciler) notify(oldObj, newObj *sourcev1.GitRepository,
oldChecksum = oldObj.GetArtifact().Checksum
}
- message := fmt.Sprintf("stored artifact for commit '%s'", commit.ShortMessage())
+ // A partial commit due to no-op clone doesn't contain the commit
+ // message information. Have separate message for it.
+ var message string
+ if git.IsConcreteCommit(commit) {
+ message = fmt.Sprintf("stored artifact for commit '%s'", commit.ShortMessage())
+ } else {
+ message = fmt.Sprintf("stored artifact for commit '%s'", commit.String())
+ }
// Notify on new artifact and failure recovery.
if oldChecksum != newObj.GetArtifact().Checksum {
@@ -414,9 +431,14 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
checkoutOpts.SemVer = ref.SemVer
}
- if oc, _ := features.Enabled(features.OptimizedGitClones); oc {
- if artifact := obj.GetArtifact(); artifact != nil {
- checkoutOpts.LastRevision = artifact.Revision
+ if val, ok := r.features[features.OptimizedGitClones]; ok && val {
+ // 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
+ }
}
}
@@ -464,12 +486,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) {
- return sreconcile.ResultSuccess,
- &serror.Waiting{Err: v, Reason: v.Message, RequeueAfter: obj.GetRequeueAfter()}
- }
-
e := &serror.Event{
Err: fmt.Errorf("failed to checkout and determine revision: %w", err),
Reason: sourcev1.GitOperationFailedReason,
@@ -480,8 +496,40 @@ 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, copy the
+ // artifact content into new source directory.
+ 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.
+ if err := os.RemoveAll(dir); err != nil {
+ ctrl.LoggerFrom(ctx).Error(err, "failed to remove source build directory")
+ }
+ if err := r.Storage.CopyToPath(obj.GetArtifact(), "/", dir); err != nil {
+ e := &serror.Event{
+ Err: fmt.Errorf("failed to copy existing artifact to source dir: %w", err),
+ Reason: 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 {
diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go
index 040b4e6e9..f1abe6804 100644
--- a/controllers/gitrepository_controller_test.go
+++ b/controllers/gitrepository_controller_test.go
@@ -57,6 +57,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
+ "github.com/fluxcd/source-controller/internal/features"
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
"github.com/fluxcd/source-controller/internal/reconcile/summarize"
"github.com/fluxcd/source-controller/pkg/git"
@@ -499,6 +500,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
+ features: features.FeatureGates(),
}
for _, i := range testGitImplementations {
@@ -545,30 +547,35 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
name string
skipForImplementation string
reference *sourcev1.GitRepositoryRef
+ beforeFunc func(obj *sourcev1.GitRepository, latestRev string)
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",
@@ -577,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",
@@ -587,32 +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",
+ 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),
+ },
+ }
+ testStorage.Archive(obj.GetArtifact(), "testdata/git/repository", nil)
+ conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
+ },
+ want: sreconcile.ResultSuccess,
+ wantRevision: "staging/<commit>",
+ wantArtifactOutdated: false,
},
}
@@ -641,6 +673,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
Client: fakeclient.NewClientBuilder().WithScheme(runtime.NewScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
+ features: features.FeatureGates(),
}
for _, tt := range tests {
@@ -674,6 +707,10 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
obj := obj.DeepCopy()
obj.Spec.GitImplementation = i
+ if tt.beforeFunc != nil {
+ tt.beforeFunc(obj, headRef.Hash().String())
+ }
+
var commit git.Commit
var includes artifactSet
got, err := r.reconcileSource(ctx, obj, &commit, &includes, tmpDir)
@@ -682,10 +719,10 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
}
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
- if tt.wantRevision != "" {
+ 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))
}
})
}
@@ -857,6 +894,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
r := &GitRepositoryReconciler{
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
+ features: features.FeatureGates(),
}
obj := &sourcev1.GitRepository{
@@ -1042,6 +1080,7 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Storage: storage,
requeueDependency: dependencyInterval,
+ features: features.FeatureGates(),
}
obj := &sourcev1.GitRepository{
@@ -1206,6 +1245,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
r := &GitRepositoryReconciler{
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
+ features: features.FeatureGates(),
}
obj := &sourcev1.GitRepository{
@@ -1247,6 +1287,7 @@ func TestGitRepositoryReconciler_reconcileDelete(t *testing.T) {
r := &GitRepositoryReconciler{
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
+ features: features.FeatureGates(),
}
obj := &sourcev1.GitRepository{
@@ -1384,6 +1425,7 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) {
r := &GitRepositoryReconciler{
EventRecorder: record.NewFakeRecorder(32),
Client: builder.Build(),
+ features: features.FeatureGates(),
}
obj := &sourcev1.GitRepository{
@@ -1525,6 +1567,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) {
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
+ features: features.FeatureGates(),
}
key := client.ObjectKeyFromObject(obj)
@@ -1773,10 +1816,22 @@ func TestGitRepositoryReconciler_statusConditions(t *testing.T) {
}
func TestGitRepositoryReconciler_notify(t *testing.T) {
+ concreteCommit := git.Commit{
+ Hash: git.Hash("some-hash"),
+ Reference: "refs/heads/main",
+ Message: "test commit",
+ Encoded: []byte("commit content"),
+ }
+ partialCommit := git.Commit{
+ Hash: git.Hash("some-hash"),
+ Reference: "refs/heads/main",
+ }
+
tests := []struct {
name string
res sreconcile.Result
resErr error
+ commit git.Commit
oldObjBeforeFunc func(obj *sourcev1.GitRepository)
newObjBeforeFunc func(obj *sourcev1.GitRepository)
wantEvent string
@@ -1790,6 +1845,7 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {
name: "new artifact",
res: sreconcile.ResultSuccess,
resErr: nil,
+ commit: concreteCommit,
newObjBeforeFunc: func(obj *sourcev1.GitRepository) {
obj.Status.Artifact = &sourcev1.Artifact{Revision: "xxx", Checksum: "yyy"}
},
@@ -1799,6 +1855,7 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {
name: "recovery from failure",
res: sreconcile.ResultSuccess,
resErr: nil,
+ commit: concreteCommit,
oldObjBeforeFunc: func(obj *sourcev1.GitRepository) {
obj.Status.Artifact = &sourcev1.Artifact{Revision: "xxx", Checksum: "yyy"}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "fail")
@@ -1814,6 +1871,7 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {
name: "recovery and new artifact",
res: sreconcile.ResultSuccess,
resErr: nil,
+ commit: concreteCommit,
oldObjBeforeFunc: func(obj *sourcev1.GitRepository) {
obj.Status.Artifact = &sourcev1.Artifact{Revision: "xxx", Checksum: "yyy"}
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "fail")
@@ -1829,6 +1887,7 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {
name: "no updates",
res: sreconcile.ResultSuccess,
resErr: nil,
+ commit: concreteCommit,
oldObjBeforeFunc: func(obj *sourcev1.GitRepository) {
obj.Status.Artifact = &sourcev1.Artifact{Revision: "xxx", Checksum: "yyy"}
conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "ready")
@@ -1838,6 +1897,22 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {
conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "ready")
},
},
+ {
+ name: "recovery with no-op clone commit",
+ res: sreconcile.ResultSuccess,
+ resErr: nil,
+ commit: partialCommit,
+ oldObjBeforeFunc: func(obj *sourcev1.GitRepository) {
+ obj.Status.Artifact = &sourcev1.Artifact{Revision: "xxx", Checksum: "yyy"}
+ conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "fail")
+ conditions.MarkFalse(obj, meta.ReadyCondition, meta.FailedReason, "foo")
+ },
+ newObjBeforeFunc: func(obj *sourcev1.GitRepository) {
+ obj.Status.Artifact = &sourcev1.Artifact{Revision: "aaa", Checksum: "bbb"}
+ conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "ready")
+ },
+ wantEvent: "Normal NewArtifact stored artifact for commit 'main/some-hash'",
+ },
}
for _, tt := range tests {
@@ -1857,11 +1932,9 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {
reconciler := &GitRepositoryReconciler{
EventRecorder: recorder,
+ features: features.FeatureGates(),
}
- commit := &git.Commit{
- Message: "test commit",
- }
- reconciler.notify(oldObj, newObj, *commit, tt.res, tt.resErr)
+ reconciler.notify(oldObj, newObj, tt.commit, tt.res, tt.resErr)
select {
case x, ok := <-recorder.Events:
diff --git a/controllers/suite_test.go b/controllers/suite_test.go
index 9ca821381..750b0afc0 100644
--- a/controllers/suite_test.go
+++ b/controllers/suite_test.go
@@ -36,6 +36,7 @@ import (
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
"github.com/fluxcd/source-controller/internal/cache"
+ "github.com/fluxcd/source-controller/internal/features"
// +kubebuilder:scaffold:imports
)
@@ -106,6 +107,7 @@ func TestMain(m *testing.M) {
EventRecorder: record.NewFakeRecorder(32),
Metrics: testMetricsH,
Storage: testStorage,
+ features: features.FeatureGates(),
}).SetupWithManager(testEnv); err != nil {
panic(fmt.Sprintf("Failed to start GitRepositoryReconciler: %v", err))
}
diff --git a/docs/spec/v1beta2/gitrepositories.md b/docs/spec/v1beta2/gitrepositories.md
index 2d95db474..412dd0c2c 100644
--- a/docs/spec/v1beta2/gitrepositories.md
+++ b/docs/spec/v1beta2/gitrepositories.md
@@ -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`.
diff --git a/pkg/git/git.go b/pkg/git/git.go
index cc45498d1..5ce6fb09a 100644
--- a/pkg/git/git.go
+++ b/pkg/git/git.go
@@ -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
}
diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go
index 9d9d94dd8..5b67b23bd 100644
--- a/pkg/git/git_test.go
+++ b/pkg/git/git_test.go
@@ -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))
+ })
+ }
+}
diff --git a/pkg/git/gogit/checkout.go b/pkg/git/gogit/checkout.go
index afa4afbf8..3b19f83de 100644
--- a/pkg/git/gogit/checkout.go
+++ b/pkg/git/gogit/checkout.go
@@ -22,6 +22,7 @@ import (
"fmt"
"io"
"sort"
+ "strings"
"time"
"github.com/Masterminds/semver/v3"
@@ -78,10 +79,15 @@ 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 reconcilation",
- 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
+ ss := strings.Split(currentRevision, "/")
+ c := &git.Commit{
+ Hash: git.Hash(ss[len(ss)-1]),
+ Reference: ref.String(),
}
+ return c, nil
}
}
@@ -153,10 +159,15 @@ 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 reconcilation",
- 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
+ ss := strings.Split(currentRevision, "/")
+ c := &git.Commit{
+ Hash: git.Hash(ss[len(ss)-1]),
+ Reference: ref.String(),
}
+ return c, nil
}
}
repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{
diff --git a/pkg/git/gogit/checkout_test.go b/pkg/git/gogit/checkout_test.go
index ba5d28231..0fa4c1610 100644
--- a/pkg/git/gogit/checkout_test.go
+++ b/pkg/git/gogit/checkout_test.go
@@ -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 reconcilation: 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,58 +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))
+ // Check checked out content for actual checkouts only.
+ 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 reconcilation",
+ 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: "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: "Annotated",
- tag: "annotated",
- annotated: true,
- checkoutTag: "annotated",
- expectTag: "annotated",
+ name: "Skip clone - last revision unchanged",
+ tagsInRepo: []testTag{{"tag-1", false}},
+ checkoutTag: "tag-1",
+ lastRevTag: "tag-1",
+ expectConcreteCommit: false,
},
{
- name: "Non existing tag",
- tag: "tag-1",
- checkoutTag: "invalid",
- expectErr: "couldn't find remote ref \"refs/tags/invalid\"",
+ name: "Last revision changed",
+ tagsInRepo: []testTag{{"tag-1", false}, {"tag-2", false}},
+ checkoutTag: "tag-2",
+ lastRevTag: "tag-1",
+ expectConcreteCommit: true,
},
}
for _, tt := range tests {
@@ -183,32 +194,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 +232,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))
+ }
})
}
}
diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go
index 9dc233fea..b09122769 100644
--- a/pkg/git/libgit2/checkout.go
+++ b/pkg/git/libgit2/checkout.go
@@ -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
}
}
}
diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go
index 98b57b24e..d5f7b1796 100644
--- a/pkg/git/libgit2/checkout_test.go
+++ b/pkg/git/libgit2/checkout_test.go
@@ -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"},
+ expectedCommit: secondCommit.String(),
+ lastRevision: fmt.Sprintf("%s/%s", defaultBranch, firstCommit.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{}
+
+ // 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(), tr.annotated, tr.name, time.Now())
+ if err != nil {
+ t.Fatal(err)
+ }
+ 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 != "" {
- 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)
- }
- }
+ // 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))
}
})
}
diff --git a/pkg/git/options.go b/pkg/git/options.go
index b5e8f2c41..ff1bccac1 100644
--- a/pkg/git/options.go
+++ b/pkg/git/options.go
@@ -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
}