chore: refactor repository

reposerver/repository/repository.go

@@ -2476,31 +2476,9 @@ func (s *Service) GetHelmCharts(ctx context.Context, q *apiclient.HelmChartsRequ
 }
 
 func (s *Service) TestRepository(ctx context.Context, q *apiclient.TestRepositoryRequest) (*apiclient.TestRepositoryResponse, error) {
-	repo := q.Repo
-	// per Type doc, "git" should be assumed if empty or absent
-	if repo.Type == "" {
-		repo.Type = "git"
-	}
-	checks := map[string]func() error{
-		"git": func() error {
-			return git.TestRepo(repo.Repo, repo.GetGitCreds(s.gitCredsStore), repo.IsInsecure(), repo.IsLFSEnabled(), repo.Proxy)
-		},
-		"helm": func() error {
-			if repo.EnableOCI {
-				if !helm.IsHelmOciRepo(repo.Repo) {
-					return errors.New("OCI Helm repository URL should include hostname and port only")
-				}
-				_, err := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI, repo.Proxy).TestHelmOCI()
-				return err
-			} else {
-				_, err := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI, repo.Proxy).GetIndex(false, s.initConstants.HelmRegistryMaxIndexSize)
-				return err
-			}
-		},
-	}
-	check := checks[repo.Type]
 	apiResp := &apiclient.TestRepositoryResponse{VerifiedRepository: false}
-	err := check()
+	testRepoFunc := s.GetTestRepoFunc(q.Repo)
+	err := testRepoFunc()
 	if err != nil {
 		return apiResp, fmt.Errorf("error testing repository connectivity: %w", err)
 	}
@@ -2779,3 +2757,34 @@ func (s *Service) updateCachedRevision(logCtx *log.Entry, oldRev string, newRev
 	logCtx.Debugf("manifest cache updated for application %s in repo %s from revision %s to revision %s", request.AppName, request.GetRepo().Repo, oldRev, newRev)
 	return nil
 }
+
+type TestRepositoryFunc func() error
+
+func TestGitRepo(repo *v1alpha1.Repository, store git.CredsStore) TestRepositoryFunc {
+	return func() error {
+		return git.TestRepo(repo.Repo, repo.GetGitCreds(store), repo.IsInsecure(), repo.IsLFSEnabled(), repo.Proxy)
+	}
+}
+
+func TestHelmRepo(repo *v1alpha1.Repository, indexSize int64) TestRepositoryFunc {
+	return func() error {
+		if repo.EnableOCI {
+			if !helm.IsHelmOciRepo(repo.Repo) {
+				return errors.New("OCI Helm repository URL should include hostname and port only")
+			}
+			_, err := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI, repo.Proxy).TestHelmOCI()
+			return err
+		} else {
+			_, err := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI, repo.Proxy).GetIndex(false, indexSize)
+			return err
+		}
+	}
+}
+
+func (s *Service) GetTestRepoFunc(repo *v1alpha1.Repository) TestRepositoryFunc {
+	if repo.Type == "helm" {
+		return TestHelmRepo(repo, s.initConstants.HelmRegistryMaxIndexSize)
+	}
+	// default to git
+	return TestGitRepo(repo, s.gitCredsStore)
+}

util/git/client.go

@@ -30,6 +30,8 @@ import (
 	"golang.org/x/crypto/ssh/knownhosts"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	utilnet "k8s.io/apimachinery/pkg/util/net"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/client-go/util/retry"
 
 	"github.com/argoproj/argo-cd/v2/common"
 	certutil "github.com/argoproj/argo-cd/v2/util/cert"
@@ -111,21 +113,14 @@ type runOpts struct {
 }
 
 var (
-	maxAttemptsCount = 1
+	maxAttemptsCount = int64(1)
 	maxRetryDuration time.Duration
 	retryDuration    time.Duration
 	factor           int64
 )
 
 func init() {
-	if countStr := os.Getenv(common.EnvGitAttemptsCount); countStr != "" {
-		if cnt, err := strconv.Atoi(countStr); err != nil {
-			panic(fmt.Sprintf("Invalid value in %s env variable: %v", common.EnvGitAttemptsCount, err))
-		} else {
-			maxAttemptsCount = int(math.Max(float64(cnt), 1))
-		}
-	}
-
+	maxAttemptsCount = env.ParseInt64FromEnv(common.EnvGitAttemptsCount, maxAttemptsCount, 1, math.MaxInt64)
 	maxRetryDuration = env.ParseDurationFromEnv(common.EnvGitRetryMaxDuration, common.DefaultGitRetryMaxDuration, 0, math.MaxInt64)
 	retryDuration = env.ParseDurationFromEnv(common.EnvGitRetryDuration, common.DefaultGitRetryDuration, 0, math.MaxInt64)
 	factor = env.ParseInt64FromEnv(common.EnvGitRetryFactor, common.DefaultGitRetryFactor, 0, math.MaxInt64)
@@ -580,22 +575,23 @@ func (m *nativeGitClient) LsRefs() (*Refs, error) {
 // runs with in-memory storage and is safe to run concurrently, or to be run without a git
 // repository locally cloned.
 func (m *nativeGitClient) LsRemote(revision string) (res string, err error) {
-	for attempt := 0; attempt < maxAttemptsCount; attempt++ {
-		res, err = m.lsRemote(revision)
-		if err == nil {
-			return
-		} else if apierrors.IsInternalError(err) || apierrors.IsTimeout(err) || apierrors.IsServerTimeout(err) ||
-			apierrors.IsTooManyRequests(err) || utilnet.IsProbableEOF(err) || utilnet.IsConnectionReset(err) {
-			// Formula: timeToWait = duration * factor^retry_number
-			// Note that timeToWait should equal to duration for the first retry attempt.
-			// When timeToWait is more than maxDuration retry should be performed at maxDuration.
-			timeToWait := float64(retryDuration) * (math.Pow(float64(factor), float64(attempt)))
-			if maxRetryDuration > 0 {
-				timeToWait = math.Min(float64(maxRetryDuration), timeToWait)
-			}
-			time.Sleep(time.Duration(timeToWait))
-		}
+	retryOpts := wait.Backoff{
+		Duration: retryDuration,
+		Factor:   float64(factor),
+		Jitter:   retry.DefaultRetry.Jitter,
+		Steps:    int(maxAttemptsCount),
+		Cap:      maxRetryDuration,
 	}
+
+	retryable := func(err error) bool {
+		return apierrors.IsInternalError(err) || apierrors.IsTimeout(err) || apierrors.IsServerTimeout(err) ||
+			apierrors.IsTooManyRequests(err) || utilnet.IsProbableEOF(err) || utilnet.IsConnectionReset(err)
+	}
+
+	err = retry.OnError(retryOpts, retryable, func() error {
+		res, err = m.lsRemote(revision)
+		return err
+	})
 	return
 }