From 4bbd854012abffcf258177c317444c94a8f12b72 Mon Sep 17 00:00:00 2001 From: Alejandro Ruiz <4057165+aruiz14@users.noreply.github.com> Date: Thu, 12 Dec 2024 13:05:02 +0100 Subject: [PATCH] Calculate Clusters resourceCount from BundleDeployments instead of GitRepos (#3102) --- e2e/single-cluster/status_test.go | 1 + .../gitops/reconciler/status_controller.go | 233 ++++++++++++++++++ .../reconciler/cluster_controller.go | 63 ++--- .../resourcekey.go | 10 +- .../resourcekey_test.go | 4 +- 5 files changed, 276 insertions(+), 35 deletions(-) create mode 100644 internal/cmd/controller/gitops/reconciler/status_controller.go rename internal/{cmd/controller/gitops/reconciler => resourcestatus}/resourcekey.go (95%) rename internal/{cmd/controller/gitops/reconciler => resourcestatus}/resourcekey_test.go (98%) diff --git a/e2e/single-cluster/status_test.go b/e2e/single-cluster/status_test.go index 83c692891d..13d60e44dd 100644 --- a/e2e/single-cluster/status_test.go +++ b/e2e/single-cluster/status_test.go @@ -75,6 +75,7 @@ var _ = Describe("Checks status updates happen for a simple deployment", Ordered out, err := k.Get("cluster", "local", "-n", "fleet-local", "-o", "jsonpath='{.status.display.readyBundles}'") g.Expect(err).ToNot(HaveOccurred(), out) + // Expected 2 bundles instead of just 1 because fleet-agent is also included here g.Expect(out).Should(Equal("'2/2'")) }).Should(Succeed()) }) diff --git a/internal/cmd/controller/gitops/reconciler/status_controller.go b/internal/cmd/controller/gitops/reconciler/status_controller.go new file mode 100644 index 0000000000..cf4d4c3293 --- /dev/null +++ b/internal/cmd/controller/gitops/reconciler/status_controller.go @@ -0,0 +1,233 @@ +package reconciler + +import ( + "context" + "fmt" + "reflect" + "sort" + + "github.com/rancher/fleet/internal/cmd/controller/summary" + "github.com/rancher/fleet/internal/resourcestatus" + v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" + "github.com/rancher/fleet/pkg/durations" + "github.com/rancher/fleet/pkg/sharding" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +type StatusReconciler struct { + client.Client + Scheme *runtime.Scheme + Workers int + ShardID string +} + +func (r *StatusReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&v1alpha1.GitRepo{}). + Watches( + // Fan out from bundle to gitrepo + &v1alpha1.Bundle{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []ctrl.Request { + repo := a.GetLabels()[v1alpha1.RepoLabel] + if repo != "" { + return []ctrl.Request{{ + NamespacedName: types.NamespacedName{ + Namespace: a.GetNamespace(), + Name: repo, + }, + }} + } + + return []ctrl.Request{} + }), + builder.WithPredicates(bundleStatusChangedPredicate()), + ). + WithEventFilter(sharding.FilterByShardID(r.ShardID)). + WithOptions(controller.Options{MaxConcurrentReconciles: r.Workers}). + Named("GitRepoStatus"). + Complete(r) +} + +// Reconcile reads the stat of the GitRepo and BundleDeployments and +// computes status fields for the GitRepo. This status is used to +// display information to the user. +func (r *StatusReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + logger := log.FromContext(ctx).WithName("gitops-status") + gitrepo := &v1alpha1.GitRepo{} + + if err := r.Get(ctx, req.NamespacedName, gitrepo); err != nil && !errors.IsNotFound(err) { + return ctrl.Result{}, err + } else if errors.IsNotFound(err) { + logger.V(1).Info("Gitrepo deleted, cleaning up poll jobs") + return ctrl.Result{}, nil + } + + // Restrictions / Overrides, gitrepo reconciler is responsible for setting error in status + if err := AuthorizeAndAssignDefaults(ctx, r.Client, gitrepo); err != nil { + // the gitjob_controller will handle the error + return ctrl.Result{}, nil + } + + if !gitrepo.DeletionTimestamp.IsZero() { + // the gitjob_controller will handle deletion + return ctrl.Result{}, nil + } + + if gitrepo.Spec.Repo == "" { + return ctrl.Result{}, nil + } + + logger = logger.WithValues("generation", gitrepo.Generation, "commit", gitrepo.Status.Commit).WithValues("conditions", gitrepo.Status.Conditions) + ctx = log.IntoContext(ctx, logger) + + logger.V(1).Info("Reconciling GitRepo status") + + bdList := &v1alpha1.BundleDeploymentList{} + err := r.List(ctx, bdList, client.MatchingLabels{ + v1alpha1.RepoLabel: gitrepo.Name, + v1alpha1.BundleNamespaceLabel: gitrepo.Namespace, + }) + if err != nil { + return ctrl.Result{}, err + } + + err = setStatus(bdList, gitrepo) + if err != nil { + return ctrl.Result{}, err + } + + if gitrepo.Status.GitJobStatus != "Current" { + gitrepo.Status.Display.State = "GitUpdating" + } + + err = r.Client.Status().Update(ctx, gitrepo) + if err != nil { + logger.Error(err, "Reconcile failed update to git repo status", "status", gitrepo.Status) + return ctrl.Result{RequeueAfter: durations.GitRepoStatusDelay}, nil + } + + return ctrl.Result{}, nil +} + +// bundleStatusChangedPredicate returns true if the bundle +// status has changed, or the bundle was created +func bundleStatusChangedPredicate() predicate.Funcs { + return predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return true + }, + UpdateFunc: func(e event.UpdateEvent) bool { + n, isBundle := e.ObjectNew.(*v1alpha1.Bundle) + if !isBundle { + return false + } + o := e.ObjectOld.(*v1alpha1.Bundle) + if n == nil || o == nil { + return false + } + return !reflect.DeepEqual(n.Status, o.Status) + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return true + }, + } +} + +func setStatus(list *v1alpha1.BundleDeploymentList, gitrepo *v1alpha1.GitRepo) error { + // sort for resourceKey? + sort.Slice(list.Items, func(i, j int) bool { + return list.Items[i].UID < list.Items[j].UID + }) + + err := setFields(list, gitrepo) + if err != nil { + return err + } + + resourcestatus.SetGitRepoResources(list, gitrepo) + + summary.SetReadyConditions(&gitrepo.Status, "Bundle", gitrepo.Status.Summary) + + gitrepo.Status.Display.ReadyBundleDeployments = fmt.Sprintf("%d/%d", + gitrepo.Status.Summary.Ready, + gitrepo.Status.Summary.DesiredReady) + + return nil +} + +// setFields sets bundledeployment related status fields: +// Summary, ReadyClusters, DesiredReadyClusters, Display.State, Display.Message, Display.Error +func setFields(list *v1alpha1.BundleDeploymentList, gitrepo *v1alpha1.GitRepo) error { + var ( + maxState v1alpha1.BundleState + message string + count = map[client.ObjectKey]int{} + readyCount = map[client.ObjectKey]int{} + ) + + gitrepo.Status.Summary = v1alpha1.BundleSummary{} + + for _, bd := range list.Items { + state := summary.GetDeploymentState(&bd) + summary.IncrementState(&gitrepo.Status.Summary, bd.Name, state, summary.MessageFromDeployment(&bd), bd.Status.ModifiedStatus, bd.Status.NonReadyStatus) + gitrepo.Status.Summary.DesiredReady++ + if v1alpha1.StateRank[state] > v1alpha1.StateRank[maxState] { + maxState = state + message = summary.MessageFromDeployment(&bd) + } + + // gather status per cluster + // try to avoid old bundle deployments, which might be missing the labels + if bd.Labels == nil { + // this should not happen + continue + } + + name := bd.Labels[v1alpha1.ClusterLabel] + namespace := bd.Labels[v1alpha1.ClusterNamespaceLabel] + if name == "" || namespace == "" { + // this should not happen + continue + } + + key := client.ObjectKey{Name: name, Namespace: namespace} + count[key]++ + if state == v1alpha1.Ready { + readyCount[key]++ + } + } + + // unique number of clusters from bundledeployments + gitrepo.Status.DesiredReadyClusters = len(count) + + // number of clusters where all deployments are ready + readyClusters := 0 + for key, n := range readyCount { + if count[key] == n { + readyClusters++ + } + } + gitrepo.Status.ReadyClusters = readyClusters + + if maxState == v1alpha1.Ready { + maxState = "" + message = "" + } + + gitrepo.Status.Display.State = string(maxState) + gitrepo.Status.Display.Message = message + gitrepo.Status.Display.Error = len(message) > 0 + + return nil +} diff --git a/internal/cmd/controller/reconciler/cluster_controller.go b/internal/cmd/controller/reconciler/cluster_controller.go index 7d7c5ba37f..c384390e10 100644 --- a/internal/cmd/controller/reconciler/cluster_controller.go +++ b/internal/cmd/controller/reconciler/cluster_controller.go @@ -6,14 +6,17 @@ import ( "context" "fmt" "reflect" + "slices" "sort" "time" "github.com/rancher/fleet/internal/cmd/controller/summary" "github.com/rancher/fleet/internal/metrics" + "github.com/rancher/fleet/internal/resourcestatus" fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" "github.com/rancher/fleet/pkg/durations" "github.com/rancher/fleet/pkg/sharding" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fleetutil "github.com/rancher/fleet/internal/cmd/controller/errorutil" "github.com/rancher/wrangler/v3/pkg/condition" @@ -43,11 +46,6 @@ var LongRetry = wait.Backoff{ Jitter: 0.1, } -type repoKey struct { - repo string - ns string -} - // ClusterReconciler reconciles a Cluster object type ClusterReconciler struct { client.Client @@ -105,6 +103,14 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } +func indexByNamespacedName[T metav1.Object](list []T) map[types.NamespacedName]T { + res := make(map[types.NamespacedName]T, len(list)) + for _, obj := range list { + res[types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}] = obj + } + return res +} + //+kubebuilder:rbac:groups=fleet.cattle.io,resources=clusters,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=fleet.cattle.io,resources=clusters/status,verbs=get;update;patch //+kubebuilder:rbac:groups=fleet.cattle.io,resources=clusters/finalizers,verbs=update @@ -144,20 +150,21 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct if err != nil { return ctrl.Result{}, r.updateErrorStatus(ctx, req.NamespacedName, cluster.Status, err) } - - deleted := map[types.UID]bool{} - for _, bundle := range cleanup { - for _, bd := range bundleDeployments.Items { - if bd.Labels[fleet.BundleLabel] == bundle.Name && bd.Labels[fleet.BundleNamespaceLabel] == bundle.Namespace { - logger.V(1).Info("cleaning up bundleDeployment not matching the cluster", "bundledeployment", bd) - err := r.Delete(ctx, &bd) - if err != nil { - logger.V(1).Error(err, "deleting bundleDeployment returned an error") - } - deleted[bd.GetUID()] = true + toDeleteBundles := indexByNamespacedName(cleanup) + + // Delete BundleDeployments for Bundles being removed while getting a filtered items list + bundleDeployments.Items = slices.DeleteFunc(bundleDeployments.Items, func(bd fleet.BundleDeployment) bool { + bundleNamespace := bd.Labels[fleet.BundleNamespaceLabel] + bundleName := bd.Labels[fleet.BundleLabel] + if _, ok := toDeleteBundles[types.NamespacedName{Namespace: bundleNamespace, Name: bundleName}]; ok { + logger.V(1).Info("cleaning up bundleDeployment not matching the cluster", "bundledeployment", bd) + if err := r.Delete(ctx, &bd); err != nil { + logger.V(1).Info("deleting bundleDeployment returned an error", "error", err) } + return true } - } + return false + }) // Count the number of gitrepo, bundledeployemt and deployed resources for this cluster cluster.Status.DesiredReadyGitRepos = 0 @@ -169,23 +176,19 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return bundleDeployments.Items[i].Name < bundleDeployments.Items[j].Name }) - repos := map[repoKey]bool{} - for _, bd := range bundleDeployments.Items { - // do not count bundledeployments that were just deleted - if deleted[bd.GetUID()] { - continue - } + resourcestatus.SetClusterResources(bundleDeployments, cluster) - bd := bd + repos := map[types.NamespacedName]bool{} + for _, bd := range bundleDeployments.Items { state := summary.GetDeploymentState(&bd) summary.IncrementState(&cluster.Status.Summary, bd.Name, state, summary.MessageFromDeployment(&bd), bd.Status.ModifiedStatus, bd.Status.NonReadyStatus) cluster.Status.Summary.DesiredReady++ - repo := bd.Labels[fleet.RepoLabel] - ns := bd.Labels[fleet.BundleNamespaceLabel] - if repo != "" && ns != "" { + repoNamespace, repoName := bd.Labels[fleet.BundleNamespaceLabel], bd.Labels[fleet.RepoLabel] + if repoNamespace != "" && repoName != "" { // a gitrepo is ready if its bundledeployments are ready, take previous state into account - repos[repoKey{repo: repo, ns: ns}] = (state == fleet.Ready) || repos[repoKey{repo: repo, ns: ns}] + repoKey := types.NamespacedName{Namespace: repoNamespace, Name: repoName} + repos[repoKey] = (state == fleet.Ready) || repos[repoKey] } } @@ -193,9 +196,7 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct allReady := true for repo, ready := range repos { gitrepo := &fleet.GitRepo{} - err := r.Get(ctx, types.NamespacedName{Namespace: repo.ns, Name: repo.repo}, gitrepo) - if err == nil { - summary.IncrementResourceCounts(&cluster.Status.ResourceCounts, gitrepo.Status.ResourceCounts) + if err := r.Get(ctx, repo, gitrepo); err == nil { cluster.Status.DesiredReadyGitRepos++ if ready { cluster.Status.ReadyGitRepos++ diff --git a/internal/cmd/controller/gitops/reconciler/resourcekey.go b/internal/resourcestatus/resourcekey.go similarity index 95% rename from internal/cmd/controller/gitops/reconciler/resourcekey.go rename to internal/resourcestatus/resourcekey.go index 16cfc52f74..cefae3a48d 100644 --- a/internal/cmd/controller/gitops/reconciler/resourcekey.go +++ b/internal/resourcestatus/resourcekey.go @@ -1,4 +1,4 @@ -package reconciler +package resourcestatus import ( "encoding/json" @@ -8,7 +8,7 @@ import ( fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" ) -func setResources(list *fleet.BundleDeploymentList, gitrepo *fleet.GitRepo) { +func SetGitRepoResources(list *fleet.BundleDeploymentList, gitrepo *fleet.GitRepo) { s := summaryState(gitrepo.Status.Summary) r, errors := fromResources(list, s) gitrepo.Status.ResourceErrors = errors @@ -16,6 +16,12 @@ func setResources(list *fleet.BundleDeploymentList, gitrepo *fleet.GitRepo) { gitrepo.Status.Resources = merge(r) } +func SetClusterResources(list *fleet.BundleDeploymentList, cluster *fleet.Cluster) { + s := summaryState(cluster.Status.Summary) + r, _ := fromResources(list, s) + cluster.Status.ResourceCounts = countResources(r) +} + // merge takes a list of GitRepo resources and deduplicates resources deployed to multiple clusters, // ensuring that for such resources, the output contains a single resource entry with a field summarizing // its status on each cluster. diff --git a/internal/cmd/controller/gitops/reconciler/resourcekey_test.go b/internal/resourcestatus/resourcekey_test.go similarity index 98% rename from internal/cmd/controller/gitops/reconciler/resourcekey_test.go rename to internal/resourcestatus/resourcekey_test.go index 295cd68cec..3c9298f5a3 100644 --- a/internal/cmd/controller/gitops/reconciler/resourcekey_test.go +++ b/internal/resourcestatus/resourcekey_test.go @@ -1,4 +1,4 @@ -package reconciler +package resourcestatus import ( . "github.com/onsi/ginkgo/v2" @@ -114,7 +114,7 @@ var _ = Describe("Resourcekey", func() { }) It("returns a list", func() { - setResources(list, gitrepo) + SetGitRepoResources(list, gitrepo) Expect(gitrepo.Status.Resources).To(HaveLen(2)) Expect(gitrepo.Status.Resources).To(ContainElement(fleet.GitRepoResource{