Refactor BundleCreationMaxConcurrency in GitJob

Author: thardeck

internal/cmd/controller/gitops/operator.go

@@ -23,7 +23,6 @@ import (
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
 	command "github.com/rancher/fleet/internal/cmd"
-	fleetapply "github.com/rancher/fleet/internal/cmd/cli/apply"
 	"github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler"
 	fcreconciler "github.com/rancher/fleet/internal/cmd/controller/reconciler"
 	"github.com/rancher/fleet/internal/metrics"
@@ -139,27 +138,21 @@ func (g *GitOperator) Run(cmd *cobra.Command, args []string) error {
 		workers = w
 	}
 
-	bundleCreationMaxConcurrency, err := fleetapply.GetBundleCreationMaxConcurrency()
-	if err != nil {
-		setupLog.Error(err, "failed to parse FLEET_BUNDLE_CREATION_MAX_CONCURRENCY, using defaults", "env_var_name", fleetapply.BundleCreationMaxConcurrencyEnv)
-	}
-
 	kh := ssh.KnownHosts{EnforceHostKeyChecks: !g.SkipHostKeyChecks}
 
 	gitJobReconciler := &reconciler.GitJobReconciler{
-		Client:                       mgr.GetClient(),
-		Scheme:                       mgr.GetScheme(),
-		Image:                        g.Image,
-		Scheduler:                    sched,
-		Workers:                      workers,
-		ShardID:                      g.ShardID,
-		JobNodeSelector:              g.ShardNodeSelector,
-		GitFetcher:                   &git.Fetch{KnownHosts: kh},
-		Clock:                        reconciler.RealClock{},
-		Recorder:                     mgr.GetEventRecorderFor(fmt.Sprintf("fleet-gitops%s", shardIDSuffix)),
-		SystemNamespace:              namespace,
-		KnownHosts:                   kh,
-		BundleCreationMaxConcurrency: bundleCreationMaxConcurrency,
+		Client:          mgr.GetClient(),
+		Scheme:          mgr.GetScheme(),
+		Image:           g.Image,
+		Scheduler:       sched,
+		Workers:         workers,
+		ShardID:         g.ShardID,
+		JobNodeSelector: g.ShardNodeSelector,
+		GitFetcher:      &git.Fetch{KnownHosts: kh},
+		Clock:           reconciler.RealClock{},
+		Recorder:        mgr.GetEventRecorderFor(fmt.Sprintf("fleet-gitops%s", shardIDSuffix)),
+		SystemNamespace: namespace,
+		KnownHosts:      kh,
 	}
 
 	statusReconciler := &reconciler.StatusReconciler{

internal/cmd/controller/gitops/reconciler/gitjob.go

@@ -413,7 +413,7 @@ func (r *GitJobReconciler) newJobSpec(ctx context.Context, gitrepo *v1alpha1.Git
 
 	saName := names.SafeConcatName("git", gitrepo.Name)
 	logger := log.FromContext(ctx)
-	args, envs := argsAndEnvs(gitrepo, logger, CACertsFilePathOverride, r.KnownHosts, drivenScanSeparator, helmInsecure, helmBasicHTTP, r.BundleCreationMaxConcurrency)
+	args, envs := argsAndEnvs(gitrepo, logger, CACertsFilePathOverride, r.KnownHosts, drivenScanSeparator, helmInsecure, helmBasicHTTP)
 
 	zero := int32(0)
 
@@ -603,6 +603,16 @@ func (r *GitJobReconciler) newGitCloner(
 	}, nil
 }
 
+// readIntEnvVar reads an integer from an environment variable using the provided getter function.
+// If an error occurs, it logs the error and returns the default value.
+func readIntEnvVar(logger logr.Logger, getter func() (int, error), envVarName string) int {
+	val, err := getter()
+	if err != nil {
+		logger.Error(err, "failed parsing env variable, using defaults", "env_var_name", envVarName)
+	}
+	return val
+}
+
 func argsAndEnvs(
 	gitrepo *v1alpha1.GitRepo,
 	logger logr.Logger,
@@ -611,7 +621,6 @@ func argsAndEnvs(
 	drivenScanSeparator string,
 	helmInsecureSkipTLS bool,
 	helmBasicHTTP bool,
-	bundleCreationMaxConcurrency int,
 ) ([]string, []corev1.EnvVar) {
 	args := []string{
 		"fleet",
@@ -654,10 +663,9 @@ func argsAndEnvs(
 		}
 	}
 
-	fleetApplyRetries, err := fleetapply.GetOnConflictRetries()
-	if err != nil {
-		logger.Error(err, "failed parsing env variable, using defaults", "env_var_name", fleetapply.FleetApplyConflictRetriesEnv)
-	}
+	fleetApplyRetries := readIntEnvVar(logger, fleetapply.GetOnConflictRetries, fleetapply.FleetApplyConflictRetriesEnv)
+	bundleCreationMaxConcurrency := readIntEnvVar(logger, fleetapply.GetBundleCreationMaxConcurrency, fleetapply.BundleCreationMaxConcurrencyEnv)
+
 	env := []corev1.EnvVar{
 		{
 			Name:  "HOME",
@@ -676,7 +684,7 @@ func argsAndEnvs(
 			Value: strconv.Itoa(fleetApplyRetries),
 		},
 		{
-			Name:  "FLEET_BUNDLE_CREATION_MAX_CONCURRENCY",
+			Name:  fleetapply.BundleCreationMaxConcurrencyEnv,
 			Value: strconv.Itoa(bundleCreationMaxConcurrency),
 		},
 	}

internal/cmd/controller/gitops/reconciler/gitjob_controller.go

@@ -115,18 +115,17 @@ type KnownHostsGetter interface {
 // GitJobReconciler reconciles a GitRepo resource to create a git cloning k8s job
 type GitJobReconciler struct {
 	client.Client
-	Scheme                       *runtime.Scheme
-	Image                        string
-	Scheduler                    quartz.Scheduler
-	Workers                      int
-	ShardID                      string
-	JobNodeSelector              string
-	GitFetcher                   GitFetcher
-	Clock                        TimeGetter
-	Recorder                     record.EventRecorder
-	SystemNamespace              string
-	KnownHosts                   KnownHostsGetter
-	BundleCreationMaxConcurrency int
+	Scheme          *runtime.Scheme
+	Image           string
+	Scheduler       quartz.Scheduler
+	Workers         int
+	ShardID         string
+	JobNodeSelector string
+	GitFetcher      GitFetcher
+	Clock           TimeGetter
+	Recorder        record.EventRecorder
+	SystemNamespace string
+	KnownHosts      KnownHostsGetter
 }
 
 func (r *GitJobReconciler) SetupWithManager(mgr ctrl.Manager) error {

internal/cmd/controller/gitops/reconciler/gitjob_test.go

@@ -2134,17 +2134,11 @@ func TestGenerateJob_EnvVars(t *testing.T) {
 				}
 			}
 
-			bundleCreationMaxConcurrency, err := fleetapply.GetBundleCreationMaxConcurrency()
-			if err != nil {
-				t.Logf("failed to parse FLEET_BUNDLE_CREATION_MAX_CONCURRENCY, using defaults")
-			}
-
 			r := GitJobReconciler{
-				Client:                       getFakeClient([]corev1.Toleration{}),
-				Image:                        "test",
-				Clock:                        RealClock{},
-				SystemNamespace:              config.DefaultNamespace,
-				BundleCreationMaxConcurrency: bundleCreationMaxConcurrency,
+				Client:          getFakeClient([]corev1.Toleration{}),
+				Image:           "test",
+				Clock:           RealClock{},
+				SystemNamespace: config.DefaultNamespace,
 				KnownHosts: mockKnownHostsGetter{
 					strict: test.strictSSHHostKeyChecks,
 				},