Retry reconcile when we cannot add finalizer to content resource (#3310)

internal/cmd/agent/controller/bundledeployment_controller.go

@@ -134,6 +134,7 @@ func (r *BundleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 		merr = append(merr, fmt.Errorf("failed deploying bundle: %w", err))
 	} else {
+		logger.V(1).Info("Bundle deployed", "status", status)
 		bd.Status = setCondition(status, nil, monitor.Cond(fleetv1.BundleDeploymentConditionDeployed))
 	}
 

internal/cmd/agent/deployer/monitor/updatestatus.go

@@ -67,6 +67,8 @@ func isAgent(bd *fleet.BundleDeployment) bool {
 	return strings.HasPrefix(bd.Name, "fleet-agent")
 }
 
+// ShouldUpdateStatus skips resource and ready status updates if the bundle
+// deployment is unchanged or not installed yet.
 func ShouldUpdateStatus(bd *fleet.BundleDeployment) bool {
 	if bd.Spec.DeploymentID != bd.Status.AppliedDeploymentID {
 		return false
@@ -81,6 +83,9 @@ func ShouldUpdateStatus(bd *fleet.BundleDeployment) bool {
 	return true
 }
 
+// UpdateStatus sets the status of the bundledeployment based on the resources from the helm release history and the live state.
+// In the status it updates: Ready, NonReadyStatus, IncompleteState, NonReadyStatus, NonModified, ModifiedStatus, Resources and ResourceCounts fields.
+// Additionally it sets the Ready condition either from the NonReadyStatus or the NonModified status field.
 func (m *Monitor) UpdateStatus(ctx context.Context, bd *fleet.BundleDeployment, resources *helmdeployer.Resources) (fleet.BundleDeploymentStatus, error) {
 	logger := log.FromContext(ctx).WithName("update-status")
 	ctx = log.IntoContext(ctx, logger)
@@ -101,14 +106,16 @@ func (m *Monitor) UpdateStatus(ctx context.Context, bd *fleet.BundleDeployment,
 
 		return origStatus, err
 	}
+
 	status := bd.Status
+	status.SyncGeneration = &bd.Spec.Options.ForceSyncGeneration
 
 	readyError := readyError(status)
 	Cond(fleet.BundleDeploymentConditionReady).SetError(&status, "", readyError)
-
-	status.SyncGeneration = &bd.Spec.Options.ForceSyncGeneration
 	if readyError != nil {
-		logger.Info("Status not ready", "error", readyError)
+		logger.Info("Status not ready according to nonModified and nonReady", "nonModified", status.NonModified, "nonReady", status.NonReadyStatus)
+	} else {
+		logger.V(1).Info("Status ready, Ready condition set to true")
 	}
 
 	removePrivateFields(&status)
@@ -149,6 +156,7 @@ func readyError(status fleet.BundleDeploymentStatus) error {
 
 // updateFromPreviousDeployment updates the status with information from the
 // helm release history and an apply dry run.
+// Modified resources are resources that have changed from the previous helm release.
 func (m *Monitor) updateFromPreviousDeployment(ctx context.Context, bd *fleet.BundleDeployment, resources *helmdeployer.Resources) error {
 	resourcesPreviousRelease, err := m.deployer.ResourcesFromPreviousReleaseVersion(bd.Name, bd.Status.Release)
 	if err != nil {

internal/cmd/controller/reconciler/bundle_controller.go

@@ -218,6 +218,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			return ctrl.Result{}, err
 		}
 	}
+	logger = logger.WithValues("manifestID", manifestID)
 
 	if err := resetStatus(&bundle.Status, matchedTargets); err != nil {
 		return ctrl.Result{}, err
@@ -332,10 +333,8 @@ func (r *BundleReconciler) createBundleDeployment(
 	}
 	if target.Deployment.Namespace == "" {
 		logger.V(1).Info(
-			"Skipping bundledeployment with empty namespace, "+
-				"waiting for agentmanagement to set cluster.status.namespace",
-			"bundledeployment",
-			target.Deployment,
+			"Skipping bundledeployment with empty namespace, waiting for agentmanagement to set cluster.status.namespace",
+			"bundledeployment", target.Deployment,
 		)
 		return nil, nil
 	}
@@ -356,15 +355,13 @@ func (r *BundleReconciler) createBundleDeployment(
 	// When content resources are stored in etcd, we need to add finalizers.
 	if !contentsInOCI && !contentsInHelmChart {
 		content := &fleet.Content{}
-		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 err := r.Get(ctx, types.NamespacedName{Name: manifestID}, content); err != nil {
+			return nil, fmt.Errorf("failed to get content resource: %w", err)
 		}
 
 		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, fmt.Errorf("could not add finalizer to content resource: %w", err)
 			}
 		}
 	}

pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go

@@ -312,9 +312,8 @@ const (
 	// BundleDeploymentConditionInstalled indicates the bundledeployment
 	// has been installed.
 	BundleDeploymentConditionInstalled = "Installed"
-	// BundleDeploymentConditionDeployed is used by the bundledeployment
-	// controller. It is true if the handler returns no error and false if
-	// an error is returned.
+	// BundleDeploymentConditionDeployed indicates whether the deployment
+	// succeeded.
 	BundleDeploymentConditionDeployed  = "Deployed"
 	BundleDeploymentConditionMonitored = "Monitored"
 )