Add ready condition helpers

api/v1alpha1/condition_types.go

@@ -37,4 +37,10 @@ const (
 
 	// InitializingReason represents the fact that a given source is being initialize.
 	InitializingReason string = "Initializing"
+
+	// StorageOperationFailedReason signals a failure caused by a storage operation.
+	StorageOperationFailedReason string = "StorageOperationFailed"
+
+	// URLInvalidReason represents the fact that a given source has an invalid URL.
+	URLInvalidReason string = "URLInvalid"
 )

api/v1alpha1/gitrepository_types.go

@@ -22,12 +22,13 @@ import (
 
 // GitRepositorySpec defines the desired state of GitRepository
 type GitRepositorySpec struct {
-	// +kubebuilder:validation:Pattern="^(http|https|ssh)://"
-
 	// The repository URL, can be a HTTP or SSH address.
-	Url string `json:"url"`
+	// +kubebuilder:validation:Pattern="^(http|https|ssh)://"
+	// +required
+	URL string `json:"url"`
 
 	// The interval at which to check for repository updates.
+	// +required
 	Interval metav1.Duration `json:"interval"`
 
 	// The git branch to checkout, defaults to ('master').
@@ -74,9 +75,8 @@ type GitRepository struct {
 	Status GitRepositoryStatus `json:"status,omitempty"`
 }
 
-// +kubebuilder:object:root=true
-
 // GitRepositoryList contains a list of GitRepository
+// +kubebuilder:object:root=true
 type GitRepositoryList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`
@@ -86,3 +86,8 @@ type GitRepositoryList struct {
 func init() {
 	SchemeBuilder.Register(&GitRepository{}, &GitRepositoryList{})
 }
+
+const (
+	GitOperationSucceedReason string = "GitOperationSucceed"
+	GitOperationFailedReason  string = "GitOperationFailed"
+)

api/v1alpha1/helmrepository_types.go

@@ -24,9 +24,11 @@ import (
 type HelmRepositorySpec struct {
 	// The repository address
 	// +kubebuilder:validation:MinLength=4
+	// +required
 	URL string `json:"url"`
 
 	// The interval at which to check for repository updates
+	// +required
 	Interval metav1.Duration `json:"interval"`
 }
 
@@ -61,9 +63,8 @@ type HelmRepository struct {
 	Status HelmRepositoryStatus `json:"status,omitempty"`
 }
 
-// +kubebuilder:object:root=true
-
 // HelmRepositoryList contains a list of HelmRepository
+// +kubebuilder:object:root=true
 type HelmRepositoryList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`
@@ -75,7 +76,11 @@ func init() {
 }
 
 const (
-	InvalidHelmRepositoryURLReason string = "InvalidHelmRepositoryURL"
+	// IndexationFailedReason represents the fact that the indexation
-	IndexFetchFailedReason         string = "IndexFetchFailedReason"
+	// of the given Helm repository failed.
-	IndexFetchSucceededReason      string = "IndexFetchSucceed"
+	IndexationFailedReason string = "IndexationFailed"
+
+	// IndexationSucceededReason represents the fact that the indexation
+	// of the given Helm repository succeeded.
+	IndexationSucceededReason string = "IndexationSucceed"
 )

controllers/conditions.go

@@ -0,0 +1,44 @@
+/*
+Copyright 2020 The Flux CD contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controllers
+
+import (
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	sourcev1 "github.com/fluxcd/source-controller/api/v1alpha1"
+)
+
+func ReadyCondition(reason, message string) sourcev1.SourceCondition {
+	return sourcev1.SourceCondition{
+		Type:               sourcev1.ReadyCondition,
+		Status:             corev1.ConditionTrue,
+		LastTransitionTime: metav1.Now(),
+		Reason:             reason,
+		Message:            message,
+	}
+}
+
+func NotReadyCondition(reason, message string) sourcev1.SourceCondition {
+	return sourcev1.SourceCondition{
+		Type:               sourcev1.ReadyCondition,
+		Status:             corev1.ConditionFalse,
+		LastTransitionTime: metav1.Now(),
+		Reason:             reason,
+		Message:            message,
+	}
+}

controllers/gitrepository_controller.go

@@ -138,56 +138,36 @@ func (r *GitRepositoryReconciler) sync(repository sourcev1.GitRepository) (sourc
 	// create tmp dir
 	dir, err := ioutil.TempDir("", repository.Name)
 	if err != nil {
-		ex := fmt.Errorf("tmp dir error %w", err)
+		err = fmt.Errorf("tmp dir error %w", err)
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  "ExecFailed",
-			Message: ex.Error(),
-		}, "", ex
 	}
 	defer os.RemoveAll(dir)
 
 	// clone to tmp
 	repo, err := git.PlainClone(dir, false, &git.CloneOptions{
-		URL:           repository.Spec.Url,
+		URL:           repository.Spec.URL,
 		Depth:         2,
 		ReferenceName: refName,
 		SingleBranch:  true,
 		Tags:          git.AllTags,
 	})
 	if err != nil {
-		ex := fmt.Errorf("git clone error %w", err)
+		err = fmt.Errorf("git clone error %w", err)
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  "GitCloneFailed",
-			Message: ex.Error(),
-		}, "", ex
 	}
 
 	// checkout tag based on semver expression
 	if repository.Spec.SemVer != "" {
 		rng, err := semver.ParseRange(repository.Spec.SemVer)
 		if err != nil {
-			ex := fmt.Errorf("semver parse range error %w", err)
+			err = fmt.Errorf("semver parse range error %w", err)
-			return sourcev1.SourceCondition{
+			return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err
-				Type:    sourcev1.ReadyCondition,
-				Status:  corev1.ConditionFalse,
-				Reason:  "GitCloneFailed",
-				Message: ex.Error(),
-			}, "", ex
 		}
 
 		repoTags, err := repo.Tags()
 		if err != nil {
-			ex := fmt.Errorf("git list tags error %w", err)
+			err = fmt.Errorf("git list tags error %w", err)
-			return sourcev1.SourceCondition{
+			return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err
-				Type:    sourcev1.ReadyCondition,
-				Status:  corev1.ConditionFalse,
-				Reason:  "GitCloneFailed",
-				Message: ex.Error(),
-			}, "", ex
 		}
 
 		tags := make(map[string]string)
@@ -214,48 +194,28 @@ func (r *GitRepositoryReconciler) sync(repository sourcev1.GitRepository) (sourc
 
 			w, err := repo.Worktree()
 			if err != nil {
-				ex := fmt.Errorf("git worktree error %w", err)
+				err = fmt.Errorf("git worktree error %w", err)
-				return sourcev1.SourceCondition{
+				return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err
-					Type:    sourcev1.ReadyCondition,
-					Status:  corev1.ConditionFalse,
-					Reason:  "GitCheckoutFailed",
-					Message: ex.Error(),
-				}, "", ex
 			}
 
 			err = w.Checkout(&git.CheckoutOptions{
 				Hash: plumbing.NewHash(commit),
 			})
 			if err != nil {
-				ex := fmt.Errorf("git checkout error %w", err)
+				err = fmt.Errorf("git checkout error %w", err)
-				return sourcev1.SourceCondition{
+				return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err
-					Type:    sourcev1.ReadyCondition,
-					Status:  corev1.ConditionFalse,
-					Reason:  "GitCheckoutFailed",
-					Message: ex.Error(),
-				}, "", ex
 			}
 		} else {
-			ex := fmt.Errorf("no match found for semver %s", repository.Spec.SemVer)
+			err = fmt.Errorf("no match found for semver %s", repository.Spec.SemVer)
-			return sourcev1.SourceCondition{
+			return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err
-				Type:    sourcev1.ReadyCondition,
-				Status:  corev1.ConditionFalse,
-				Reason:  "GitCheckoutFailed",
-				Message: ex.Error(),
-			}, "", ex
 		}
 	}
 
 	// read commit hash
 	ref, err := repo.Head()
 	if err != nil {
-		ex := fmt.Errorf("git resolve HEAD error %w", err)
+		err = fmt.Errorf("git resolve HEAD error %w", err)
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  "GitHeadFailed",
-			Message: ex.Error(),
-		}, "", ex
 	}
 
 	artifact := r.Storage.ArtifactFor(r.Kind, repository.ObjectMeta.GetObjectMeta(),
@@ -264,33 +224,27 @@ func (r *GitRepositoryReconciler) sync(repository sourcev1.GitRepository) (sourc
 	// create artifact dir
 	err = r.Storage.MkdirAll(artifact)
 	if err != nil {
-		ex := fmt.Errorf("mkdir dir error %w", err)
+		err = fmt.Errorf("mkdir dir error %w", err)
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  "ExecFailed",
-			Message: ex.Error(),
-		}, "", ex
 	}
 
+	// acquire lock
+	unlock, err := r.Storage.Lock(artifact)
+	if err != nil {
+		err = fmt.Errorf("unable to acquire lock: %w", err)
+		return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err
+	}
+	defer unlock()
+
 	// archive artifact
 	err = r.Storage.Archive(artifact, dir, "")
 	if err != nil {
-		ex := fmt.Errorf("storage error %w", err)
+		err = fmt.Errorf("storage error %w", err)
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  "ExecFailed",
-			Message: ex.Error(),
-		}, "", ex
 	}
 
-	return sourcev1.SourceCondition{
+	message := fmt.Sprintf("Artifact is available at %s", artifact.Path)
-		Type:    sourcev1.ReadyCondition,
+	return ReadyCondition(sourcev1.GitOperationSucceedReason, message), artifact.URL, nil
-		Status:  corev1.ConditionTrue,
-		Reason:  "GitCloneSucceed",
-		Message: fmt.Sprintf("Artifact is available at %s", artifact.Path),
-	}, artifact.URL, nil
 }
 
 func (r *GitRepositoryReconciler) shouldResetStatus(repository sourcev1.GitRepository) (bool, sourcev1.GitRepositoryStatus) {

controllers/helmrepository_controller.go

@@ -130,22 +130,12 @@ func (r *HelmRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error {
 func (r *HelmRepositoryReconciler) index(repository sourcev1.HelmRepository) (sourcev1.SourceCondition, string, error) {
 	u, err := url.Parse(repository.Spec.URL)
 	if err != nil {
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.URLInvalidReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  sourcev1.InvalidHelmRepositoryURLReason,
-			Message: err.Error(),
-		}, "", err
 	}
 
 	c, err := r.Getters.ByScheme(u.Scheme)
 	if err != nil {
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.URLInvalidReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  sourcev1.InvalidHelmRepositoryURLReason,
-			Message: err.Error(),
-		}, "", err
 	}
 
 	u.RawPath = path.Join(u.RawPath, "index.yaml")
@@ -155,42 +145,22 @@ func (r *HelmRepositoryReconciler) index(repository sourcev1.HelmRepository) (so
 	// TODO(hidde): add authentication config
 	res, err := c.Get(indexURL, getter.WithURL(repository.Spec.URL))
 	if err != nil {
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.IndexationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  sourcev1.IndexFetchFailedReason,
-			Message: err.Error(),
-		}, "", err
 	}
 
 	data, err := ioutil.ReadAll(res)
 	if err != nil {
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.IndexationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  sourcev1.IndexFetchFailedReason,
-			Message: err.Error(),
-		}, "", err
 	}
 
 	i := &repo.IndexFile{}
 	if err := yaml.Unmarshal(data, i); err != nil {
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.IndexationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  sourcev1.IndexFetchFailedReason,
-			Message: err.Error(),
-		}, "", err
 	}
 
 	index, err := yaml.Marshal(i)
 	if err != nil {
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.IndexationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  sourcev1.IndexFetchFailedReason,
-			Message: err.Error(),
-		}, "", err
 	}
 
 	sum := r.Storage.Checksum(index)
@@ -201,24 +171,14 @@ func (r *HelmRepositoryReconciler) index(repository sourcev1.HelmRepository) (so
 	err = r.Storage.MkdirAll(artifact)
 	if err != nil {
 		err = fmt.Errorf("unable to create repository index directory: %w", err)
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  sourcev1.IndexFetchFailedReason,
-			Message: err.Error(),
-		}, "", err
 	}
 
 	// acquire lock
 	unlock, err := r.Storage.Lock(artifact)
 	if err != nil {
 		err = fmt.Errorf("unable to acquire lock: %w", err)
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  sourcev1.IndexFetchFailedReason,
-			Message: err.Error(),
-		}, "", err
 	}
 	defer unlock()
 
@@ -226,20 +186,11 @@ func (r *HelmRepositoryReconciler) index(repository sourcev1.HelmRepository) (so
 	err = r.Storage.WriteFile(artifact, index)
 	if err != nil {
 		err = fmt.Errorf("unable to write repository index file: %w", err)
-		return sourcev1.SourceCondition{
+		return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err
-			Type:    sourcev1.ReadyCondition,
-			Status:  corev1.ConditionFalse,
-			Reason:  sourcev1.IndexFetchFailedReason,
-			Message: err.Error(),
-		}, "", err
 	}
 
-	return sourcev1.SourceCondition{
+	message := fmt.Sprintf("Artifact is available at %s", artifact.Path)
-		Type:    sourcev1.ReadyCondition,
+	return ReadyCondition(sourcev1.IndexationSucceededReason, message), artifact.URL, nil
-		Status:  corev1.ConditionTrue,
-		Reason:  sourcev1.IndexFetchSucceededReason,
-		Message: fmt.Sprintf("Artifact is available at %s", artifact.Path),
-	}, artifact.URL, nil
 }
 
 func (r *HelmRepositoryReconciler) shouldResetStatus(repository sourcev1.HelmRepository) (bool, sourcev1.HelmRepositoryStatus) {

controllers/predicate.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2020 The Flux CD contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package controllers
 
 import (

controllers/storage.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2020 The Flux CD contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package controllers
 
 import (