Feat: Guarantee one-time actions emit as distinct events

Ensures create/update/delete/approve operations trigger an immediate, distinct condition/event emission to guarantee visibility.
Signed-off-by: yiraeChristineKim <[email protected]>
working well exept metrics things

Author: yiraeChristineKim

controllers/operatorpolicy_controller.go

@@ -421,7 +421,8 @@ func (r *OperatorPolicyReconciler) handleResources(ctx context.Context, policy *
 		return earlyComplianceEvents, condChanged, err
 	}
 
-	changed, err = r.handleInstallPlan(ctx, policy, subscription)
+	changed, earlyConds, err = r.handleInstallPlan(ctx, policy, subscription)
+	earlyComplianceEvents = append(earlyComplianceEvents, earlyConds...)
 	condChanged = condChanged || changed
 
 	if err != nil {
@@ -1118,7 +1119,9 @@ func (r *OperatorPolicyReconciler) musthaveOpGroup(
 		// Now the OperatorGroup should match, so report Compliance
 		updateStatus(policy, createdCond("OperatorGroup"), createdObj(desiredOpGroup))
 		// Emit a dedicated "created" event regardless of later status aggregation
-		earlyConds = append(earlyConds, createdCond("OperatorGroup"))
+		cond := createdCond("OperatorGroup")
+		cond.LastTransitionTime = metav1.Now()
+		earlyConds = append(earlyConds, cond)
 
 		return true, earlyConds, true, nil
 	case 1:
@@ -1203,6 +1206,10 @@ func (r *OperatorPolicyReconciler) musthaveOpGroup(
 		desiredOpGroup.SetGroupVersionKind(operatorGroupGVK) // Update stripped this information
 
 		updateStatus(policy, updatedCond("OperatorGroup"), updatedObj(desiredOpGroup))
+		// Emit a dedicated "updated" event
+		upd := updatedCond("OperatorGroup")
+		upd.LastTransitionTime = metav1.Now()
+		earlyConds = append(earlyConds, upd)
 
 		return true, earlyConds, true, nil
 	default:
@@ -1363,6 +1370,10 @@ func (r *OperatorPolicyReconciler) mustnothaveOpGroup(
 	desiredOpGroup.SetGroupVersionKind(operatorGroupGVK) // Delete stripped this information
 
 	updateStatus(policy, deletedCond("OperatorGroup"), deletedObj(desiredOpGroup))
+	// Emit a dedicated "deleted" event
+	del := deletedCond("OperatorGroup")
+	del.LastTransitionTime = metav1.Now()
+	earlyConds = append(earlyConds, del)
 
 	return earlyConds, true, nil
 }
@@ -1416,7 +1427,6 @@ func (r *OperatorPolicyReconciler) musthaveSubscription(
 			earlyConds = append(earlyConds, calculateComplianceCondition(policy))
 		}
 
-		log.Info("!!!!!!!!!!!!!! create subscription!!!!!!!!!!!")
 		err := r.createWithNamespace(ctx, desiredSub)
 		if err != nil {
 			return nil, nil, changed, fmt.Errorf("error creating the Subscription: %w", err)
@@ -1533,6 +1543,10 @@ func (r *OperatorPolicyReconciler) musthaveSubscription(
 	mergedSub.SetGroupVersionKind(subscriptionGVK) // Update stripped this information
 
 	updateStatus(policy, updatedCond("Subscription"), updatedObj(mergedSub))
+	// Emit a dedicated "updated" event
+	upd := updatedCond("Subscription")
+	upd.LastTransitionTime = metav1.Now()
+	earlyConds = append(earlyConds, upd)
 
 	return mergedSub, earlyConds, true, nil
 }
@@ -1856,6 +1870,10 @@ func (r *OperatorPolicyReconciler) mustnothaveSubscription(
 	}
 
 	updateStatus(policy, deletedCond("Subscription"), deletedObj(desiredSub))
+	// Emit a dedicated "deleted" event
+	del := deletedCond("Subscription")
+	del.LastTransitionTime = metav1.Now()
+	earlyConds = append(earlyConds, del)
 
 	return foundSub, earlyConds, true, nil
 }
@@ -1887,14 +1905,14 @@ func messageIncludesSubscription(subscription *operatorv1alpha1.Subscription, me
 
 func (r *OperatorPolicyReconciler) handleInstallPlan(
 	ctx context.Context, policy *policyv1beta1.OperatorPolicy, sub *operatorv1alpha1.Subscription,
-) (bool, error) {
+) (bool, []metav1.Condition, error) {
 	if sub == nil {
 		// Note: existing related objects will not be removed by this status update
-		return updateStatus(policy, invalidCausingUnknownCond("InstallPlan")), nil
+		return updateStatus(policy, invalidCausingUnknownCond("InstallPlan")), []metav1.Condition{}, nil
 	}
 
 	if sub.Status.CurrentCSV == "" && sub.Status.InstalledCSV == "" {
-		return updateStatus(policy, noInstallPlansCond, noInstallPlansObj(sub.Namespace)), nil
+		return updateStatus(policy, noInstallPlansCond, noInstallPlansObj(sub.Namespace)), []metav1.Condition{}, nil
 	}
 
 	watcher := opPolIdentifier(policy.Namespace, policy.Name)
@@ -1907,14 +1925,14 @@ func (r *OperatorPolicyReconciler) handleInstallPlan(
 	// potentially vulnerable operators.
 	installPlans, err := r.DynamicWatcher.List(watcher, installPlanGVK, sub.Namespace, labels.Everything())
 	if err != nil {
-		return false, fmt.Errorf("error listing InstallPlans: %w", err)
+		return false, []metav1.Condition{}, fmt.Errorf("error listing InstallPlans: %w", err)
 	}
 
 	// InstallPlans are generally kept in order to provide a history of actions on the cluster, but
 	// they can be deleted without impacting the installed operator. So, not finding any should not
 	// be considered a reason for NonCompliance, regardless of musthave or mustnothave.
 	if len(installPlans) == 0 {
-		return updateStatus(policy, noInstallPlansCond, noInstallPlansObj(sub.Namespace)), nil
+		return updateStatus(policy, noInstallPlansCond, noInstallPlansObj(sub.Namespace)), []metav1.Condition{}, nil
 	}
 
 	var latestInstallPlan *unstructured.Unstructured
@@ -1982,24 +2000,26 @@ func (r *OperatorPolicyReconciler) handleInstallPlan(
 			"subscription", sub.Name, "namespace", sub.Namespace,
 		)
 
-		return updateStatus(policy, noInstallPlansCond, noInstallPlansObj(sub.Namespace)), nil
+		return updateStatus(policy, noInstallPlansCond, noInstallPlansObj(sub.Namespace)), []metav1.Condition{}, nil
 	}
 
 	if policy.Spec.ComplianceType.IsMustHave() {
-		changed, err := r.musthaveInstallPlan(ctx, policy, sub, latestInstallPlan)
+		changed, earlyConds, err := r.musthaveInstallPlan(ctx, policy, sub, latestInstallPlan)
 
-		return changed, err
+		return changed, earlyConds, err
 	}
 
-	return r.mustnothaveInstallPlan(policy, latestInstallPlan)
+	changed, err := r.mustnothaveInstallPlan(policy, latestInstallPlan)
+
+	return changed, []metav1.Condition{}, err
 }
 
 func (r *OperatorPolicyReconciler) musthaveInstallPlan(
 	ctx context.Context,
 	policy *policyv1beta1.OperatorPolicy,
 	sub *operatorv1alpha1.Subscription,
 	latestInstallPlanUnstruct *unstructured.Unstructured,
-) (bool, error) {
+) (bool, []metav1.Condition, error) {
 	opLog := ctrl.LoggerFrom(ctx)
 	complianceConfig := policy.Spec.ComplianceConfig.UpgradesAvailable
 	latestInstallPlan := operatorv1alpha1.InstallPlan{}
@@ -2009,7 +2029,7 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan(
 		opLog.Error(err, "Unable to determine the CSV names of the current InstallPlan",
 			"InstallPlan.Name", latestInstallPlanUnstruct.GetName())
 
-		return updateStatus(policy, installPlansNoApprovals), nil
+		return updateStatus(policy, installPlansNoApprovals), []metav1.Condition{}, nil
 	}
 
 	ipCSVs := latestInstallPlan.Spec.ClusterServiceVersionNames
@@ -2024,21 +2044,21 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan(
 		currentCSV := sub.Status.CurrentCSV
 
 		if installedCSV != "" && currentCSV != "" && installedCSV == currentCSV {
-			return updateStatus(policy, installPlansNoApprovals), nil
+			return updateStatus(policy, installPlansNoApprovals), []metav1.Condition{}, nil
 		}
 	case operatorv1alpha1.InstallPlanPhaseInstalling:
 		return updateStatus(
 			policy,
 			installPlanInstallingCond,
 			existingInstallPlanObj(&latestInstallPlan, string(phase), complianceConfig),
-		), nil
+		), []metav1.Condition{}, nil
 	case operatorv1alpha1.InstallPlanFailed:
 		// Generally, a failed InstallPlan is not a reason for NonCompliance, because it could be from
 		// an old installation. But if the current InstallPlan is failed, we should alert the user.
 		if len(ipCSVs) == 1 {
 			return updateStatus(
 				policy, installPlanFailed, existingInstallPlanObj(&latestInstallPlan, string(phase), complianceConfig),
-			), nil
+			), []metav1.Condition{}, nil
 		}
 
 		// If there is more than one CSV in the InstallPlan, make sure this CSV failed before marking it as
@@ -2056,27 +2076,30 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan(
 					policy,
 					installPlanFailed,
 					existingInstallPlanObj(&latestInstallPlan, string(phase), complianceConfig),
-				), nil
+				), []metav1.Condition{}, nil
 			}
 		}
 
 		// If no step for this CSV failed in the InstallPlan, treat it the same as no install plans requiring approval
-		return updateStatus(policy, installPlansNoApprovals), nil
+		return updateStatus(policy, installPlansNoApprovals), []metav1.Condition{}, nil
 	default:
 		return updateStatus(
 			policy,
 			installPlansNoApprovals, existingInstallPlanObj(&latestInstallPlan, string(phase), complianceConfig),
-		), nil
+		), []metav1.Condition{}, nil
 	}
 
 	// Check if it's already approved. This can happen if this OperatorPolicy or another managing the same InstallPlan
 	// performed the approval but it hasn't started installing yet.
 	if latestInstallPlan.Spec.Approved {
+		ipApproved := installPlanApprovedCond(sub.Status.CurrentCSV)
+		ipApproved.LastTransitionTime = metav1.Now()
+
 		return updateStatus(
 			policy,
 			installPlanApprovedCond(sub.Status.CurrentCSV),
 			existingInstallPlanObj(&latestInstallPlan, string(phase), complianceConfig),
-		), nil
+		), []metav1.Condition{ipApproved}, nil
 	}
 
 	initialInstall := sub.Status.InstalledCSV == ""
@@ -2085,43 +2108,54 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan(
 	// Only report this status when not approving an InstallPlan, because otherwise it could easily
 	// oscillate between this and another condition.
 	if policy.Spec.RemediationAction.IsInform() || (!initialInstall && !autoUpgrade) {
+		// Emit an early event to record that an upgrade is available and requires approval
+		ipUpgrade := installPlanUpgradeCond(complianceConfig, ipCSVs, nil)
+		ipUpgrade.LastTransitionTime = metav1.Now()
+
 		return updateStatus(
 			policy,
-			installPlanUpgradeCond(complianceConfig, ipCSVs, nil),
+			ipUpgrade,
 			existingInstallPlanObj(&latestInstallPlan, string(phase), complianceConfig),
-		), nil
+		), []metav1.Condition{ipUpgrade}, nil
 	}
 
 	remainingCSVsToApprove, err := r.getRemainingCSVApprovals(ctx, policy, sub, &latestInstallPlan)
 	if err != nil {
-		return false, fmt.Errorf("error finding the InstallPlan approvals: %w", err)
+		return false, []metav1.Condition{}, fmt.Errorf("error finding the InstallPlan approvals: %w", err)
 	}
 
 	if len(remainingCSVsToApprove) != 0 {
+		// Emit an early event that approval is required for the listed CSVs
+		ipUpgrade := installPlanUpgradeCond(complianceConfig, ipCSVs, remainingCSVsToApprove)
+		ipUpgrade.LastTransitionTime = metav1.Now()
+
 		return updateStatus(
 			policy,
-			installPlanUpgradeCond(complianceConfig, ipCSVs, remainingCSVsToApprove),
+			ipUpgrade,
 			existingInstallPlanObj(&latestInstallPlan, string(phase), complianceConfig),
-		), nil
+		), []metav1.Condition{ipUpgrade}, nil
 	}
 
 	opLog.Info("Approving InstallPlan", "InstallPlanName", latestInstallPlan.Name,
 		"InstallPlanNamespace", latestInstallPlan.Namespace)
 
 	if err := unstructured.SetNestedField(latestInstallPlanUnstruct.Object, true, "spec", "approved"); err != nil {
-		return false, fmt.Errorf("error approving InstallPlan: %w", err)
+		return false, []metav1.Condition{}, fmt.Errorf("error approving InstallPlan: %w", err)
 	}
 
 	if err := r.TargetClient.Update(ctx, latestInstallPlanUnstruct); err != nil {
-		return false, fmt.Errorf("error updating approved InstallPlan: %w", err)
+		return false, []metav1.Condition{}, fmt.Errorf("error updating approved InstallPlan: %w", err)
 	}
 
+	ipApproved := installPlanApprovedCond(sub.Status.CurrentCSV)
+	ipApproved.LastTransitionTime = metav1.Now()
+
 	return updateStatus(
 			policy,
 			installPlanApprovedCond(sub.Status.CurrentCSV),
 			existingInstallPlanObj(&latestInstallPlan, string(phase), complianceConfig),
 		),
-		nil
+		[]metav1.Condition{ipApproved}, nil
 }
 
 // getRemainingCSVApprovals will return all CSVs that can't be approved by an OperatorPolicy (currentPolicy and others).
@@ -2577,6 +2611,7 @@ func (r *OperatorPolicyReconciler) mustnothaveCSV(
 	}
 
 	updateStatus(policy, deletedCond("ClusterServiceVersion", csvNames...), relatedCSVs...)
+	earlyConds = append(earlyConds, deletedCond("ClusterServiceVersion", csvNames...))
 
 	return earlyConds, true, nil
 }
@@ -2755,6 +2790,7 @@ func (r *OperatorPolicyReconciler) handleCRDs(
 	}
 
 	updateStatus(policy, deletedCond("CustomResourceDefinition"), relatedCRDs...)
+	earlyConds = append(earlyConds, deletedCond("CustomResourceDefinition"))
 
 	return earlyConds, true, nil
 }