Skip to content

Commit

Permalink
Return a subscription from handleSubscription
Browse files Browse the repository at this point in the history
Some of the additional checks will need pieces of information from the
Subscription. Converting it before returning it (as opposed to using an
Unstructured) should help reduce other error checking.

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
  • Loading branch information
JustinKuli committed Feb 2, 2024
1 parent 3da5031 commit ef38ec5
Showing 1 changed file with 28 additions and 20 deletions.
48 changes: 28 additions & 20 deletions controllers/operatorpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
return reconcile.Result{}, err
}

if err := r.handleSubscription(ctx, policy); err != nil {
_, err = r.handleSubscription(ctx, policy)
if err != nil {
OpLog.Error(err, "Error handling Subscription")

return reconcile.Result{}, err
Expand Down Expand Up @@ -358,98 +359,105 @@ func buildOperatorGroup(
return operatorGroup, nil
}

func (r *OperatorPolicyReconciler) handleSubscription(ctx context.Context, policy *policyv1beta1.OperatorPolicy) error {
func (r *OperatorPolicyReconciler) handleSubscription(
ctx context.Context, policy *policyv1beta1.OperatorPolicy,
) (*operatorv1alpha1.Subscription, error) {
watcher := opPolIdentifier(policy.Namespace, policy.Name)

desiredSub, err := buildSubscription(policy)
if err != nil {
return fmt.Errorf("error building subscription: %w", err)
return desiredSub, fmt.Errorf("error building subscription: %w", err)
}

foundSub, err := r.DynamicWatcher.Get(watcher, subscriptionGVK, desiredSub.Namespace, desiredSub.Name)
if err != nil {
return fmt.Errorf("error getting the Subscription: %w", err)
return desiredSub, fmt.Errorf("error getting the Subscription: %w", err)
}

if foundSub == nil {
// Missing Subscription: report NonCompliance
err := r.updateStatus(ctx, policy, missingWantedCond("Subscription"), missingWantedObj(desiredSub))
if err != nil {
return fmt.Errorf("error updating status for a missing Subscription: %w", err)
return desiredSub, fmt.Errorf("error updating status for a missing Subscription: %w", err)
}

if policy.Spec.RemediationAction.IsEnforce() {
err := r.Create(ctx, desiredSub)
if err != nil {
return fmt.Errorf("error creating the Subscription: %w", err)
return desiredSub, fmt.Errorf("error creating the Subscription: %w", err)
}

desiredSub.SetGroupVersionKind(subscriptionGVK) // Create stripped this information

// Now it should match, so report Compliance
err = r.updateStatus(ctx, policy, createdCond("Subscription"), createdObj(desiredSub))
if err != nil {
return fmt.Errorf("error updating the status for a created Subscription: %w", err)
return desiredSub, fmt.Errorf("error updating the status for a created Subscription: %w", err)
}
}

return nil
return desiredSub, nil
}

// Subscription found; check if specs match
desiredUnstruct, err := runtime.DefaultUnstructuredConverter.ToUnstructured(desiredSub)
if err != nil {
return fmt.Errorf("error converting desired Subscription to an Unstructured: %w", err)
return desiredSub, fmt.Errorf("error converting desired Subscription to an Unstructured: %w", err)
}

merged := foundSub.DeepCopy() // Copy it so that the value in the cache is not changed

updateNeeded, skipUpdate, err := r.mergeObjects(ctx, desiredUnstruct, merged, string(policy.Spec.ComplianceType))
if err != nil {
return fmt.Errorf("error checking if the Subscription needs an update: %w", err)
return desiredSub, fmt.Errorf("error checking if the Subscription needs an update: %w", err)
}

mergedSub := new(operatorv1alpha1.Subscription)
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(merged.Object, mergedSub); err != nil {
return desiredSub, fmt.Errorf("error converting the retrieved Subscription to the go type: %w", err)
}

if !updateNeeded {
// FUTURE: Check more details about the *status* of the Subscription
// For now, just mark it as compliant
err := r.updateStatus(ctx, policy, matchesCond("Subscription"), matchedObj(foundSub))
if err != nil {
return fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err)
return mergedSub, fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err)
}

return nil
return mergedSub, nil
}

// Specs don't match.
if policy.Spec.RemediationAction.IsEnforce() && skipUpdate {
err = r.updateStatus(ctx, policy, mismatchCondUnfixable("Subscription"), mismatchedObj(foundSub))
if err != nil {
return fmt.Errorf("error updating status for a mismatched Subscription that can't be enforced: %w", err)
return mergedSub, fmt.Errorf("error updating status for a mismatched Subscription that can't be enforced: %w", err)
}

return nil
return mergedSub, nil
}

err = r.updateStatus(ctx, policy, mismatchCond("Subscription"), mismatchedObj(foundSub))
if err != nil {
return fmt.Errorf("error updating status for a mismatched Subscription: %w", err)
return mergedSub, fmt.Errorf("error updating status for a mismatched Subscription: %w", err)
}

if policy.Spec.RemediationAction.IsEnforce() {
err := r.Update(ctx, merged)
if err != nil {
return fmt.Errorf("error updating the Subscription: %w", err)
return mergedSub, fmt.Errorf("error updating the Subscription: %w", err)
}

desiredSub.SetGroupVersionKind(subscriptionGVK) // Update stripped this information
merged.SetGroupVersionKind(subscriptionGVK) // Update stripped this information

err = r.updateStatus(ctx, policy, updatedCond("Subscription"), updatedObj(desiredSub))
err = r.updateStatus(ctx, policy, updatedCond("Subscription"), updatedObj(merged))
if err != nil {
return fmt.Errorf("error updating status after updating the Subscription: %w", err)
return mergedSub, fmt.Errorf("error updating status after updating the Subscription: %w", err)
}
}

return nil
return mergedSub, nil
}

// buildSubscription bootstraps the subscription spec defined in the operator policy
Expand Down

0 comments on commit ef38ec5

Please sign in to comment.