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 5, 2024
1 parent 3da5031 commit b45e55c
Showing 1 changed file with 29 additions and 20 deletions.
49 changes: 29 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,106 @@ 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 nil, 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 nil, 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 nil, 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 nil, 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 nil, 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 nil, 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 nil, 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 nil, 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 nil, 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 nil, 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 nil, 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 nil, 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 nil, 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 b45e55c

Please sign in to comment.