diff --git a/api/v1alpha1/gitrepository_types.go b/api/v1alpha1/gitrepository_types.go index c2560564..323667af 100644 --- a/api/v1alpha1/gitrepository_types.go +++ b/api/v1alpha1/gitrepository_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1alpha1 import ( + "time" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -40,6 +42,11 @@ type GitRepositorySpec struct { // +required Interval metav1.Duration `json:"interval"` + // The timeout for remote git operations like cloning. + // +kubebuilder:validation:Default=20s + // +optional + Timeout *metav1.Duration `json:"timeout,omitempty"` + // The git reference to checkout and monitor for changes, defaults to // master branch. // +optional @@ -105,6 +112,10 @@ const ( GitOperationFailedReason string = "GitOperationFailed" ) +// GitRepositoryReady sets the given artifact and url on the +// HelmRepository and resets the conditions to SourceCondition of +// type Ready with status true and the given reason and message. +// It returns the modified GitRepository. func GitRepositoryReady(repository GitRepository, artifact Artifact, url, reason, message string) GitRepository { repository.Status.Conditions = []SourceCondition{ { @@ -128,6 +139,9 @@ func GitRepositoryReady(repository GitRepository, artifact Artifact, url, reason return repository } +// GitRepositoryNotReady resets the conditions of the HelmRepository +// to SourceCondition of type Ready with status false and the given +// reason and message. It returns the modified GitRepository. func GitRepositoryNotReady(repository GitRepository, reason, message string) GitRepository { repository.Status.Conditions = []SourceCondition{ { @@ -141,15 +155,25 @@ func GitRepositoryNotReady(repository GitRepository, reason, message string) Git return repository } +// ReadyMessage returns the message of the SourceCondition +// of type Ready with status true if present, or an empty string. func GitRepositoryReadyMessage(repository GitRepository) string { for _, condition := range repository.Status.Conditions { - if condition.Type == ReadyCondition { + if condition.Type == ReadyCondition && condition.Status == corev1.ConditionTrue { return condition.Message } } return "" } +// GetTimeout returns the configured timeout or the default. +func (in *GitRepository) GetTimeout() time.Duration { + if in.Spec.Timeout != nil { + return in.Spec.Timeout.Duration + } + return time.Second * 20 +} + // GetArtifact returns the latest artifact from the source // if present in the status sub-resource. func (in *GitRepository) GetArtifact() *Artifact { diff --git a/api/v1alpha1/helmchart_types.go b/api/v1alpha1/helmchart_types.go index b61f82c4..0f3bef06 100644 --- a/api/v1alpha1/helmchart_types.go +++ b/api/v1alpha1/helmchart_types.go @@ -66,6 +66,10 @@ const ( ChartPullSucceededReason string = "ChartPullSucceeded" ) +// HelmChartReady sets the given artifact and url on the HelmChart +// and resets the conditions to SourceCondition of type Ready with +// status true and the given reason and message. It returns the +// modified HelmChart. func HelmChartReady(chart HelmChart, artifact Artifact, url, reason, message string) HelmChart { chart.Status.Conditions = []SourceCondition{ { @@ -89,6 +93,9 @@ func HelmChartReady(chart HelmChart, artifact Artifact, url, reason, message str return chart } +// HelmChartNotReady resets the conditions of the HelmChart to +// SourceCondition of type Ready with status false and the given +// reason and message. It returns the modified HelmChart. func HelmChartNotReady(chart HelmChart, reason, message string) HelmChart { chart.Status.Conditions = []SourceCondition{ { @@ -102,6 +109,8 @@ func HelmChartNotReady(chart HelmChart, reason, message string) HelmChart { return chart } +// HelmChartReadyMessage returns the message of the SourceCondition +// of type Ready with status true if present, or an empty string. func HelmChartReadyMessage(chart HelmChart) string { for _, condition := range chart.Status.Conditions { if condition.Type == ReadyCondition && condition.Status == corev1.ConditionTrue { diff --git a/api/v1alpha1/helmrepository_types.go b/api/v1alpha1/helmrepository_types.go index aab539eb..17c9985c 100644 --- a/api/v1alpha1/helmrepository_types.go +++ b/api/v1alpha1/helmrepository_types.go @@ -65,6 +65,10 @@ const ( IndexationSucceededReason string = "IndexationSucceed" ) +// HelmRepositoryReady sets the given artifact and url on the +// HelmRepository and resets the conditions to SourceCondition of +// type Ready with status true and the given reason and message. +// It returns the modified HelmRepository. func HelmRepositoryReady(repository HelmRepository, artifact Artifact, url, reason, message string) HelmRepository { repository.Status.Conditions = []SourceCondition{ { @@ -88,6 +92,9 @@ func HelmRepositoryReady(repository HelmRepository, artifact Artifact, url, reas return repository } +// HelmRepositoryNotReady resets the conditions of the HelmRepository +// to SourceCondition of type Ready with status false and the given +// reason and message. It returns the modified HelmRepository. func HelmRepositoryNotReady(repository HelmRepository, reason, message string) HelmRepository { repository.Status.Conditions = []SourceCondition{ { @@ -101,6 +108,8 @@ func HelmRepositoryNotReady(repository HelmRepository, reason, message string) H return repository } +// HelmRepositoryReadyMessage returns the message of the SourceCondition +// of type Ready with status true if present, or an empty string. func HelmRepositoryReadyMessage(repository HelmRepository) string { for _, condition := range repository.Status.Conditions { if condition.Type == ReadyCondition && condition.Status == corev1.ConditionTrue { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index cabc171b..f97d00a3 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -22,6 +22,7 @@ package v1alpha1 import ( "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -124,6 +125,11 @@ func (in *GitRepositorySpec) DeepCopyInto(out *GitRepositorySpec) { **out = **in } out.Interval = in.Interval + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(metav1.Duration) + **out = **in + } if in.Reference != nil { in, out := &in.Reference, &out.Reference *out = new(GitRepositoryRef) diff --git a/config/crd/bases/source.fluxcd.io_gitrepositories.yaml b/config/crd/bases/source.fluxcd.io_gitrepositories.yaml index 7fdd6fec..3b1ac754 100644 --- a/config/crd/bases/source.fluxcd.io_gitrepositories.yaml +++ b/config/crd/bases/source.fluxcd.io_gitrepositories.yaml @@ -82,6 +82,9 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + timeout: + description: The timeout for remote git operations like cloning. + type: string url: description: The repository URL, can be a HTTP or SSH address. pattern: ^(http|https|ssh):// diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 2f968c8a..b2a58bce 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -21,7 +21,6 @@ import ( "fmt" "io/ioutil" "os" - "time" "github.com/blang/semver" "github.com/go-git/go-git/v5" @@ -52,8 +51,7 @@ type GitRepositoryReconciler struct { // +kubebuilder:rbac:groups=source.fluxcd.io,resources=gitrepositories/status,verbs=get;update;patch func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() + ctx := context.Background() var repo sourcev1.GitRepository if err := r.Get(ctx, req.NamespacedName, &repo); err != nil { @@ -174,7 +172,8 @@ func (r *GitRepositoryReconciler) sync(ctx context.Context, repository sourcev1. defer os.RemoveAll(tmpGit) // clone to tmp - repo, err := git.PlainClone(tmpGit, false, &git.CloneOptions{ + gitCtx, cancel := context.WithTimeout(ctx, repository.GetTimeout()) + repo, err := git.PlainCloneContext(gitCtx, tmpGit, false, &git.CloneOptions{ URL: repository.Spec.URL, Auth: auth, RemoteName: "origin", @@ -186,6 +185,7 @@ func (r *GitRepositoryReconciler) sync(ctx context.Context, repository sourcev1. Progress: nil, Tags: tagMode, }) + cancel() if err != nil { err = fmt.Errorf("git clone error: %w", err) return sourcev1.GitRepositoryNotReady(repository, sourcev1.GitOperationFailedReason, err.Error()), err @@ -350,6 +350,8 @@ func (r *GitRepositoryReconciler) sync(ctx context.Context, repository sourcev1. return sourcev1.GitRepositoryReady(repository, artifact, url, sourcev1.GitOperationSucceedReason, message), nil } +// shouldResetStatus returns a boolean indicating if the status of the +// given repository should be reset and a reset HelmChartStatus. func (r *GitRepositoryReconciler) shouldResetStatus(repository sourcev1.GitRepository) (bool, sourcev1.GitRepositoryStatus) { resetStatus := false if repository.Status.Artifact != nil { @@ -374,6 +376,8 @@ func (r *GitRepositoryReconciler) shouldResetStatus(repository sourcev1.GitRepos } } +// gc performs a garbage collection on all but current artifacts of +// the given repository. func (r *GitRepositoryReconciler) gc(repository sourcev1.GitRepository) error { if repository.Status.Artifact != nil { return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 1ab4114d..7f755227 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -21,7 +21,6 @@ import ( "fmt" "io/ioutil" "net/url" - "time" "github.com/go-logr/logr" "helm.sh/helm/v3/pkg/getter" @@ -52,8 +51,7 @@ type HelmChartReconciler struct { // +kubebuilder:rbac:groups=source.fluxcd.io,resources=helmcharts/status,verbs=get;update;patch func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() + ctx := context.Background() var chart sourcev1.HelmChart if err := r.Get(ctx, req.NamespacedName, &chart); err != nil { @@ -78,7 +76,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } // get referenced chart repository - repository, err := r.chartRepository(ctx, chart) + repository, err := r.getChartRepositoryWithArtifact(ctx, chart) if err != nil { chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error()) if err := r.Status().Update(ctx, &chart); err != nil { @@ -94,7 +92,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } // try to pull chart - pulledChart, err := r.sync(repository, *chart.DeepCopy()) + pulledChart, err := r.sync(ctx, repository, *chart.DeepCopy()) if err != nil { log.Error(err, "Helm chart sync failed") if err := r.Status().Update(ctx, &pulledChart); err != nil { @@ -132,7 +130,7 @@ func (r *HelmChartReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts Complete(r) } -func (r *HelmChartReconciler) sync(repository sourcev1.HelmRepository, chart sourcev1.HelmChart) (sourcev1.HelmChart, error) { +func (r *HelmChartReconciler) sync(ctx context.Context, repository sourcev1.HelmRepository, chart sourcev1.HelmChart) (sourcev1.HelmChart, error) { indexBytes, err := ioutil.ReadFile(repository.Status.Artifact.Path) if err != nil { err = fmt.Errorf("failed to read Helm repository index file: %w", err) @@ -182,7 +180,7 @@ func (r *HelmChartReconciler) sync(repository sourcev1.HelmRepository, chart sou } var secret corev1.Secret - err := r.Client.Get(context.TODO(), name, &secret) + err := r.Client.Get(ctx, name, &secret) if err != nil { err = fmt.Errorf("auth secret error: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err @@ -199,6 +197,8 @@ func (r *HelmChartReconciler) sync(repository sourcev1.HelmRepository, chart sou clientOpts = opts } + // TODO(hidde): implement timeout from the HelmRepository + // https://github.com/helm/helm/pull/7950 res, err := c.Get(u.String(), clientOpts...) if err != nil { return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err @@ -246,7 +246,10 @@ func (r *HelmChartReconciler) sync(repository sourcev1.HelmRepository, chart sou return sourcev1.HelmChartReady(chart, artifact, chartUrl, sourcev1.ChartPullSucceededReason, message), nil } -func (r *HelmChartReconciler) chartRepository(ctx context.Context, chart sourcev1.HelmChart) (sourcev1.HelmRepository, error) { +// getChartRepositoryWithArtifact attempts to get the ChartRepository +// for the given chart. It returns an error if the HelmRepository could +// not be retrieved or if does not have an artifact. +func (r *HelmChartReconciler) getChartRepositoryWithArtifact(ctx context.Context, chart sourcev1.HelmChart) (sourcev1.HelmRepository, error) { if chart.Spec.HelmRepositoryRef.Name == "" { return sourcev1.HelmRepository{}, fmt.Errorf("no HelmRepository reference given") } @@ -270,6 +273,8 @@ func (r *HelmChartReconciler) chartRepository(ctx context.Context, chart sourcev return repository, err } +// shouldResetStatus returns a boolean indicating if the status of the +// given chart should be reset and a reset HelmChartStatus. func (r *HelmChartReconciler) shouldResetStatus(chart sourcev1.HelmChart) (bool, sourcev1.HelmChartStatus) { resetStatus := false if chart.Status.Artifact != nil { @@ -295,6 +300,8 @@ func (r *HelmChartReconciler) shouldResetStatus(chart sourcev1.HelmChart) (bool, } } +// gc performs a garbage collection on all but current artifacts of +// the given chart. func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart) error { if chart.Status.Artifact != nil { return r.Storage.RemoveAllButCurrent(*chart.Status.Artifact) @@ -302,11 +309,14 @@ func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart) error { return nil } +// setOwnerRef appends the owner reference of the given chart to the +// repository if it is not present. func (r *HelmChartReconciler) setOwnerRef(ctx context.Context, chart *sourcev1.HelmChart, repository sourcev1.HelmRepository) error { - if !metav1.IsControlledBy(chart.GetObjectMeta(), repository.GetObjectMeta()) { - chart.SetOwnerReferences(append(chart.GetOwnerReferences(), - *metav1.NewControllerRef(repository.GetObjectMeta(), repository.GroupVersionKind()))) - return r.Update(ctx, chart) + if metav1.IsControlledBy(chart.GetObjectMeta(), repository.GetObjectMeta()) { + return nil } - return nil + chart.SetOwnerReferences(append(chart.GetOwnerReferences(), *metav1.NewControllerRef( + repository.GetObjectMeta(), repository.GroupVersionKind(), + ))) + return r.Update(ctx, chart) } diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 3b097438..d54e081e 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -22,7 +22,6 @@ import ( "io/ioutil" "net/url" "path" - "time" "github.com/go-logr/logr" "helm.sh/helm/v3/pkg/getter" @@ -54,8 +53,7 @@ type HelmRepositoryReconciler struct { // +kubebuilder:rbac:groups=source.fluxcd.io,resources=helmcharts/finalizers,verbs=get;update;patch func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() + ctx := context.Background() var repository sourcev1.HelmRepository if err := r.Get(ctx, req.NamespacedName, &repository); err != nil { @@ -80,7 +78,7 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err } // try to download index - syncedRepo, err := r.sync(*repository.DeepCopy()) + syncedRepo, err := r.sync(ctx, *repository.DeepCopy()) if err != nil { log.Error(err, "Helm repository sync failed") if err := r.Status().Update(ctx, &syncedRepo); err != nil { @@ -117,7 +115,7 @@ func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, Complete(r) } -func (r *HelmRepositoryReconciler) sync(repository sourcev1.HelmRepository) (sourcev1.HelmRepository, error) { +func (r *HelmRepositoryReconciler) sync(ctx context.Context, repository sourcev1.HelmRepository) (sourcev1.HelmRepository, error) { u, err := url.Parse(repository.Spec.URL) if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.URLInvalidReason, err.Error()), err @@ -139,7 +137,7 @@ func (r *HelmRepositoryReconciler) sync(repository sourcev1.HelmRepository) (sou } var secret corev1.Secret - err := r.Client.Get(context.TODO(), name, &secret) + err := r.Client.Get(ctx, name, &secret) if err != nil { err = fmt.Errorf("auth secret error: %w", err) return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err @@ -156,6 +154,8 @@ func (r *HelmRepositoryReconciler) sync(repository sourcev1.HelmRepository) (sou clientOpts = opts } + // TODO(hidde): implement timeout from the HelmRepository + // https://github.com/helm/helm/pull/7950 res, err := c.Get(u.String(), clientOpts...) if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err @@ -214,6 +214,8 @@ func (r *HelmRepositoryReconciler) sync(repository sourcev1.HelmRepository) (sou return sourcev1.HelmRepositoryReady(repository, artifact, indexURL, sourcev1.IndexationSucceededReason, message), nil } +// shouldResetStatus returns a boolean indicating if the status of the +// given repository should be reset and a reset HelmChartStatus. func (r *HelmRepositoryReconciler) shouldResetStatus(repository sourcev1.HelmRepository) (bool, sourcev1.HelmRepositoryStatus) { resetStatus := false if repository.Status.Artifact != nil { @@ -239,6 +241,8 @@ func (r *HelmRepositoryReconciler) shouldResetStatus(repository sourcev1.HelmRep } } +// gc performs a garbage collection on all but current artifacts of +// the given repository. func (r *HelmRepositoryReconciler) gc(repository sourcev1.HelmRepository) error { if repository.Status.Artifact != nil { return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact)