Merge pull request #665 from pjbgf/optimise-clone

Optimise clone operations

Author: Paulo Gomes

controllers/gitrepository_controller.go

@@ -48,6 +48,7 @@ import (
 
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
 	serror "github.com/fluxcd/source-controller/internal/error"
+	"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/internal/util"
@@ -311,8 +312,9 @@ func (r *GitRepositoryReconciler) notify(oldObj, newObj *sourcev1.GitRepository,
 // reconcileStorage ensures the current state of the storage matches the
 // desired and previously observed state.
 //
-// All Artifacts for the object except for the current one in the Status are
-// garbage collected from the Storage.
+// The garbage collection is executed based on the flag based settings and
+// may remove files that are beyond their TTL or the maximum number of files
+// to survive a collection cycle.
 // If the Artifact in the Status of the object disappeared from the Storage,
 // it is removed from the object.
 // If the object does not have an Artifact in its Status, a Reconciling
@@ -411,6 +413,13 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 		checkoutOpts.Tag = ref.Tag
 		checkoutOpts.SemVer = ref.SemVer
 	}
+
+	if oc, _ := features.Enabled(features.OptimizedGitClones); oc {
+		if artifact := obj.GetArtifact(); artifact != nil {
+			checkoutOpts.LastRevision = artifact.Revision
+		}
+	}
+
 	checkoutStrategy, err := strategy.CheckoutStrategyForImplementation(ctx,
 		git.Implementation(obj.Spec.GitImplementation), checkoutOpts)
 	if err != nil {
@@ -455,6 +464,12 @@ 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,
@@ -495,6 +510,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 // object are set, and the symlink in the Storage is updated to its path.
 func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
 	obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
+
 	// Create potential new artifact with current available metadata
 	artifact := r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), commit.String(), fmt.Sprintf("%s.tar.gz", commit.Hash.String()))
 

controllers/gitrepository_controller_test.go

@@ -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"),
 			},
 		},
 		{

docs/spec/v1beta2/gitrepositories.md

@@ -399,6 +399,22 @@ transport being handled by the controller, instead of `libgit2`.
 This may lead to an increased number of timeout messages in the logs, however
 it will fix the bug in which Git operations make the controllers hang indefinitely.
 
+#### Optimized Git clones
+
+Optimized Git clones decreases resource utilization for GitRepository
+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.
+
+This feature is enabled by default. It can be disabled by starting the
+controller with the argument `--feature-gates=OptimizedGitClones=false`.
+
+NB: GitRepository objects configured for SemVer or Commit clones are
+not affected by this functionality.
+
 #### Proxy support
 
 When a proxy is configured in the source-controller Pod through the appropriate

go.mod

@@ -25,7 +25,7 @@ require (
 	github.com/fluxcd/pkg/gitutil v0.1.0
 	github.com/fluxcd/pkg/helmtestserver v0.7.2
 	github.com/fluxcd/pkg/lockedfile v0.1.0
-	github.com/fluxcd/pkg/runtime v0.14.2
+	github.com/fluxcd/pkg/runtime v0.15.1
 	github.com/fluxcd/pkg/ssh v0.3.3
 	github.com/fluxcd/pkg/testserver v0.2.0
 	github.com/fluxcd/pkg/untar v0.1.0
@@ -185,7 +185,7 @@ require (
 	github.com/shopspring/decimal v1.2.0 // indirect
 	github.com/sirupsen/logrus v1.8.1 // indirect
 	github.com/spf13/cast v1.4.1 // indirect
-	github.com/spf13/cobra v1.3.0 // indirect
+	github.com/spf13/cobra v1.4.0 // indirect
 	github.com/stretchr/testify v1.7.1 // indirect
 	github.com/xanzy/ssh-agent v0.3.1 // indirect
 	github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect

go.sum

@@ -364,8 +364,8 @@ github.com/fluxcd/pkg/helmtestserver v0.7.2/go.mod h1:WtUXBrfpJdwK54LX1Tqd8PpLJY
 github.com/fluxcd/pkg/lockedfile v0.1.0 h1:YsYFAkd6wawMCcD74ikadAKXA4s2sukdxrn7w8RB5eo=
 github.com/fluxcd/pkg/lockedfile v0.1.0/go.mod h1:EJLan8t9MiOcgTs8+puDjbE6I/KAfHbdvIy9VUgIjm8=
 github.com/fluxcd/pkg/runtime v0.13.0-rc.6/go.mod h1:4oKUO19TeudXrnCRnxCfMSS7EQTYpYlgfXwlQuDJ/Eg=
-github.com/fluxcd/pkg/runtime v0.14.2 h1:ktyUjcX4pHoC8DRoBmhEP6eMHbmR6+/MYoARe4YulZY=
-github.com/fluxcd/pkg/runtime v0.14.2/go.mod h1:NZr3PRK7xX2M1bl0LdtugvQyWkOmu2NcW3NrZH6U0is=
+github.com/fluxcd/pkg/runtime v0.15.1 h1:PKooYqlZM+KLhnNz10sQnBH0AHllS40PIDHtiRH/BGU=
+github.com/fluxcd/pkg/runtime v0.15.1/go.mod h1:TPAoOEgUFG60FXBA4ID41uaPldxuXCEI4jt3qfd5i5Q=
 github.com/fluxcd/pkg/ssh v0.3.3 h1:/tc7W7LO1VoVUI5jB+p9ZHCA+iQaXTkaSCDZJsxcZ9k=
 github.com/fluxcd/pkg/ssh v0.3.3/go.mod h1:+bKhuv0/pJy3HZwkK54Shz68sNv1uf5aI6wtPaEHaYk=
 github.com/fluxcd/pkg/testserver v0.2.0 h1:Mj0TapmKaywI6Fi5wvt1LAZpakUHmtzWQpJNKQ0Krt4=
@@ -990,8 +990,9 @@ github.com/spf13/cobra v0.0.6/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHN
 github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE=
 github.com/spf13/cobra v1.1.3/go.mod h1:pGADOWyqRD/YMrPZigI/zbliZ2wVD/23d+is3pSWzOo=
 github.com/spf13/cobra v1.2.1/go.mod h1:ExllRjgxM/piMAM+3tAZvg8fsklGAf3tPfi+i8t68Nk=
-github.com/spf13/cobra v1.3.0 h1:R7cSvGu+Vv+qX0gW5R/85dx2kmmJT5z5NM8ifdYjdn0=
 github.com/spf13/cobra v1.3.0/go.mod h1:BrRVncBjOJa/eUcVVm9CE+oC6as8k+VYr4NY7WCi9V4=
+github.com/spf13/cobra v1.4.0 h1:y+wJpx64xcgO1V+RcnwW0LEHxTKRi2ZDPSBjWnrg88Q=
+github.com/spf13/cobra v1.4.0/go.mod h1:Wo4iy3BUC+X2Fybo0PDqwJIv3dNRiZLHQymsfxlB84g=
 github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo=
 github.com/spf13/jwalterweatherman v1.1.0/go.mod h1:aNWZUN0dPAAO/Ljvb5BEdw96iTZ0EXowPYD95IqWIGo=
 github.com/spf13/pflag v0.0.0-20170130214245-9ff6c6923cff/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=

internal/features/features.go

@@ -0,0 +1,54 @@
+/*
+Copyright 2022 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package features sets the feature gates that
+// source-controller supports, and their default
+// states.
+package features
+
+import feathelper "github.com/fluxcd/pkg/runtime/features"
+
+const (
+	// OptimizedGitClones decreases resource utilization for GitRepository
+	// 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.
+	OptimizedGitClones = "OptimizedGitClones"
+)
+
+var features = map[string]bool{
+	// OptimizedGitClones
+	// opt-out from v0.25
+	OptimizedGitClones: true,
+}
+
+// DefaultFeatureGates contains a list of all supported feature gates and
+// their default values.
+func FeatureGates() map[string]bool {
+	return features
+}
+
+// Enabled verifies whether the feature is enabled or not.
+//
+// This is only a wrapper around the Enabled func in
+// pkg/runtime/features, so callers won't need to import
+// both packages for checking whether a feature is enabled.
+func Enabled(feature string) (bool, error) {
+	return feathelper.Enabled(feature)
+}

main.go

@@ -36,10 +36,12 @@ import (
 	"github.com/fluxcd/pkg/runtime/client"
 	helper "github.com/fluxcd/pkg/runtime/controller"
 	"github.com/fluxcd/pkg/runtime/events"
+	feathelper "github.com/fluxcd/pkg/runtime/features"
 	"github.com/fluxcd/pkg/runtime/leaderelection"
 	"github.com/fluxcd/pkg/runtime/logger"
 	"github.com/fluxcd/pkg/runtime/pprof"
 	"github.com/fluxcd/pkg/runtime/probes"
+	"github.com/fluxcd/source-controller/internal/features"
 
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
 	"github.com/fluxcd/source-controller/controllers"
@@ -88,6 +90,7 @@ func main() {
 		logOptions               logger.Options
 		leaderElectionOptions    leaderelection.Options
 		rateLimiterOptions       helper.RateLimiterOptions
+		featureGates             feathelper.FeatureGates
 		helmCacheMaxSize         int
 		helmCacheTTL             string
 		helmCachePurgeInterval   string
@@ -136,11 +139,20 @@ func main() {
 	logOptions.BindFlags(flag.CommandLine)
 	leaderElectionOptions.BindFlags(flag.CommandLine)
 	rateLimiterOptions.BindFlags(flag.CommandLine)
+	featureGates.BindFlags(flag.CommandLine)
 
 	flag.Parse()
 
 	ctrl.SetLogger(logger.NewLogger(logOptions))
 
+	err := featureGates.WithLogger(setupLog).
+		SupportedFeatures(features.FeatureGates())
+
+	if err != nil {
+		setupLog.Error(err, "unable to load feature gates")
+		os.Exit(1)
+	}
+
 	// Set upper bound file size limits Helm
 	helm.MaxIndexSize = helmIndexLimit
 	helm.MaxChartSize = helmChartLimit

pkg/git/git.go

@@ -106,3 +106,15 @@ func (c *Commit) ShortMessage() string {
 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)
+}

pkg/git/gogit/checkout.go

@@ -26,8 +26,11 @@ import (
 
 	"github.com/Masterminds/semver/v3"
 	extgogit "github.com/go-git/go-git/v5"
+	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/object"
+	"github.com/go-git/go-git/v5/plumbing/transport"
+	"github.com/go-git/go-git/v5/storage/memory"
 
 	"github.com/fluxcd/pkg/gitutil"
 	"github.com/fluxcd/pkg/version"
@@ -44,27 +47,44 @@ func CheckoutStrategyForOptions(_ context.Context, opts git.CheckoutOptions) git
 	case opts.SemVer != "":
 		return &CheckoutSemVer{SemVer: opts.SemVer, RecurseSubmodules: opts.RecurseSubmodules}
 	case opts.Tag != "":
-		return &CheckoutTag{Tag: opts.Tag, RecurseSubmodules: opts.RecurseSubmodules}
+		return &CheckoutTag{Tag: opts.Tag, RecurseSubmodules: opts.RecurseSubmodules, LastRevision: opts.LastRevision}
 	default:
 		branch := opts.Branch
 		if branch == "" {
 			branch = git.DefaultBranch
 		}
-		return &CheckoutBranch{Branch: branch, RecurseSubmodules: opts.RecurseSubmodules}
+		return &CheckoutBranch{Branch: branch, RecurseSubmodules: opts.RecurseSubmodules, LastRevision: opts.LastRevision}
 	}
 }
 
 type CheckoutBranch struct {
 	Branch            string
 	RecurseSubmodules bool
+	LastRevision      string
 }
 
 func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) {
 	authMethod, err := transportAuth(opts)
 	if err != nil {
 		return nil, fmt.Errorf("failed to construct auth method with options: %w", err)
 	}
+
 	ref := plumbing.NewBranchReferenceName(c.Branch)
+	// check if previous revision has changed before attempting to clone
+	if c.LastRevision != "" {
+		currentRevision, err := getLastRevision(ctx, url, ref, opts, authMethod)
+		if err != nil {
+			return nil, err
+		}
+
+		if currentRevision != "" && currentRevision == c.LastRevision {
+			return nil, git.NoChangesError{
+				Message:          "no changes since last reconcilation",
+				ObservedRevision: currentRevision,
+			}
+		}
+	}
+
 	repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{
 		URL:               url,
 		Auth:              authMethod,
@@ -92,9 +112,31 @@ func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *g
 	return buildCommitWithRef(cc, ref)
 }
 
+func getLastRevision(ctx context.Context, url string, ref plumbing.ReferenceName, opts *git.AuthOptions, authMethod transport.AuthMethod) (string, error) {
+	config := &config.RemoteConfig{
+		Name: git.DefaultOrigin,
+		URLs: []string{url},
+	}
+	rem := extgogit.NewRemote(memory.NewStorage(), config)
+	listOpts := &extgogit.ListOptions{
+		Auth: authMethod,
+	}
+	if opts != nil && opts.CAFile != nil {
+		listOpts.CABundle = opts.CAFile
+	}
+	refs, err := rem.ListContext(ctx, listOpts)
+	if err != nil {
+		return "", fmt.Errorf("unable to list remote for '%s': %w", url, err)
+	}
+
+	currentRevision := filterRefs(refs, ref)
+	return currentRevision, nil
+}
+
 type CheckoutTag struct {
 	Tag               string
 	RecurseSubmodules bool
+	LastRevision      string
 }
 
 func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) {
@@ -103,6 +145,20 @@ func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.
 		return nil, fmt.Errorf("failed to construct auth method with options: %w", err)
 	}
 	ref := plumbing.NewTagReferenceName(c.Tag)
+	// check if previous revision has changed before attempting to clone
+	if c.LastRevision != "" {
+		currentRevision, err := getLastRevision(ctx, url, ref, opts, authMethod)
+		if err != nil {
+			return nil, err
+		}
+
+		if currentRevision != "" && currentRevision == c.LastRevision {
+			return nil, git.NoChangesError{
+				Message:          "no changes since last reconcilation",
+				ObservedRevision: currentRevision,
+			}
+		}
+	}
 	repo, err := extgogit.PlainCloneContext(ctx, path, false, &extgogit.CloneOptions{
 		URL:               url,
 		Auth:              authMethod,
@@ -333,3 +389,13 @@ func recurseSubmodules(recurse bool) extgogit.SubmoduleRescursivity {
 	}
 	return extgogit.NoRecurseSubmodules
 }
+
+func filterRefs(refs []*plumbing.Reference, currentRef plumbing.ReferenceName) string {
+	for _, ref := range refs {
+		if ref.Name().String() == currentRef.String() {
+			return fmt.Sprintf("%s/%s", currentRef.Short(), ref.Hash().String())
+		}
+	}
+
+	return ""
+}