Skip to content

Commit

Permalink
Show bundle errors in Bundle and GitRepo
Browse files Browse the repository at this point in the history 
Refers to rancher#2943
  • Loading branch information
@p-se
p-se committed Dec 18, 2024
1 parent db97ffd commit 2af210c
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 52 deletions.
82 changes: 82 additions & 0 deletions internal/cmd/controller/gitops/reconciler/status_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
101 changes: 49 additions & 52 deletions internal/cmd/controller/reconciler/bundle_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 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
}

Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -366,24 +352,21 @@ 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 nil, err
}
}

return r.Update(ctx, content)
})
if err != nil {
logger.Error(err, "Reconcile failed to add content finalizer", "content ID", manifestID)
}

contentsInHelmChart := helmAppOptions != nil
Expand Down Expand Up @@ -503,3 +486,17 @@ 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 for the 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
}

0 comments on commit 2af210c

Please sign in to comment.