From 7e99998c5f0ddc8f64830d292bf4c55fa5bcffff Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Sun, 12 Apr 2020 21:03:32 +0200 Subject: [PATCH] Factor out injection of kind string in controllers --- controllers/gitrepository_controller.go | 39 ++++++++++++------------ controllers/helmchart_controller.go | 17 +++++++---- controllers/helmrepository_controller.go | 33 +++++++++++--------- controllers/storage.go | 2 +- main.go | 3 -- 5 files changed, 51 insertions(+), 43 deletions(-) diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index de225032..36217de7 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -38,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -50,7 +51,6 @@ type GitRepositoryReconciler struct { Log logr.Logger Scheme *runtime.Scheme Storage *Storage - Kind string } // +kubebuilder:rbac:groups=source.fluxcd.io,resources=gitrepositories,verbs=get;list;watch;create;update;patch;delete @@ -60,16 +60,16 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() - log := r.Log.WithValues(r.Kind, req.NamespacedName) - var repo sourcev1.GitRepository if err := r.Get(ctx, req.NamespacedName, &repo); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + log := r.Log.WithValues(repo.Kind, req.NamespacedName) + // set initial status if reset, status := r.shouldResetStatus(repo); reset { - log.Info("Initializing repository") + log.Info("Initializing Git repository") repo.Status = status if err := r.Status().Update(ctx, &repo); err != nil { log.Error(err, "unable to update GitRepository status") @@ -83,7 +83,7 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro // try git clone syncedRepo, err := r.sync(*repo.DeepCopy()) if err != nil { - log.Info("Repository sync failed", "error", err.Error()) + log.Info("Git repository sync failed", "error", err.Error()) } // update status @@ -92,7 +92,7 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro return ctrl.Result{Requeue: true}, err } - log.Info("Repository sync succeeded", "msg", sourcev1.GitRepositoryReadyMessage(syncedRepo)) + log.Info("Git repository sync succeeded", "msg", sourcev1.GitRepositoryReadyMessage(syncedRepo)) // requeue repository return ctrl.Result{RequeueAfter: repo.Spec.Interval.Duration}, nil @@ -104,16 +104,21 @@ func (r *GitRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error { WithEventFilter(RepositoryChangePredicate{}). WithEventFilter(predicate.Funcs{ DeleteFunc: func(e event.DeleteEvent) bool { + gvk, err := apiutil.GVKForObject(e.Object, r.Scheme) + if err != nil { + r.Log.Error(err, "unable to get GroupVersionKind for deleted object") + return false + } // delete artifacts - artifact := r.Storage.ArtifactFor(r.Kind, e.Meta, "dummy", "") + artifact := r.Storage.ArtifactFor(gvk.Kind, e.Meta, "", "") if err := r.Storage.RemoveAll(artifact); err != nil { r.Log.Error(err, "unable to delete artifacts", - r.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) + gvk.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) } else { - r.Log.Info("Repository artifacts deleted", - r.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) + r.Log.Info("Git repository artifacts deleted", + gvk.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) } - return false + return true }, }). Complete(r) @@ -199,7 +204,7 @@ func (r *GitRepositoryReconciler) sync(repository sourcev1.GitRepository) (sourc Force: true, }) if err != nil { - err = fmt.Errorf("git checkout %s for %s error: %w", commit, branch, err) + err = fmt.Errorf("git checkout '%s' for '%s' error: %w", commit, branch, err) return sourcev1.GitRepositoryNotReady(repository, sourcev1.GitOperationFailedReason, err.Error()), err } } else if exp := repository.Spec.Reference.SemVer; exp != "" { @@ -222,7 +227,7 @@ func (r *GitRepositoryReconciler) sync(repository sourcev1.GitRepository) (sourc }) svTags := make(map[string]string) - svers := []semver.Version{} + var svers []semver.Version for tag, _ := range tags { v, _ := semver.ParseTolerant(tag) if rng(v) { @@ -269,7 +274,7 @@ func (r *GitRepositoryReconciler) sync(repository sourcev1.GitRepository) (sourc revision = fmt.Sprintf("%s/%s", branch, ref.Hash().String()) } - artifact := r.Storage.ArtifactFor(r.Kind, repository.ObjectMeta.GetObjectMeta(), + artifact := r.Storage.ArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), fmt.Sprintf("%s.tar.gz", ref.Hash().String()), revision) // create artifact dir @@ -301,7 +306,7 @@ func (r *GitRepositoryReconciler) sync(repository sourcev1.GitRepository) (sourc return sourcev1.GitRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err } - message := fmt.Sprintf("Artifact is available at: %s", artifact.Path) + message := fmt.Sprintf("Git repoistory artifacts are available at: %s", artifact.Path) return sourcev1.GitRepositoryReady(repository, artifact, url, sourcev1.GitOperationSucceedReason, message), nil } @@ -361,13 +366,9 @@ func (r *GitRepositoryReconciler) auth(repository sourcev1.GitRepository, tmp st auth := &http.BasicAuth{} if username, ok := credentials["username"]; ok { auth.Username = string(username) - } else { - return nil, fmt.Errorf("%s secret does not contain username", repository.Spec.SecretRef.Name) } if password, ok := credentials["password"]; ok { auth.Password = string(password) - } else { - return nil, fmt.Errorf("%s secret does not contain password", repository.Spec.SecretRef.Name) } if auth.Username == "" || auth.Password == "" { diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 6b7a8f76..f065db7b 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -33,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/yaml" @@ -46,7 +47,6 @@ type HelmChartReconciler struct { Log logr.Logger Scheme *runtime.Scheme Storage *Storage - Kind string Getters getter.Providers } @@ -90,7 +90,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { // try to pull chart pulledChart, err := r.sync(repository, *chart.DeepCopy()) if err != nil { - log.Info("Helm chart pull failed", "error", err.Error()) + log.Info("Helm chart sync failed", "error", err.Error()) } // update status @@ -111,16 +111,21 @@ func (r *HelmChartReconciler) SetupWithManager(mgr ctrl.Manager) error { WithEventFilter(RepositoryChangePredicate{}). WithEventFilter(predicate.Funcs{ DeleteFunc: func(e event.DeleteEvent) bool { + gvk, err := apiutil.GVKForObject(e.Object, r.Scheme) + if err != nil { + r.Log.Error(err, "unable to get GroupVersionKind for deleted object") + return false + } // delete artifacts - artifact := r.Storage.ArtifactFor(r.Kind, e.Meta, "", "") + artifact := r.Storage.ArtifactFor(gvk.Kind, e.Meta, "", "") if err := r.Storage.RemoveAll(artifact); err != nil { r.Log.Error(err, "unable to delete artifacts", - r.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) + gvk.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) } else { r.Log.Info("Helm chart artifacts deleted", - r.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) + gvk.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) } - return false + return true }, }). Complete(r) diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index bf24f490..89ffa34f 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -32,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/yaml" @@ -45,7 +46,6 @@ type HelmRepositoryReconciler struct { Log logr.Logger Scheme *runtime.Scheme Storage *Storage - Kind string Getters getter.Providers } @@ -56,16 +56,16 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() - log := r.Log.WithValues(r.Kind, req.NamespacedName) - var repository sourcev1.HelmRepository if err := r.Get(ctx, req.NamespacedName, &repository); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + log := r.Log.WithValues(repository.Kind, req.NamespacedName) + // set initial status if reset, status := r.shouldResetStatus(repository); reset { - log.Info("Initializing repository") + log.Info("Initializing Helm repository") repository.Status = status if err := r.Status().Update(ctx, &repository); err != nil { log.Error(err, "unable to update HelmRepository status") @@ -79,7 +79,7 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err // try to download index syncedRepo, err := r.sync(*repository.DeepCopy()) if err != nil { - log.Info("Helm repository index failed", "error", err.Error()) + log.Info("Helm repository sync failed", "error", err.Error()) } // update status @@ -88,7 +88,7 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err return ctrl.Result{Requeue: true}, err } - log.Info("Repository sync succeeded", "msg", sourcev1.HelmRepositoryReadyMessage(syncedRepo)) + log.Info("Helm repository sync succeeded", "msg", sourcev1.HelmRepositoryReadyMessage(syncedRepo)) // requeue repository return ctrl.Result{RequeueAfter: repository.Spec.Interval.Duration}, nil @@ -100,16 +100,21 @@ func (r *HelmRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error { WithEventFilter(RepositoryChangePredicate{}). WithEventFilter(predicate.Funcs{ DeleteFunc: func(e event.DeleteEvent) bool { + gvk, err := apiutil.GVKForObject(e.Object, r.Scheme) + if err != nil { + r.Log.Error(err, "unable to get GroupVersionKind for deleted object") + return false + } // delete artifacts - artifact := r.Storage.ArtifactFor(r.Kind, e.Meta, "", "") + artifact := r.Storage.ArtifactFor(gvk.Kind, e.Meta, "", "") if err := r.Storage.RemoveAll(artifact); err != nil { r.Log.Error(err, "unable to delete artifacts", - r.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) + gvk.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) } else { - r.Log.Info("Repository artifacts deleted", - r.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) + r.Log.Info("Helm repository artifacts deleted", + gvk.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName())) } - return false + return true }, }). Complete(r) @@ -152,7 +157,7 @@ func (r *HelmRepositoryReconciler) sync(repository sourcev1.HelmRepository) (sou } sum := r.Storage.Checksum(index) - artifact := r.Storage.ArtifactFor(r.Kind, repository.ObjectMeta.GetObjectMeta(), + artifact := r.Storage.ArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), fmt.Sprintf("index-%s.yaml", sum), sum) // create artifact dir @@ -180,11 +185,11 @@ func (r *HelmRepositoryReconciler) sync(repository sourcev1.HelmRepository) (sou // update index symlink indexUrl, err := r.Storage.Symlink(artifact, "index.yaml") if err != nil { - err = fmt.Errorf("storage error %w", err) + err = fmt.Errorf("storage error: %w", err) return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err } - message := fmt.Sprintf("Index is available at %s", artifact.Path) + message := fmt.Sprintf("Helm repository index is available at: %s", artifact.Path) return sourcev1.HelmRepositoryReady(repository, artifact, indexUrl, sourcev1.IndexationSucceededReason, message), nil } diff --git a/controllers/storage.go b/controllers/storage.go index f9198cc2..6d94e9d9 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -48,7 +48,7 @@ type Storage struct { // NewStorage creates the storage helper for a given path and hostname func NewStorage(basePath string, hostname string, timeout time.Duration) (*Storage, error) { if f, err := os.Stat(basePath); os.IsNotExist(err) || !f.IsDir() { - return nil, fmt.Errorf("invalid dir path %s", basePath) + return nil, fmt.Errorf("invalid dir path: %s", basePath) } return &Storage{ diff --git a/main.go b/main.go index d5285598..898227c6 100644 --- a/main.go +++ b/main.go @@ -92,7 +92,6 @@ func main() { Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("GitRepository"), Scheme: mgr.GetScheme(), - Kind: "gitrepository", Storage: storage, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "GitRepository") @@ -102,7 +101,6 @@ func main() { Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("HelmRepository"), Scheme: mgr.GetScheme(), - Kind: "helmrepository", Storage: storage, Getters: getters, }).SetupWithManager(mgr); err != nil { @@ -113,7 +111,6 @@ func main() { Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("HelmChart"), Scheme: mgr.GetScheme(), - Kind: "helmchart", Storage: storage, Getters: getters, }).SetupWithManager(mgr); err != nil {