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.

Sign up for GitHub

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

Jump to bottom

Update the operator policy messages #231

Merged
merged 3 commits into from
Apr 24, 2024
Merged

Update the operator policy messages #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0276169
Make the operator policy message case consistent
mprahl Apr 22, 2024
4ab61a0
Remove commas from operator policy messages
mprahl Apr 22, 2024
3a2f21e
Include the CSV name in compliance messages
mprahl Apr 23, 2024
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
11 changes: 7 additions & 4 deletions controllers/operatorpolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1393,11 +1393,14 @@ func (r *OperatorPolicyReconciler) mustnothaveCSV(
return nil, updateStatus(policy, keptCond("ClusterServiceVersion"), relatedCSVs...), nil
}

csvNames := make([]string, 0, len(csvList))

for i := range csvList {
relatedCSVs = append(relatedCSVs, foundNotWantedObj(&csvList[i]))
csvNames = append(csvNames, csvList[i].GetName())
}

changed := updateStatus(policy, foundNotWantedCond("ClusterServiceVersion"), relatedCSVs...)
changed := updateStatus(policy, foundNotWantedCond("ClusterServiceVersion", csvNames...), relatedCSVs...)

if policy.Spec.RemediationAction.IsInform() {
return nil, changed, nil
Expand All @@ -1421,7 +1424,7 @@ func (r *OperatorPolicyReconciler) mustnothaveCSV(

err := r.Delete(ctx, &csvList[i])
if err != nil {
changed := updateStatus(policy, foundNotWantedCond("ClusterServiceVersion"), relatedCSVs...)
changed := updateStatus(policy, foundNotWantedCond("ClusterServiceVersion", csvNames...), relatedCSVs...)

if anyAlreadyDeleting {
// reset the "early" conditions to avoid flapping
Expand All @@ -1439,10 +1442,10 @@ func (r *OperatorPolicyReconciler) mustnothaveCSV(
// reset the "early" conditions to avoid flapping
earlyConds = []metav1.Condition{}

return earlyConds, updateStatus(policy, deletingCond("ClusterServiceVersion"), relatedCSVs...), nil
return earlyConds, updateStatus(policy, deletingCond("ClusterServiceVersion", csvNames...), relatedCSVs...), nil
}

updateStatus(policy, deletedCond("ClusterServiceVersion"), relatedCSVs...)
updateStatus(policy, deletedCond("ClusterServiceVersion", csvNames...), relatedCSVs...)

return earlyConds, true, nil
}
Expand Down
54 changes: 42 additions & 12 deletions controllers/operatorpolicy_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,12 +428,22 @@ func missingNotWantedCond(kind string) metav1.Condition {

// foundNotWantedCond returns a NonCompliant condition with a Reason like '____Present'
// and a Message like 'the ____ is present'
func foundNotWantedCond(kind string) metav1.Condition {
func foundNotWantedCond(kind string, identifiers ...string) metav1.Condition {
var extraInfo string

if len(identifiers) == 1 {
extraInfo = fmt.Sprintf(" (%s) is", identifiers[0])
} else if len(identifiers) > 1 {
extraInfo = fmt.Sprintf("s (%s) are", strings.Join(identifiers, ", "))
} else {
extraInfo = " is"
}

return metav1.Condition{
Type: condType(kind),
Status: metav1.ConditionFalse,
Reason: kind + "Present",
Message: "the " + kind + " is present",
Message: "the " + kind + extraInfo + " present",
}
}

Expand All @@ -450,23 +460,43 @@ func createdCond(kind string) metav1.Condition {

// deletedCond returns a Compliant condition, with a Reason like '____Deleted',
// and a Message like 'the ____ was deleted'
func deletedCond(kind string) metav1.Condition {
func deletedCond(kind string, identifiers ...string) metav1.Condition {
var extraInfo string

if len(identifiers) == 1 {
extraInfo = fmt.Sprintf(" (%s) was", identifiers[0])
} else if len(identifiers) > 1 {
extraInfo = fmt.Sprintf("s (%s) were", strings.Join(identifiers, ", "))
} else {
extraInfo = " was"
}

return metav1.Condition{
Type: condType(kind),
Status: metav1.ConditionTrue,
Reason: kind + "Deleted",
Message: "the " + kind + " was deleted",
Message: "the " + kind + extraInfo + " deleted",
}
}

// deletingCond returns a NonCompliant condition, with a Reason like '____Deleting',
// and a Message like 'the ____ has a deletion timestamp'
func deletingCond(kind string) metav1.Condition {
func deletingCond(kind string, identifiers ...string) metav1.Condition {
var extraInfo string

if len(identifiers) == 1 {
extraInfo = fmt.Sprintf(" (%s) has", identifiers[0])
} else if len(identifiers) > 1 {
extraInfo = fmt.Sprintf("s (%s) have", strings.Join(identifiers, ", "))
} else {
extraInfo = " has"
}

return metav1.Condition{
Type: condType(kind),
Status: metav1.ConditionFalse,
Reason: kind + "Deleting",
Message: "the " + kind + " has a deletion timestamp",
Message: "the " + kind + extraInfo + " a deletion timestamp",
}
}

Expand Down Expand Up @@ -687,7 +717,7 @@ func buildCSVCond(csv *operatorv1alpha1.ClusterServiceVersion) metav1.Condition
Type: condType(csv.Kind),
Status: status,
Reason: string(csv.Status.Reason),
Message: "ClusterServiceVersion - " + csv.Status.Message,
Message: "ClusterServiceVersion (" + csv.Name + ") - " + csv.Status.Message,
}
}

Expand All @@ -696,23 +726,23 @@ var noCSVCond = metav1.Condition{
Type: csvConditionType,
Status: metav1.ConditionFalse,
Reason: "RelevantCSVNotFound",
Message: "A relevant installed ClusterServiceVersion could not be found",
Message: "a relevant installed ClusterServiceVersion could not be found",
}

// noCRDCond is a Compliant condition for when no CRDs are found
var noCRDCond = metav1.Condition{
Type: crdConditionType,
Status: metav1.ConditionTrue,
Reason: "RelevantCRDNotFound",
Message: "No CRDs were found for the operator",
Message: "no CRDs were found for the operator",
}

// crdFoundCond is a Compliant condition for when CRDs are found
var crdFoundCond = metav1.Condition{
Type: crdConditionType,
Status: metav1.ConditionTrue,
Reason: "RelevantCRDFound",
Message: "There are CRDs present for the operator",
Message: "there are CRDs present for the operator",
}

// buildDeploymentCond creates a Condition for deployments. If any are not at their
Expand Down Expand Up @@ -758,7 +788,7 @@ var noDeploymentsCond = metav1.Condition{
Type: deploymentConditionType,
Status: metav1.ConditionTrue,
Reason: "NoRelevantDeployments",
Message: "The ClusterServiceVersion is missing, thus meaning there are no relevant deployments",
Message: "there are no relevant deployments because the ClusterServiceVersion is missing",
}

// catalogSourceFindCond is a conditionally compliant condition with reason
Expand Down Expand Up @@ -793,7 +823,7 @@ var catalogSourceUnknownCond = metav1.Condition{
Type: "CatalogSourcesUnknownState",
Status: metav1.ConditionTrue,
Reason: "LastObservedUnknown",
Message: "Could not determine last observed state of CatalogSource",
Message: "could not determine last observed state of CatalogSource",
}

// missingWantedObj returns a NonCompliant RelatedObject with reason = 'Resource not found but should exist'
Expand Down
47 changes: 26 additions & 21 deletions test/e2e/case38_install_operator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,12 +745,15 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Reason: "InstallSucceeded",
}},
metav1.Condition{
Type: "ClusterServiceVersionCompliant",
Status: metav1.ConditionTrue,
Reason: "InstallSucceeded",
Message: "ClusterServiceVersion - install strategy completed with no errors",
Type: "ClusterServiceVersionCompliant",
Status: metav1.ConditionTrue,
Reason: "InstallSucceeded",
Message: "ClusterServiceVersion (quay-operator.v3.8.13) - install strategy completed with " +
"no errors",
},
"ClusterServiceVersion - install strategy completed with no errors",
regexp.QuoteMeta(
"ClusterServiceVersion (quay-operator.v3.8.13) - install strategy completed with no errors",
),
)
})

Expand Down Expand Up @@ -863,11 +866,13 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Type: "ClusterServiceVersionCompliant",
Status: metav1.ConditionFalse,
Reason: "UnsupportedOperatorGroup",
Message: "ClusterServiceVersion - AllNamespaces InstallModeType not supported," +
" cannot configure to watch all namespaces",
Message: "ClusterServiceVersion (etcdoperator.v0.9.2) - AllNamespaces InstallModeType not " +
"supported, cannot configure to watch all namespaces",
},
"ClusterServiceVersion - AllNamespaces InstallModeType not supported,"+
" cannot configure to watch all namespaces",
regexp.QuoteMeta(
"ClusterServiceVersion (etcdoperator.v0.9.2) - AllNamespaces InstallModeType not supported,"+
" cannot configure to watch all namespaces",
),
)
})

Expand Down Expand Up @@ -1328,9 +1333,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Type: "CustomResourceDefinitionCompliant",
Status: metav1.ConditionTrue,
Reason: "RelevantCRDNotFound",
Message: "No CRDs were found for the operator",
Message: "no CRDs were found for the operator",
},
"No CRDs were found for the operator",
"no CRDs were found for the operator",
)
})

Expand Down Expand Up @@ -1384,9 +1389,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Type: "CustomResourceDefinitionCompliant",
Status: metav1.ConditionTrue,
Reason: "RelevantCRDFound",
Message: "There are CRDs present for the operator",
Message: "there are CRDs present for the operator",
},
"There are CRDs present for the operator",
"there are CRDs present for the operator",
)
})
})
Expand Down Expand Up @@ -1667,9 +1672,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Type: "CustomResourceDefinitionCompliant",
Status: metav1.ConditionTrue,
Reason: "RelevantCRDNotFound",
Message: "No CRDs were found for the operator",
Message: "no CRDs were found for the operator",
},
`No CRDs were found for the operator`,
`no CRDs were found for the operator`,
)
check(
opPolName,
Expand Down Expand Up @@ -1808,9 +1813,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Type: "ClusterServiceVersionCompliant",
Status: metav1.ConditionFalse,
Reason: "ClusterServiceVersionPresent",
Message: "the ClusterServiceVersion is present",
Message: "the ClusterServiceVersion (quay-operator.v3.8.13) is present",
},
`the ClusterServiceVersion is present`,
regexp.QuoteMeta("the ClusterServiceVersion (quay-operator.v3.8.13) is present"),
)
check(
opPolName,
Expand Down Expand Up @@ -2142,9 +2147,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Type: "ClusterServiceVersionCompliant",
Status: metav1.ConditionFalse,
Reason: "ClusterServiceVersionDeleting",
Message: "the ClusterServiceVersion has a deletion timestamp",
Message: "the ClusterServiceVersion (" + csvName + ") has a deletion timestamp",
},
`the ClusterServiceVersion was deleted`,
regexp.QuoteMeta("the ClusterServiceVersion ("+csvName+") was deleted"),
)
desiredCRDObjects := make([]policyv1.RelatedObject, 0)
for _, name := range crdNames {
Expand Down Expand Up @@ -2275,7 +2280,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Reason: "ClusterServiceVersionNotPresent",
Message: "the ClusterServiceVersion is not present",
},
`the ClusterServiceVersion was deleted`,
regexp.QuoteMeta("the ClusterServiceVersion (quay-operator.v3.8.13) was deleted"),
)
check(
opPolName,
Expand All @@ -2295,7 +2300,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Type: "CustomResourceDefinitionCompliant",
Status: metav1.ConditionTrue,
Reason: "RelevantCRDNotFound",
Message: "No CRDs were found for the operator",
Message: "no CRDs were found for the operator",
},
`the CustomResourceDefinition was deleted`,
)
Expand Down