diff --git a/internal/cmd/controller/gitops/reconciler/status_controller.go b/internal/cmd/controller/gitops/reconciler/status_controller.go index 8b5a790175..e99972a653 100644 --- a/internal/cmd/controller/gitops/reconciler/status_controller.go +++ b/internal/cmd/controller/gitops/reconciler/status_controller.go @@ -11,7 +11,9 @@ import ( fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" "github.com/rancher/fleet/pkg/durations" "github.com/rancher/fleet/pkg/sharding" + "github.com/rancher/wrangler/v3/pkg/genericcondition" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -109,6 +111,29 @@ func (r *StatusReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr gitrepo.Status.Display.State = "GitUpdating" } + // We're explicitly setting the ready status from a bundle here, but only if it isn't ready. + // + // - If the bundle has no deployments, there is no status to be copied from the setStatus + // function, so that we won't overwrite anything. + // + // - If the bundle has rendering issues and there are deployments of which there is at least one + // in a failed state, the status of the bundle deployments would be overwritten by the bundle + // status. + // + // - If the bundle has no rendering issues but there are deployments in a failed state, the code + // will overwrite the gitrepo's ready status condition with the ready status condition coming + // from the bundle. Because both have the same content, we can unconditionally set the status + // from the bundle. + // + // So we're basically just making sure the status from the bundle is being set on the gitrepo, + // even if there are no bundle deployments, which is the case for issues with rendering the + // manifests, for instance. In that case no bundle deployments are created, but an error is set + // in a ready status condition on the bundle. + err = r.setReadyStatusFromBundle(ctx, gitrepo) + if err != nil { + return ctrl.Result{}, err + } + err = r.Client.Status().Update(ctx, gitrepo) if err != nil { logger.Error(err, "Reconcile failed update to git repo status", "status", gitrepo.Status) @@ -139,3 +164,60 @@ func setStatus(list *fleet.BundleDeploymentList, gitrepo *fleet.GitRepo) error { return nil } + +// setReadyStatusFromBundle fetches all bundles from a given gitrepo, checks the ready status conditions +// from the bundles and applies one on the gitrepo if it isn't ready. The purpose is to make +// rendering issues visible in the gitrepo status. Those issues need to be made explicitly visible +// since the other statuses are calculated from bundle deployments, which do not exist when +// rendering manifests fail. Should an issue be on the bundle, it will be copied to the gitrepo. +func (r StatusReconciler) setReadyStatusFromBundle(ctx context.Context, gitrepo *fleet.GitRepo) error { + bList := &fleet.BundleList{} + err := r.List(ctx, bList, client.MatchingLabels{ + fleet.RepoLabel: gitrepo.Name, + }, client.InNamespace(gitrepo.Namespace)) + if err != nil { + return err + } + + found := false + // Find a ready status condition in a bundle which is not ready. + var condition genericcondition.GenericCondition +bundles: + for _, bundle := range bList.Items { + if bundle.Status.Conditions == nil { + continue + } + + for _, c := range bundle.Status.Conditions { + if c.Type == string(fleet.Ready) && c.Status == v1.ConditionFalse { + condition = c + found = true + break bundles + } + } + } + + // No ready condition found in any bundle, nothing to do here. + if !found { + return nil + } + + found = false + newConditions := make([]genericcondition.GenericCondition, 0, len(gitrepo.Status.Conditions)) + for _, c := range gitrepo.Status.Conditions { + if c.Type == string(fleet.Ready) { + // Replace the ready condition with the one from the bundle + newConditions = append(newConditions, condition) + found = true + continue + } + newConditions = append(newConditions, c) + } + if !found { + // Add the ready condition from the bundle to the gitrepo. + newConditions = append(newConditions, condition) + } + gitrepo.Status.Conditions = newConditions + + return nil +} diff --git a/internal/cmd/controller/reconciler/bundle_controller.go b/internal/cmd/controller/reconciler/bundle_controller.go index 3d0b6dad94..f107211842 100644 --- a/internal/cmd/controller/reconciler/bundle_controller.go +++ b/internal/cmd/controller/reconciler/bundle_controller.go @@ -7,8 +7,10 @@ import ( "fmt" "os" "strconv" + "time" "github.com/go-logr/logr" + "github.com/rancher/fleet/internal/cmd/controller/finalize" "github.com/rancher/fleet/internal/cmd/controller/summary" "github.com/rancher/fleet/internal/cmd/controller/target" @@ -17,13 +19,14 @@ import ( "github.com/rancher/fleet/internal/ociwrapper" fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" "github.com/rancher/fleet/pkg/sharding" + "github.com/rancher/wrangler/v3/pkg/genericcondition" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -132,6 +135,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr if err := r.Get(ctx, req.NamespacedName, bundle); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } + bundleOrig := bundle.DeepCopy() if bundle.Labels[fleet.RepoLabel] != "" { logger = logger.WithValues( @@ -184,6 +188,18 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr matchedTargets, err := r.Builder.Targets(ctx, bundle, manifestID) if err != nil { + // When targeting fails, we don't want to continue and we make the error message visible in + // the UI. For that we use a status condition of type Ready. + bundle.Status.Conditions = []genericcondition.GenericCondition{ + { + Type: string(fleet.Ready), + Status: v1.ConditionFalse, + Message: "Targeting error: " + err.Error(), + LastUpdateTime: metav1.Now().UTC().Format(time.RFC3339), + }, + } + + err := r.updateStatus(ctx, bundleOrig, bundle) return ctrl.Result{}, err } @@ -226,9 +242,8 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr summary.SetReadyConditions(&bundle.Status, "Cluster", bundle.Status.Summary) bundle.Status.ObservedGeneration = bundle.Generation - // build BundleDeployments out of targets discarding Status, replacing - // DependsOn with the bundle's DependsOn (pure function) and replacing - // the labels with the bundle's labels + // build BundleDeployments out of targets discarding Status, replacing DependsOn with the + // bundle's DependsOn (pure function) and replacing the labels with the bundle's labels for _, target := range matchedTargets { bd, err := r.createBundleDeployment( ctx, @@ -247,20 +262,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } updateDisplay(&bundle.Status) - - err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - t := &fleet.Bundle{} - if err := r.Get(ctx, req.NamespacedName, t); err != nil { - return err - } - t.Status = bundle.Status - return r.Status().Update(ctx, t) - }) - if err != nil { - logger.V(1).Info("Reconcile failed final update to bundle status", "status", bundle.Status, "error", err) - } else { - metrics.BundleCollector.Collect(ctx, bundle) - } + err = r.updateStatus(ctx, bundleOrig, bundle) return ctrl.Result{}, err } @@ -297,16 +299,8 @@ func (r *BundleReconciler) addOrRemoveFinalizer(ctx context.Context, logger logr return true, client.IgnoreNotFound(err) } - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - if err := r.Get(ctx, req.NamespacedName, bundle); err != nil { - return err - } - - controllerutil.RemoveFinalizer(bundle, bundleFinalizer) - - return r.Update(ctx, bundle) - }) - + controllerutil.RemoveFinalizer(bundle, bundleFinalizer) + err := r.Update(ctx, bundle) if client.IgnoreNotFound(err) != nil { return true, err } @@ -316,16 +310,8 @@ func (r *BundleReconciler) addOrRemoveFinalizer(ctx context.Context, logger logr } if !controllerutil.ContainsFinalizer(bundle, bundleFinalizer) { - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - if err := r.Get(ctx, req.NamespacedName, bundle); err != nil { - return err - } - - controllerutil.AddFinalizer(bundle, bundleFinalizer) - - return r.Update(ctx, bundle) - }) - + controllerutil.AddFinalizer(bundle, bundleFinalizer) + err := r.Update(ctx, bundle) if client.IgnoreNotFound(err) != nil { return true, err } @@ -366,24 +352,20 @@ func (r *BundleReconciler) createBundleDeployment( bd.Spec.OCIContents = contentsInOCI bd.Spec.HelmChartOptions = helmAppOptions - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - if contentsInOCI { - return nil // no contents resources stored in etcd, no finalizers to add here. - } - + // contents resources stored in etcd, finalizers to add here. + if !contentsInOCI { content := &fleet.Content{} - if err := r.Get(ctx, types.NamespacedName{Name: manifestID}, content); err != nil { - return client.IgnoreNotFound(err) + err := r.Get(ctx, types.NamespacedName{Name: manifestID}, content) + if client.IgnoreNotFound(err) != nil { + logger.Error(err, "Reconcile failed to get content", "content ID", manifestID) + return nil, err } - if added := controllerutil.AddFinalizer(content, bd.Name); !added { - return nil + if added := controllerutil.AddFinalizer(content, bd.Name); added { + if err := r.Update(ctx, content); err != nil { + logger.Error(err, "Reconcile failed to add content finalizer", "content ID", manifestID) + } } - - return r.Update(ctx, content) - }) - if err != nil { - logger.Error(err, "Reconcile failed to add content finalizer", "content ID", manifestID) } contentsInHelmChart := helmAppOptions != nil @@ -503,3 +485,18 @@ func experimentalHelmOpsEnabled() bool { value, err := strconv.ParseBool(os.Getenv("EXPERIMENTAL_HELM_OPS")) return err == nil && value } + +// updateStatus patches the status of the bundle and collects metrics upon a successful update of +// the bundle status. It returns nil if the status update is successful, otherwise it returns an +// error. +func (r *BundleReconciler) updateStatus(ctx context.Context, orig *fleet.Bundle, bundle *fleet.Bundle) error { + logger := log.FromContext(ctx).WithName("bundle - updateStatus") + statusPatch := client.MergeFrom(orig) + err := r.Status().Patch(ctx, bundle, statusPatch) + if err != nil { + logger.V(1).Info("Reconcile failed update to bundle status", "status", bundle.Status, "error", err) + return err + } + metrics.BundleCollector.Collect(ctx, bundle) + return nil +}