Configurable git client timeout

Fixes #2188

Also fixes an issue in the config reconciler which has prevented the
config from being reloaded when it was changed.

charts/fleet/templates/configmap.yaml

@@ -22,5 +22,6 @@ data:
         "agentNamespace": "{{.Values.bootstrap.agentNamespace}}"
       },
       "webhookReceiverURL": "{{.Values.webhookReceiverURL}}",
-      "githubURLPrefix": "{{.Values.githubURLPrefix}}"
+      "githubURLPrefix": "{{.Values.githubURLPrefix}}",
+      "gitClientTimeout": "{{.Values.gitClientTimeout}}"
     }

charts/fleet/values.yaml

@@ -37,6 +37,10 @@ ignoreClusterRegistrationLabels: false
 # comma separated list of domains or ip addresses that will not use the proxy
 noProxy: 127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,.svc,.cluster.local
 
+# The amount of time to wait for a response from the server before canceling the
+# request.  Used to retrieve the latest commit of configured git repositories.
+gitClientTimeout: 30s
+
 bootstrap:
   enabled: true
   # The namespace that will be autocreated and the local cluster will be registered in

integrationtests/gitjob/controller/controller_test.go

@@ -9,20 +9,20 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-
-	"github.com/rancher/fleet/integrationtests/utils"
-	v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
-	"github.com/rancher/wrangler/v3/pkg/genericcondition"
-	"github.com/rancher/wrangler/v3/pkg/name"
-
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/rancher/wrangler/v3/pkg/genericcondition"
+	"github.com/rancher/wrangler/v3/pkg/name"
+
+	"github.com/rancher/fleet/integrationtests/utils"
+	v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 )
 
 const (
@@ -51,7 +51,6 @@ func checkCondition(gitrepo *v1alpha1.GitRepo, condType string, status corev1.Co
 }
 
 var _ = Describe("GitJob controller", func() {
-
 	When("a new GitRepo is created", func() {
 		var (
 			gitRepo     v1alpha1.GitRepo
@@ -614,7 +613,6 @@ var _ = Describe("GitRepo", func() {
 })
 
 var _ = Describe("GitRepo Status Fields", func() {
-
 	var (
 		gitrepo *v1alpha1.GitRepo
 		bd      *v1alpha1.BundleDeployment

integrationtests/gitjob/controller/suite_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler"
 	ctrlreconciler "github.com/rancher/fleet/internal/cmd/controller/reconciler"
 	"github.com/rancher/fleet/internal/cmd/controller/target"
+	"github.com/rancher/fleet/internal/config"
 	"github.com/rancher/fleet/internal/manifest"
 	v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 	"github.com/rancher/fleet/pkg/git/mocks"
@@ -90,6 +91,8 @@ var _ = BeforeSuite(func() {
 	sched := quartz.NewStdScheduler()
 	Expect(sched).ToNot(BeNil())
 
+	config.Set(&config.Config{})
+
 	err = (&reconciler.GitJobReconciler{
 		Client:     mgr.GetClient(),
 		Scheme:     mgr.GetScheme(),

integrationtests/gitjob/git/git_test.go

@@ -17,28 +17,27 @@ import (
 	dockercontainer "github.com/docker/docker/api/types/container"
 	dockermount "github.com/docker/docker/api/types/mount"
 	gogit "github.com/go-git/go-git/v5"
-	"github.com/go-git/go-git/v5/config"
+	gitconfig "github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/go-git/go-git/v5/plumbing/transport"
 	httpgit "github.com/go-git/go-git/v5/plumbing/transport/http"
-
 	"github.com/gogits/go-gogs-client"
 	cp "github.com/otiai10/copy"
 	"github.com/stretchr/testify/require"
 	"github.com/testcontainers/testcontainers-go"
 	"github.com/testcontainers/testcontainers-go/wait"
 	"go.uber.org/mock/gomock"
 	"golang.org/x/crypto/ssh"
-
-	"github.com/rancher/fleet/internal/mocks"
-	v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
-	"github.com/rancher/fleet/pkg/git"
-
 	v1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/rancher/fleet/internal/config"
+	"github.com/rancher/fleet/internal/mocks"
+	v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
+	"github.com/rancher/fleet/pkg/git"
 )
 
 /*
@@ -111,6 +110,7 @@ func TestLatestCommit_NoAuth(t *testing.T) {
 		},
 	}
 
+	config.Set(&config.Config{})
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
 			f := git.Fetch{}
@@ -466,7 +466,7 @@ func initRepo(url string, name string, private bool) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	cfg.Remotes["upstream"] = &config.RemoteConfig{
+	cfg.Remotes["upstream"] = &gitconfig.RemoteConfig{
 		Name: "upstream",
 		URLs: []string{repoURL},
 	}
@@ -529,7 +529,7 @@ func addRepoCommitAndTag(url string, name string, tag string, tagMessage string)
 	if err != nil {
 		return "", err
 	}
-	cfg.Remotes["upstream"] = &config.RemoteConfig{
+	cfg.Remotes["upstream"] = &gitconfig.RemoteConfig{
 		Name: "upstream",
 		URLs: []string{url},
 	}
@@ -574,7 +574,7 @@ func addRepoCommitAndTag(url string, name string, tag string, tagMessage string)
 			Username: gogsUser,
 			Password: gogsPass,
 		},
-		RefSpecs: []config.RefSpec{config.RefSpec("refs/tags/*:refs/tags/*")},
+		RefSpecs: []gitconfig.RefSpec{gitconfig.RefSpec("refs/tags/*:refs/tags/*")},
 	})
 	if err != nil {
 		return "", nil

internal/cmd/controller/gitops/operator.go

@@ -8,16 +8,8 @@ import (
 	"strconv"
 	"time"
 
-	command "github.com/rancher/fleet/internal/cmd"
-	"github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler"
-	"github.com/rancher/fleet/internal/metrics"
-	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
-	"github.com/rancher/fleet/pkg/git"
-	"github.com/rancher/fleet/pkg/version"
-	"github.com/rancher/fleet/pkg/webhook"
 	"github.com/reugn/go-quartz/quartz"
 	"github.com/spf13/cobra"
-
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -27,8 +19,16 @@ import (
 	clog "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
-
 	"golang.org/x/sync/errgroup"
+
+	command "github.com/rancher/fleet/internal/cmd"
+	"github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler"
+	fcreconciler "github.com/rancher/fleet/internal/cmd/controller/reconciler"
+	"github.com/rancher/fleet/internal/metrics"
+	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
+	"github.com/rancher/fleet/pkg/git"
+	"github.com/rancher/fleet/pkg/version"
+	"github.com/rancher/fleet/pkg/webhook"
 )
 
 var (
@@ -121,7 +121,7 @@ func (g *GitOperator) Run(cmd *cobra.Command, args []string) error {
 		workers = w
 	}
 
-	reconciler := &reconciler.GitJobReconciler{
+	gitJobReconciler := &reconciler.GitJobReconciler{
 		Client:     mgr.GetClient(),
 		Scheme:     mgr.GetScheme(),
 		Image:      g.Image,
@@ -132,14 +132,30 @@ func (g *GitOperator) Run(cmd *cobra.Command, args []string) error {
 		Clock:      reconciler.RealClock{},
 	}
 
+	configReconciler := &fcreconciler.ConfigReconciler{
+		Client:          mgr.GetClient(),
+		Scheme:          mgr.GetScheme(),
+		SystemNamespace: namespace,
+		ShardID:         g.ShardID,
+	}
+
+	if err := fcreconciler.Load(ctx, mgr.GetAPIReader(), namespace); err != nil {
+		setupLog.Error(err, "failed to load config")
+		return err
+	}
+
 	group, ctx := errgroup.WithContext(ctx)
 	group.Go(func() error {
 		return startWebhook(ctx, namespace, g.Listen, mgr.GetClient(), mgr.GetCache())
 	})
 	group.Go(func() error {
-		setupLog.Info("starting manager")
+		setupLog.Info("starting config manager")
+		if err = configReconciler.SetupWithManager(mgr); err != nil {
+			return err
+		}
 
-		if err = reconciler.SetupWithManager(mgr); err != nil {
+		setupLog.Info("starting gitops manager")
+		if err = gitJobReconciler.SetupWithManager(mgr); err != nil {
 			return err
 		}
 

internal/cmd/controller/reconciler/config_controller.go

@@ -58,6 +58,7 @@ func (r *ConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 						object.GetName() == config.ManagerConfigName
 				}),
 				predicate.Or(
+					predicate.ResourceVersionChangedPredicate{},
 					predicate.GenerationChangedPredicate{},
 					predicate.AnnotationChangedPredicate{},
 					predicate.LabelChangedPredicate{},

internal/config/config.go

@@ -108,6 +108,11 @@ type Config struct {
 	// to trust CA bundles from the operating system's store. If set to `strict`, then the agent shall only connect
 	// to a server which uses the exact CA configured when creating/updating the agent.
 	AgentTLSMode string `json:"agentTLSMode,omitempty"`
+
+	// The amount of time to wait for a response from the server before
+	// canceling the request.  Used to retrieve the latest commit of configured
+	// git repositories.
+	GitClientTimeout metav1.Duration `json:"gitClientTimeout,omitempty"`
 }
 
 type Bootstrap struct {

pkg/git/fetch.go

@@ -3,6 +3,7 @@ package git
 import (
 	"context"
 
+	"github.com/rancher/fleet/internal/config"
 	v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 
 	corev1 "k8s.io/api/core/v1"
@@ -46,6 +47,7 @@ func (f *Fetch) LatestCommit(ctx context.Context, gitrepo *v1alpha1.GitRepo, cli
 		CABundle:          gitrepo.Spec.CABundle,
 		Credential:        &secret,
 		InsecureTLSVerify: gitrepo.Spec.InsecureSkipTLSverify,
+		Timeout:           config.Get().GitClientTimeout.Duration,
 		log:               log.FromContext(ctx),
 	})
 	if err != nil {

pkg/git/fetch_test.go

@@ -8,6 +8,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/rancher/fleet/internal/config"
 	fleetv1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 	"github.com/rancher/fleet/pkg/git"
 	corev1 "k8s.io/api/core/v1"
@@ -76,6 +77,9 @@ var _ = Describe("git fetch's LatestCommit tests", func() {
 		})
 
 		It("returns the commit for the expected revision", func() {
+			config.Set(&config.Config{
+				GitClientTimeout: metav1.Duration{Duration: 0},
+			})
 			secret := &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "test-secret",

pkg/git/netutils.go

@@ -16,8 +16,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-var clientTimeout = time.Duration(30)
-
 // GetAuthFromSecret returns the AuthMethod calculated from the given secret
 // The credentials secret is expected to be either basic-auth or ssh-auth (with extra known_hosts data option)
 func GetAuthFromSecret(url string, creds *corev1.Secret) (transport.AuthMethod, error) {
@@ -59,7 +57,7 @@ func GetAuthFromSecret(url string, creds *corev1.Secret) (transport.AuthMethod,
 
 // GetHTTPClientFromSecret returns a HTTP client filled from the information in the given secret
 // and optional CABundle and insecureTLSVerify
-func GetHTTPClientFromSecret(creds *corev1.Secret, CABundle []byte, insecureTLSVerify bool) (*http.Client, error) {
+func GetHTTPClientFromSecret(creds *corev1.Secret, CABundle []byte, insecureTLSVerify bool, timeout time.Duration) (*http.Client, error) {
 	var (
 		username  string
 		password  string
@@ -99,7 +97,7 @@ func GetHTTPClientFromSecret(creds *corev1.Secret, CABundle []byte, insecureTLSV
 
 	client := &http.Client{
 		Transport: transport,
-		Timeout:   clientTimeout * time.Second,
+		Timeout:   timeout,
 	}
 	if username != "" || password != "" {
 		client.Transport = &basicRoundTripper{

pkg/git/netutils_test.go

@@ -3,6 +3,7 @@ package git_test
 import (
 	"encoding/pem"
 	"net/http"
+	"time"
 
 	httpgit "github.com/go-git/go-git/v5/plumbing/transport/http"
 	gossh "github.com/go-git/go-git/v5/plumbing/transport/ssh"
@@ -13,6 +14,8 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
+const gitClientTimeout = time.Second * 30
+
 var _ = Describe("git's GetAuthFromSecret tests", func() {
 	var (
 		secret *corev1.Secret
@@ -252,7 +255,7 @@ YcwLYudAztZeA/A4aM5Y0MA6PlNIeoHohuMkSZNOBcvkNEWdzGBpKb34yLfMarNm
 var _ = Describe("git's GetHTTPClientFromSecret tests", func() {
 	When("using a nil secret, no caBudle and InsecureSkipVerify = false", func() {
 		var caBundle []byte
-		client, err := git.GetHTTPClientFromSecret(nil, caBundle, false)
+		client, err := git.GetHTTPClientFromSecret(nil, caBundle, false, gitClientTimeout)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(client).ToNot(BeNil())
 		expectedTransport, ok := client.Transport.(*http.Transport)
@@ -278,7 +281,7 @@ var _ = Describe("git's GetHTTPClientFromSecret tests", func() {
 
 	When("using a nil secret, no caBudle and InsecureSkipVerify = true", func() {
 		var caBundle []byte
-		client, err := git.GetHTTPClientFromSecret(nil, caBundle, true)
+		client, err := git.GetHTTPClientFromSecret(nil, caBundle, true, gitClientTimeout)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(client).ToNot(BeNil())
 		expectedTransport, ok := client.Transport.(*http.Transport)
@@ -320,7 +323,7 @@ DXZDjC5Ty3zfDBeWUA==
 
 		block, _ := pem.Decode([]byte(caBundlePEM))
 		Expect(block).ToNot(BeNil())
-		client, err := git.GetHTTPClientFromSecret(nil, block.Bytes, true)
+		client, err := git.GetHTTPClientFromSecret(nil, block.Bytes, true, gitClientTimeout)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(client).ToNot(BeNil())
 		expectedTransport, ok := client.Transport.(*http.Transport)
@@ -348,7 +351,7 @@ DXZDjC5Ty3zfDBeWUA==
 		caBundle := []byte(`-----BEGIN CERTIFICATE-----
 SUPER FAKE CERT
 -----END CERTIFICATE-----`)
-		client, err := git.GetHTTPClientFromSecret(nil, caBundle, true)
+		client, err := git.GetHTTPClientFromSecret(nil, caBundle, true, gitClientTimeout)
 		It("returns an error", func() {
 			Expect(err).To(HaveOccurred())
 			Expect(client).To(BeNil())
@@ -366,7 +369,7 @@ SUPER FAKE CERT
 				corev1.BasicAuthPasswordKey: password,
 			},
 		}
-		client, err := git.GetHTTPClientFromSecret(secret, nil, false)
+		client, err := git.GetHTTPClientFromSecret(secret, nil, false, gitClientTimeout)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(client).ToNot(BeNil())
 
@@ -403,7 +406,7 @@ MC4CAQAwBQYDK2VwBCIEINifzf07d9qx3d44e0FSbV4mC/xQxT644RRbpgNpin7I
 				corev1.TLSPrivateKeyKey: []byte(keyPEM),
 			},
 		}
-		client, err := git.GetHTTPClientFromSecret(secret, nil, false)
+		client, err := git.GetHTTPClientFromSecret(secret, nil, false, gitClientTimeout)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(client).ToNot(BeNil())
 
@@ -431,7 +434,7 @@ THIS IS NOT A VALID KEY
 				corev1.TLSPrivateKeyKey: []byte(keyPEM),
 			},
 		}
-		_, err := git.GetHTTPClientFromSecret(secret, nil, false)
+		_, err := git.GetHTTPClientFromSecret(secret, nil, false, gitClientTimeout)
 
 		It("returns an error", func() {
 			Expect(err).To(HaveOccurred())