Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable status reporting for CatalogSource in OperatorPolicy #195

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 81 additions & 1 deletion controllers/operatorpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (

const (
OperatorControllerName string = "operator-policy-controller"
CatalogSourceReady string = "READY"
)

var (
Expand All @@ -58,6 +59,11 @@ var (
Version: "v1",
Kind: "Deployment",
}
catalogSrcGVK = schema.GroupVersionKind{
Group: "operators.coreos.com",
Version: "v1alpha1",
Kind: "CatalogSource",
}
)

// OperatorPolicyReconciler reconciles a OperatorPolicy object
Expand Down Expand Up @@ -165,9 +171,83 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque
return reconcile.Result{}, err
}

// 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)

var catalogName string
var catalogNS string

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

catalogName = sub.Spec.CatalogSource
catalogNS = sub.Spec.CatalogSourceNamespace
} else {
catalogName = subscription.Spec.CatalogSource
catalogNS = subscription.Spec.CatalogSourceNamespace
}

// Check if CatalogSource exists
foundCatalogSrc, err := r.DynamicWatcher.Get(watcher, catalogSrcGVK,
catalogNS, catalogName)
if err != nil {
return fmt.Errorf("error getting CatalogSource: %w", err)
}

isMissing := foundCatalogSrc == nil
isUnhealthy := isMissing

if !isMissing {
// CatalogSource is found, initiate health check
catalogSrcUnstruct := foundCatalogSrc.DeepCopy()
catalogSrc := new(operatorv1alpha1.CatalogSource)

err := runtime.DefaultUnstructuredConverter.
FromUnstructured(catalogSrcUnstruct.Object, catalogSrc)
if err != nil {
return fmt.Errorf("error converting the retrieved CatalogSource to the Go type: %w", err)
}

if catalogSrc.Status.GRPCConnectionState == nil {
// Unknown State
err := r.updateStatus(ctx, policy, catalogSourceUnknownCond, catalogSrcUnknownObj(catalogName, catalogNS))
if err != nil {
return fmt.Errorf("error retrieving the status for a CatalogSource: %w", err)
}

return nil
}

CatalogSrcState := catalogSrc.Status.GRPCConnectionState.LastObservedState
isUnhealthy = (CatalogSrcState != CatalogSourceReady)
}

err = r.updateStatus(ctx, policy, catalogSourceFindCond(isUnhealthy, isMissing),
catalogSourceObj(catalogName, catalogNS, isUnhealthy, isMissing))
if err != nil {
return fmt.Errorf("error updating the status for a 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 @@ -453,7 +533,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
94 changes: 94 additions & 0 deletions controllers/operatorpolicy_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,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 @@ -305,6 +316,7 @@ const (
subConditionType = "SubscriptionCompliant"
csvConditionType = "ClusterServiceVersionCompliant"
deploymentConditionType = "DeploymentCompliant"
catalogSrcConditionType = "CatalogSourcesUnhealthy"
)

func condType(kind string) string {
Expand Down Expand Up @@ -468,6 +480,88 @@ var noDeploymentsCond = metav1.Condition{
Message: "The ClusterServiceVersion is missing, thus meaning there are no relevant deployments",
}

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

if isUnhealthy {
status = metav1.ConditionTrue
reason = "CatalogSourcesFoundUnhealthy"
message = "CatalogSource was found but is unhealthy"
}

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

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

// catalogSourceUnknownCond is a NonCompliant condition
var catalogSourceUnknownCond = metav1.Condition{
Type: "CatalogSourcesUnknownState",
Status: metav1.ConditionTrue,
Reason: "LastObservedUnknown",
Message: "Could not determine last observed state of CatalogSource",
}

// catalogSourceObj returns a conditionally compliant RelatedObject with reason based on the
// `isUnhealthy` and `isMissing` parameters
func catalogSourceObj(catalogName string, catalogNS string, isUnhealthy bool, isMissing bool) policyv1.RelatedObject {
compliance := string(policyv1.Compliant)
reason := reasonWantFoundExists

if isUnhealthy {
compliance = string(policyv1.NonCompliant)
reason = reasonWantFoundExists + " but is unhealthy"
}

if isMissing {
compliance = string(policyv1.NonCompliant)
reason = reasonWantFoundDNE
}

return policyv1.RelatedObject{
Object: policyv1.ObjectResource{
Kind: catalogSrcGVK.Kind,
APIVersion: catalogSrcGVK.GroupVersion().String(),
Metadata: policyv1.ObjectMetadata{
Name: catalogName,
Namespace: catalogNS,
},
},
Compliant: compliance,
Reason: reason,
}
}

// catalogSrcUnknownObj returns a NonCompliant RelatedObject with
// reason = 'Resource found but current state is unknown'
func catalogSrcUnknownObj(catalogName string, catalogNS string) policyv1.RelatedObject {
return policyv1.RelatedObject{
Object: policyv1.ObjectResource{
Kind: catalogSrcGVK.Kind,
APIVersion: catalogSrcGVK.GroupVersion().String(),
Metadata: policyv1.ObjectMetadata{
Name: catalogName,
Namespace: catalogNS,
},
},
Compliant: string(policyv1.NonCompliant),
Reason: "Resource found but current state is unknown",
}
}

// 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
145 changes: 142 additions & 3 deletions test/e2e/case38_install_operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,6 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun
createObjWithParent(parentPolicyYAML, parentPolicyName,
opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy)
})

It("Should initially notice the matching Subscription", func() {
check(
opPolName,
Expand Down Expand Up @@ -550,7 +549,6 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun
)
})
})

Describe("Test health checks on OLM resources after OperatorPolicy operator installation", Ordered, func() {
const (
opPolYAML = "../resources/case38_operator_install/operator-policy-no-group-enforce.yaml"
Expand Down Expand Up @@ -610,7 +608,6 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun
)
})
})

Describe("Test health checks on OLM resources on OperatorPolicy with failed CSV", Ordered, func() {
const (
opPolYAML = "../resources/case38_operator_install/operator-policy-no-group-csv-fail.yaml"
Expand Down Expand Up @@ -672,4 +669,146 @@ 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"
catSrcName = "operatorhubio-catalog"
)

BeforeAll(func() {
By("Applying creating a ns and the test policy")
utils.Kubectl("create", "ns", opPolTestNS)
DeferCleanup(func() {
utils.Kubectl("patch", "catalogsource", catSrcName, "-n", "olm", "--type=json", "-p",
`[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/catalog:latest"}]`)
utils.Kubectl("delete", "ns", opPolTestNS)
})

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

It("Should initially show the CatalogSource is compliant", func() {
By("Checking the condition fields")
check(
OpPlcName,
false,
[]policyv1.RelatedObject{{
Object: policyv1.ObjectResource{
Kind: "CatalogSource",
APIVersion: "operators.coreos.com/v1alpha1",
Metadata: policyv1.ObjectMetadata{
Name: catSrcName,
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 the condition fields")
check(
OpPlcName,
false,
[]policyv1.RelatedObject{{
Object: policyv1.ObjectResource{
Kind: "CatalogSource",
APIVersion: "operators.coreos.com/v1alpha1",
Metadata: policyv1.ObjectMetadata{
Name: catSrcName,
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 become NonCompliant when CatalogSource DNE", 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: "CatalogSourcesNotFound",
Message: "CatalogSource was not found",
},
"CatalogSource was not found",
)
})
It("Should remain NonCompliant when CatalogSource fails", func() {
By("Patching the policy to point to an existing CatalogSource")
utils.Kubectl("patch", "operatorpolicy", OpPlcName, "-n", opPolTestNS, "--type=json", "-p",
`[{"op": "replace", "path": "/spec/subscription/source", "value": "operatorhubio-catalog"}]`)

By("Patching the CatalogSource to reference a broken image link")
utils.Kubectl("patch", "catalogsource", catSrcName, "-n", "olm", "--type=json", "-p",
`[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/fakecatalog:latest"}]`)

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: catSrcName,
Namespace: opPolTestNS,
},
},
Compliant: "NonCompliant",
Reason: "Resource found as expected but is unhealthy",
}},
metav1.Condition{
Type: "CatalogSourcesUnhealthy",
Status: metav1.ConditionTrue,
Reason: "CatalogSourcesFoundUnhealthy",
Message: "CatalogSource was found but is unhealthy",
},
"CatalogSource was found but is unhealthy",
)
})
})
})
Loading
Loading