Skip to content

api: add timeout field to GitRepositorySpec #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 22, 2020
Merged

api: add timeout field to GitRepositorySpec #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion api/v1alpha1/gitrepository_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand Down Expand Up @@ -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{
{
Expand All @@ -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{
{
Expand All @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions api/v1alpha1/helmchart_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand All @@ -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{
{
Expand All @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions api/v1alpha1/helmrepository_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand All @@ -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{
{
Expand All @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions config/crd/bases/source.fluxcd.io_gitrepositories.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)://
Expand Down
12 changes: 8 additions & 4 deletions controllers/gitrepository_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"fmt"
"io/ioutil"
"os"
"time"

"github.com/blang/semver"
"github.com/go-git/go-git/v5"
Expand Down Expand Up @@ -51,8 +50,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 {
Expand Down Expand Up @@ -164,7 +162,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",
Expand All @@ -176,6 +175,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
Expand Down Expand Up @@ -340,6 +340,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 {
Expand All @@ -364,6 +366,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)
Expand Down
36 changes: 23 additions & 13 deletions controllers/helmchart_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"fmt"
"io/ioutil"
"net/url"
"time"

"github.com/go-logr/logr"
"helm.sh/helm/v3/pkg/getter"
Expand Down Expand Up @@ -51,8 +50,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 {
Expand All @@ -77,7 +75,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 {
Expand All @@ -93,7 +91,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 {
Expand Down Expand Up @@ -122,7 +120,7 @@ func (r *HelmChartReconciler) SetupWithManager(mgr ctrl.Manager) error {
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)
Expand Down Expand Up @@ -172,7 +170,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
Expand All @@ -189,6 +187,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
Expand Down Expand Up @@ -236,7 +236,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")
}
Expand All @@ -260,6 +263,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 {
Expand All @@ -285,18 +290,23 @@ 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)
}
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)
}
16 changes: 10 additions & 6 deletions controllers/helmrepository_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"io/ioutil"
"net/url"
"path"
"time"

"github.com/go-logr/logr"
"helm.sh/helm/v3/pkg/getter"
Expand Down Expand Up @@ -53,8 +52,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 {
Expand All @@ -79,7 +77,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 {
Expand Down Expand Up @@ -107,7 +105,7 @@ func (r *HelmRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error {
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
Expand All @@ -129,7 +127,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
Expand All @@ -146,6 +144,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
Expand Down Expand Up @@ -204,6 +204,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 {
Expand All @@ -229,6 +231,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)
Expand Down