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 6, 2024
1 parent 934c69b commit bd8bef9
Show file tree
Hide file tree
Showing 5 changed files with 253 additions and 58 deletions.
86 changes: 82 additions & 4 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 @@ -125,7 +138,7 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
return reconcile.Result{}, err
}

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

Expand All @@ -134,9 +147,74 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque

// FUTURE: more resource checks

// handle catalogsource
if err := r.handleCatalogSource(ctx, policy, subscription); 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,
subscription *operatorv1alpha1.Subscription,
) error {
watcher := opPolIdentifier(policy.Namespace, policy.Name)

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

if len(subscription.Status.Conditions) == 0 {
// Subscription not found, check the CatalogSource object
foundCatalogSrc, err := r.DynamicWatcher.Get(watcher, catalogSrcGVK,
catalogNS, catalogName)
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
}

// Iterate over all the conditions to determine the state of the CatalogSource
for _, condition := range subscription.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 @@ -422,7 +500,7 @@ func (r *OperatorPolicyReconciler) handleSubscription(
// For now, just mark it as compliant
err := r.updateStatus(ctx, policy, matchesCond("Subscription"), matchedObj(foundSub))
if err != nil {
return nil, fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err)
return nil, fmt.Errorf("error updating the status for a Subscription that matches: %w", err)
}

return mergedSub, 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",
)
})
})
})
22 changes: 5 additions & 17 deletions test/resources/case38_operator_install/case38_OpPlc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,20 @@ kind: OperatorPolicy
metadata:
name: test-operator-policy
spec:
remediationAction: enforce # or inform
remediationAction: inform # or inform
severity: medium
complianceType: musthave # or mustnothave
operatorGroup: # optional
name: og-single
namespace: op-1
namespace: default
target:
namespaces:
- op-1
- default
subscription:
channel: stable-3.8
name: project-quay
namespace: op-1
namespace: default
installPlanApproval: Automatic # may be overridden to Manual by other settings
source: operatorhubio-catalog
sourceNamespace: olm
startingCSV: quay-operator.v3.8.1
removalBehavior:
operatorGroups: DeleteIfUnused
subscriptions: Delete
clusterServiceVersions: Delete
installPlans: Keep
customResourceDefinitions: Keep
apiServiceDefinitions: Keep
statusConfig:
catalogSourceUnhealthy: StatusMessageOnly
deploymentsUnavailable: NonCompliant
upgradesAvailable: StatusMessageOnly
upgradesProgressing: NonCompliant
startingCSV: quay-operator.v3.8.1
34 changes: 0 additions & 34 deletions test/resources/operatorpolicy.yaml

This file was deleted.

0 comments on commit bd8bef9

Please sign in to comment.