Skip to content

Commit

Permalink
gogit: Add new ForceGoGitImplementation FeatureGate
Browse files Browse the repository at this point in the history
ForceGoGitImplementation ignores the value set for gitImplementation
and ensures that go-git is used for all GitRepository objects.
This can be used to confirm that Flux instances won't break if/when
the libgit2 implementation was to be deprecated.

When enabled, libgit2 won't be initialized, nor will any git2go cgo
code be called.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
  • Loading branch information
Paulo Gomes committed Nov 2, 2022
1 parent a079f52 commit 9a9fa2c
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 19 deletions.
18 changes: 14 additions & 4 deletions controllers/gitrepository_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,8 +427,13 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context,
// change, it short-circuits the whole reconciliation with an early return.
func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
gitImplementation := obj.Spec.GitImplementation
if enabled, _ := r.features[features.ForceGoGitImplementation]; enabled {
gitImplementation = sourcev1.GoGitImplementation
}

// Exit early, if we need to use libgit2 AND managed transport hasn't been intialized.
if !r.Libgit2TransportInitialized() && obj.Spec.GitImplementation == sourcev1.LibGit2Implementation {
if !r.Libgit2TransportInitialized() && gitImplementation == sourcev1.LibGit2Implementation {
return sreconcile.ResultEmpty, serror.NewStalling(
errors.New("libgit2 managed transport not initialized"), "Libgit2TransportNotEnabled",
)
Expand Down Expand Up @@ -758,18 +763,23 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context,
var gitReader git.RepositoryReader
var err error

switch obj.Spec.GitImplementation {
gitImplementation := obj.Spec.GitImplementation
if enabled, _ := r.features[features.ForceGoGitImplementation]; enabled {
gitImplementation = sourcev1.GoGitImplementation
}

switch gitImplementation {
case sourcev1.LibGit2Implementation:
gitReader, err = libgit2.NewClient(dir, authOpts)
case sourcev1.GoGitImplementation:
gitReader, err = gogit.NewClient(dir, authOpts)
default:
err = fmt.Errorf("invalid Git implementation: %s", obj.Spec.GitImplementation)
err = fmt.Errorf("invalid Git implementation: %s", gitImplementation)
}
if err != nil {
// Do not return err as recovery without changes is impossible.
e := serror.NewStalling(
fmt.Errorf("failed to create Git client for implementation '%s': %w", obj.Spec.GitImplementation, err),
fmt.Errorf("failed to create Git client for implementation '%s': %w", gitImplementation, err),
sourcev1.GitOperationFailedReason,
)
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error())
Expand Down
30 changes: 18 additions & 12 deletions controllers/gitrepository_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,12 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
}

r := &GitRepositoryReconciler{
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: features.FeatureGates(),
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: map[string]bool{
features.ForceGoGitImplementation: false,
},
Libgit2TransportInitialized: transport.Enabled,
}

Expand Down Expand Up @@ -543,10 +545,12 @@ func TestGitRepositoryReconciler_reconcileSource_libgit2TransportUninitialized(t
g := NewWithT(t)

r := &GitRepositoryReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(runtime.NewScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: features.FeatureGates(),
Client: fakeclient.NewClientBuilder().WithScheme(runtime.NewScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: map[string]bool{
features.ForceGoGitImplementation: false,
},
Libgit2TransportInitialized: mockTransportNotInitialized,
}

Expand Down Expand Up @@ -727,10 +731,12 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
}

r := &GitRepositoryReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(runtime.NewScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: features.FeatureGates(),
Client: fakeclient.NewClientBuilder().WithScheme(runtime.NewScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: map[string]bool{
features.ForceGoGitImplementation: false,
},
Libgit2TransportInitialized: transport.Enabled,
}

Expand Down
9 changes: 9 additions & 0 deletions internal/features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,21 @@ const (
// the last revision is still the same at the target repository,
// and if that is so, skips the reconciliation.
OptimizedGitClones = "OptimizedGitClones"
// ForceGoGitImplementation ignores the value set for gitImplementation
// and ensures that go-git is used for all GitRepository objects.
//
// When enabled, libgit2 won't be initialized, nor will any git2go cgo
// code be called.
ForceGoGitImplementation = "ForceGoGitImplementation"
)

var features = map[string]bool{
// OptimizedGitClones
// opt-out from v0.25
OptimizedGitClones: true,
// ForceGoGitImplementation
// opt-in from v0.32
ForceGoGitImplementation: true,
}

// DefaultFeatureGates contains a list of all supported feature gates and
Expand Down
8 changes: 5 additions & 3 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,11 @@ func main() {
}
storage := mustInitStorage(storagePath, storageAdvAddr, artifactRetentionTTL, artifactRetentionRecords, setupLog)

if err = transport.InitManagedTransport(); err != nil {
// Log the error, but don't exit so as to not block reconcilers that are healthy.
setupLog.Error(err, "unable to initialize libgit2 managed transport")
if gogitOnly, _ := features.Enabled(features.ForceGoGitImplementation); !gogitOnly {
if err = transport.InitManagedTransport(); err != nil {
// Log the error, but don't exit so as to not block reconcilers that are healthy.
setupLog.Error(err, "unable to initialize libgit2 managed transport")
}
}

if err = (&controllers.GitRepositoryReconciler{
Expand Down

0 comments on commit 9a9fa2c

Please sign in to comment.