Bug 1862481: Correctly count non-default Marketplace CRs #322
Conversation
Problem: It is possible for the marketplace operator to report that deprecated CRs are present on cluster despite no CRs being present. Solution: Marketplace already searches for non-default CRs on cluster when reporting status. Marketplace will now use this search to report the number of deprecated CRs on cluster.
awgreene
changed the title
openshift-ci-robot
added
the
bugzilla/severity-low
|
@awgreene: This pull request references Bugzilla bug 1862481, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
bugzilla/valid-bug
| } | ||
| for _, opsrc := range opsrcs.Items { | ||
| if !defaults.IsDefaultSource(opsrc.Name) { | ||
| return true, nil | ||
| nonDefaultOpsrcCount++ |
There was a problem hiding this comment.
Why do we need to keep a count at all? Why can't we just have knowledge of if they exist on the cluster?
There was a problem hiding this comment.
I do not have a strong preference, but we lose information regarding how many Marketplace CRs exist on cluster by going binary. If you can explain potential benefits, I'll be happy to make the change.
There was a problem hiding this comment.
I actually think it's beneficial to have a count coz it enhances the UX for an admin.
If it is binary:
- Sees the alert
- Deletes an opsrc
- Sees the alert still exists (but I already deleted a custom resource)
If it is a count:
- Sees the alert
- Deletes a CR and sees the count go down
- Alert tells the admin there's more CRs to be deleted.
|
/lgtm |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: awgreene, kevinrizza The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
Ci issue:
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
1 similar comment
|
/retest |
|
@awgreene: All pull requests linked via external trackers have merged: operator-framework/operator-marketplace#322. Bugzilla bug 1862481 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/cherry-pick release-4.5 |
|
@awgreene: new pull request created: #324 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/cherry-pick release-4.4 |
|
@jonesbr17: only operator-framework org members may request cherry picks. You can still do the cherry-pick manually. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@awgreene would it be possible to get this cherry-picked to release-4.4 as well? Thanks. |
|
@awgreene Any updates on this? This bug is still present in 4.4. Can this be cherry-picked? |
|
Good morning @jonesbr17, this bugfix was backported to 4.4 in #326. |
Problem: It is possible for the marketplace operator to report that
deprecated CRs are present on cluster despite no CRs being present.
Solution: Marketplace already searches for non-default CRs on cluster
when reporting status. Marketplace will now use this search to report
the number of deprecated CRs on cluster.