Skip to content

Commit

Permalink
Enable status reporting for CatalogSource in OperatorPolicy
Browse files Browse the repository at this point in the history
Following the schema of implementing multiple `handle` functions,
the CatalogSource status reporting logic utilizes the same
`updateStatus` function to facilitate the `conditions` and
`relatedObject` field updates on the related OperatorPolicy. The
controller reads the conditions field on the watched Subscription to
determine the health of the CatalogSource. In the event a Subscription
is not created due to either an `inform` OperatorPolicy or an error,
the controller directly checks if the CatalogSource object exists on
the cluster to propagate relevant health information up to the
OperatorPolicy.

ref: https://issues.redhat.com/browse/ACM-9285
Signed-off-by: Jason Zhang <jaszhang@redhat.com>
  • Loading branch information
zyjjay committed Feb 5, 2024
1 parent 3da5031 commit c5215ec
Show file tree
Hide file tree
Showing 4 changed files with 265 additions and 40 deletions.
102 changes: 99 additions & 3 deletions controllers/operatorpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,21 @@ const (
)

var (
subscriptionGVK = schema.GroupVersionKind{Group: "operators.coreos.com", Version: "v1alpha1", Kind: "Subscription"}
operatorGroupGVK = schema.GroupVersionKind{Group: "operators.coreos.com", Version: "v1", Kind: "OperatorGroup"}
subscriptionGVK = schema.GroupVersionKind{
Group: "operators.coreos.com",
Version: "v1alpha1",
Kind: "Subscription",
}
operatorGroupGVK = schema.GroupVersionKind{
Group: "operators.coreos.com",
Version: "v1",
Kind: "OperatorGroup",
}
catalogSrcGVK = schema.GroupVersionKind{
Group: "operators.coreos.com",
Version: "v1alpha1",
Kind: "CatalogSource",
}
)

// OperatorPolicyReconciler reconciles a OperatorPolicy object
Expand Down Expand Up @@ -133,9 +146,92 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque

// FUTURE: more resource checks

// handle catalogsource
if err := r.handleCatalogSource(ctx, policy); err != nil {
OpLog.Error(err, "Error handling CatalogSource")

return reconcile.Result{}, err
}

return reconcile.Result{}, nil
}

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

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

catalogName := desiredSub.Spec.CatalogSource
catalogNS := desiredSub.Spec.CatalogSourceNamespace

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

if foundSub == nil {
// Subscription not found, check the CatalogSource object
foundCatalogSrc, err := r.DynamicWatcher.Get(watcher, catalogSrcGVK,
desiredSub.Spec.CatalogSourceNamespace, desiredSub.Spec.CatalogSource)
if err != nil {
return fmt.Errorf("error getting CatalogSource: %w", err)
}

// Conditionally emit health event using the defined status template
isMissing := foundCatalogSrc == nil

err = r.updateStatus(ctx, policy, catalogSourceFindCond(isMissing),
catalogSourceObj(catalogName, catalogNS, isMissing))
if err != nil {
return fmt.Errorf("error updating the status for CatalogSource when Subscription DNE on cluster: %w", err)
}

return nil
}

unstructured := foundSub.UnstructuredContent()
var subObj operatorv1alpha1.Subscription

err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructured, &subObj)
if err != nil {
return fmt.Errorf("failed to convert type from unstructured to Subscription")
}

// Iterate over all the conditions to determine the state of the CatalogSource
for _, condition := range subObj.Status.Conditions {
if condition.Type == catalogSrcConditionType {
status := metav1.ConditionFalse
if string(condition.Status) == string(metav1.ConditionTrue) {
status = metav1.ConditionTrue
}

isMissing := string(condition.Status) == string(metav1.ConditionTrue)

healthCondition := metav1.Condition{
Type: string(condition.Type),
Status: status,
LastTransitionTime: *condition.LastTransitionTime,
Reason: condition.Reason,
Message: condition.Message,
}

err := r.updateStatus(ctx, policy, healthCondition, catalogSourceObj(catalogName, catalogNS, isMissing))
if err != nil {
return fmt.Errorf("error updating the status for a missing CatalogSource: %w", err)
}
}
}

return nil
}

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

Expand Down Expand Up @@ -414,7 +510,7 @@ func (r *OperatorPolicyReconciler) handleSubscription(ctx context.Context, polic
// 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 fmt.Errorf("error updating the status for a Subscription that matches: %w", err)
}

return nil
Expand Down
63 changes: 60 additions & 3 deletions controllers/operatorpolicy_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,17 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C
}

// FUTURE: check additional conditions
idx, cond = policy.Status.GetCondition(catalogSrcConditionType)
if idx == -1 {
messages = append(messages, "the status of the CatalogSource is unknown")
foundNonCompliant = true
} else {
messages = append(messages, cond.Message)

if cond.Status != metav1.ConditionFalse {
foundNonCompliant = true
}
}

if foundNonCompliant {
return metav1.Condition{
Expand Down Expand Up @@ -274,9 +285,10 @@ func (r *OperatorPolicyReconciler) emitComplianceEvent(
}

const (
compliantConditionType = "Compliant"
opGroupConditionType = "OperatorGroupCompliant"
subConditionType = "SubscriptionCompliant"
compliantConditionType = "Compliant"
opGroupConditionType = "OperatorGroupCompliant"
subConditionType = "SubscriptionCompliant"
catalogSrcConditionType = "CatalogSourcesUnhealthy"
)

func condType(kind string) string {
Expand Down Expand Up @@ -373,6 +385,51 @@ var opGroupTooManyCond = metav1.Condition{
Message: "there is more than one OperatorGroup in the namespace",
}

// catalogSourceFindCond is a conditionally compliant condition with reason
// based on the `isMissing` parameter
func catalogSourceFindCond(isMissing bool) metav1.Condition {
status := metav1.ConditionFalse
reason := "CatalogSourcesFound"
message := "CatalogSource was found"

if isMissing {
status = metav1.ConditionTrue
reason = "CatalogSourcesNotFound"
message = "CatalogSource was not found"
}

return metav1.Condition{
Type: "CatalogSourcesUnhealthy",
Status: status,
Reason: reason,
Message: message,
}
}

// catalogSourceObj returns a conditionally compliant RelatedObject with reason based on the `isMissing` parameter
func catalogSourceObj(catalogName string, catalogNS string, isMissing bool) policyv1.RelatedObject {
compliance := string(policyv1.NonCompliant)
reason := reasonWantFoundDNE

if !isMissing {
compliance = string(policyv1.Compliant)
reason = reasonWantFoundExists
}

return policyv1.RelatedObject{
Object: policyv1.ObjectResource{
Kind: "CatalogSource",
APIVersion: "operators.coreos.com/v1alpha1",
Metadata: policyv1.ObjectMetadata{
Name: catalogName,
Namespace: catalogNS,
},
},
Compliant: compliance,
Reason: reason,
}
}

// missingWantedObj returns a NonCompliant RelatedObject with reason = 'Resource not found but should exist'
func missingWantedObj(obj client.Object) policyv1.RelatedObject {
return policyv1.RelatedObject{
Expand Down
106 changes: 106 additions & 0 deletions test/e2e/case38_install_operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,4 +550,110 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun
)
})
})
Describe("Test status reporting for catalogsource", Ordered, func() {
const (
OpPlcYAML = "../resources/case38_operator_install/operator-policy-with-group.yaml"
OpPlcName = "oppol-with-group"
subName = "project-quay"
)

BeforeAll(func() {
By("Applying creating a ns and the test policy")
utils.Kubectl("create", "ns", opPolTestNS)
DeferCleanup(func() {
utils.Kubectl("delete", "ns", opPolTestNS)
})

createObjWithParent(parentPolicyYAML, parentPolicyName,
OpPlcYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy)
})

It("Should initially be Compliant", func() {
check(
OpPlcName,
false,
[]policyv1.RelatedObject{{
Object: policyv1.ObjectResource{
Kind: "CatalogSource",
APIVersion: "operators.coreos.com/v1alpha1",
Metadata: policyv1.ObjectMetadata{
Name: "operatorhubio-catalog",
Namespace: opPolTestNS,
},
},
Compliant: "Compliant",
Reason: "Resource found as expected",
}},
metav1.Condition{
Type: "CatalogSourcesUnhealthy",
Status: metav1.ConditionFalse,
Reason: "CatalogSourcesFound",
Message: "CatalogSource was found",
},
"CatalogSource was found",
)
})

It("Should remain compliant when policy is enforced", func() {
By("Enforcing the policy")
utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", opPolTestNS, "--type=json", "-p",
`[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`)

By("Checking if the condition fields were inherited from the Subscription")
check(
OpPlcName,
false,
[]policyv1.RelatedObject{{
Object: policyv1.ObjectResource{
Kind: "CatalogSource",
APIVersion: "operators.coreos.com/v1alpha1",
Metadata: policyv1.ObjectMetadata{
Name: "operatorhubio-catalog",
Namespace: opPolTestNS,
},
},
Compliant: "Compliant",
Reason: "Resource found as expected",
}},
metav1.Condition{
Type: "CatalogSourcesUnhealthy",
Status: metav1.ConditionFalse,
Reason: "AllCatalogSourcesHealthy",
Message: "all available catalogsources are healthy",
},
"all available catalogsources are healthy",
)
})

It("Should become NonCompliant when CatalogSource fails", func() {
By("Patching the policy to reference a catalogSource that DNE to emulate failure")
utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", opPolTestNS, "--type=json", "-p",
`[{"op": "replace", "path": "/spec/subscription/source", "value": "fakeName"}]`)

By("Checking the conditions and relatedObj in the policy")
check(
OpPlcName,
true,
[]policyv1.RelatedObject{{
Object: policyv1.ObjectResource{
Kind: "CatalogSource",
APIVersion: "operators.coreos.com/v1alpha1",
Metadata: policyv1.ObjectMetadata{
Name: "fakeName",
Namespace: opPolTestNS,
},
},
Compliant: "NonCompliant",
Reason: "Resource not found but should exist",
}},
metav1.Condition{
Type: "CatalogSourcesUnhealthy",
Status: metav1.ConditionTrue,
Reason: "UnhealthyCatalogSourceFound",
Message: "targeted catalogsource olm/fakeName missing",
},
"targeted catalogsource olm/fakeName missing",
)
})
})
})
34 changes: 0 additions & 34 deletions test/resources/operatorpolicy.yaml

This file was deleted.

0 comments on commit c5215ec

Please sign in to comment.