From e7ff6b20d371d6e3db88fb0c2a08f5e88774f875 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Fri, 10 Apr 2020 18:52:37 +0300 Subject: [PATCH] Add ready condition helpers --- api/v1alpha1/condition_types.go | 3 + api/v1alpha1/gitrepository_types.go | 15 ++-- api/v1alpha1/helmrepository_types.go | 7 +- controllers/conditions.go | 44 +++++++++ controllers/gitrepository_controller.go | 108 +++++++---------------- controllers/helmrepository_controller.go | 71 +++------------ controllers/predicate.go | 16 ++++ controllers/storage.go | 16 ++++ 8 files changed, 135 insertions(+), 145 deletions(-) create mode 100644 controllers/conditions.go diff --git a/api/v1alpha1/condition_types.go b/api/v1alpha1/condition_types.go index 480840713..128f32f14 100644 --- a/api/v1alpha1/condition_types.go +++ b/api/v1alpha1/condition_types.go @@ -37,4 +37,7 @@ 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" ) diff --git a/api/v1alpha1/gitrepository_types.go b/api/v1alpha1/gitrepository_types.go index 1a9fb7500..8d19af637 100644 --- a/api/v1alpha1/gitrepository_types.go +++ b/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" +) diff --git a/api/v1alpha1/helmrepository_types.go b/api/v1alpha1/helmrepository_types.go index d492ab5f4..68db5f9e4 100644 --- a/api/v1alpha1/helmrepository_types.go +++ b/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"` @@ -76,6 +77,6 @@ func init() { const ( InvalidHelmRepositoryURLReason string = "InvalidHelmRepositoryURL" - IndexFetchFailedReason string = "IndexFetchFailedReason" + IndexFetchFailedReason string = "IndexFetchFailed" IndexFetchSucceededReason string = "IndexFetchSucceed" ) diff --git a/controllers/conditions.go b/controllers/conditions.go new file mode 100644 index 000000000..f151ae67c --- /dev/null +++ b/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, + } +} diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 97b6c4251..2d82c1a1f 100644 --- a/controllers/gitrepository_controller.go +++ b/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) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "ExecFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("tmp dir error %w", err) + return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err } 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) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "GitCloneFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("git clone error %w", err) + return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err } // 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) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "GitCloneFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("semver parse range error %w", err) + return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err } repoTags, err := repo.Tags() if err != nil { - ex := fmt.Errorf("git list tags error %w", err) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "GitCloneFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("git list tags error %w", err) + return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err } 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) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "GitCheckoutFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("git worktree error %w", err) + return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err } err = w.Checkout(&git.CheckoutOptions{ Hash: plumbing.NewHash(commit), }) if err != nil { - ex := fmt.Errorf("git checkout error %w", err) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "GitCheckoutFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("git checkout error %w", err) + return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err } } else { - ex := fmt.Errorf("no match found for semver %s", repository.Spec.SemVer) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "GitCheckoutFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("no match found for semver %s", repository.Spec.SemVer) + return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err } } // read commit hash ref, err := repo.Head() if err != nil { - ex := fmt.Errorf("git resolve HEAD error %w", err) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "GitHeadFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("git resolve HEAD error %w", err) + return NotReadyCondition(sourcev1.GitOperationFailedReason, err.Error()), "", err } 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) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "ExecFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("mkdir dir error %w", err) + return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err } + // 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) - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: "ExecFailed", - Message: ex.Error(), - }, "", ex + err = fmt.Errorf("storage error %w", err) + return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err } - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionTrue, - Reason: "GitCloneSucceed", - Message: fmt.Sprintf("Artifact is available at %s", artifact.Path), - }, artifact.URL, nil + message := fmt.Sprintf("Artifact is available at %s", artifact.Path) + return ReadyCondition(sourcev1.GitOperationSucceedReason, message), artifact.URL, nil } func (r *GitRepositoryReconciler) shouldResetStatus(repository sourcev1.GitRepository) (bool, sourcev1.GitRepositoryStatus) { diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index f2511388f..24b3d988b 100644 --- a/controllers/helmrepository_controller.go +++ b/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{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: sourcev1.InvalidHelmRepositoryURLReason, - Message: err.Error(), - }, "", err + return NotReadyCondition(sourcev1.InvalidHelmRepositoryURLReason, err.Error()), "", err } c, err := r.Getters.ByScheme(u.Scheme) if err != nil { - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: sourcev1.InvalidHelmRepositoryURLReason, - Message: err.Error(), - }, "", err + return NotReadyCondition(sourcev1.InvalidHelmRepositoryURLReason, 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{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: sourcev1.IndexFetchFailedReason, - Message: err.Error(), - }, "", err + return NotReadyCondition(sourcev1.IndexFetchFailedReason, err.Error()), "", err } data, err := ioutil.ReadAll(res) if err != nil { - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: sourcev1.IndexFetchFailedReason, - Message: err.Error(), - }, "", err + return NotReadyCondition(sourcev1.IndexFetchFailedReason, err.Error()), "", err } i := &repo.IndexFile{} if err := yaml.Unmarshal(data, i); err != nil { - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: sourcev1.IndexFetchFailedReason, - Message: err.Error(), - }, "", err + return NotReadyCondition(sourcev1.IndexFetchFailedReason, err.Error()), "", err } index, err := yaml.Marshal(i) if err != nil { - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: sourcev1.IndexFetchFailedReason, - Message: err.Error(), - }, "", err + return NotReadyCondition(sourcev1.IndexFetchFailedReason, 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{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: sourcev1.IndexFetchFailedReason, - Message: err.Error(), - }, "", err + return NotReadyCondition(sourcev1.StorageOperationFailedReason, 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{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: sourcev1.IndexFetchFailedReason, - Message: err.Error(), - }, "", err + return NotReadyCondition(sourcev1.StorageOperationFailedReason, 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{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionFalse, - Reason: sourcev1.IndexFetchFailedReason, - Message: err.Error(), - }, "", err + return NotReadyCondition(sourcev1.StorageOperationFailedReason, err.Error()), "", err } - return sourcev1.SourceCondition{ - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionTrue, - Reason: sourcev1.IndexFetchSucceededReason, - Message: fmt.Sprintf("Artifact is available at %s", artifact.Path), - }, artifact.URL, nil + message := fmt.Sprintf("Artifact is available at %s", artifact.Path) + return ReadyCondition(sourcev1.IndexFetchSucceededReason, message), artifact.URL, nil } func (r *HelmRepositoryReconciler) shouldResetStatus(repository sourcev1.HelmRepository) (bool, sourcev1.HelmRepositoryStatus) { diff --git a/controllers/predicate.go b/controllers/predicate.go index 8ede12a9a..aee7e2afe 100644 --- a/controllers/predicate.go +++ b/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 ( diff --git a/controllers/storage.go b/controllers/storage.go index d78d28d5f..f52ea2496 100644 --- a/controllers/storage.go +++ b/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 (