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
[release-4.5] Bug 1878297: remove duplicate catalog source pod when polling #1765
[release-4.5] Bug 1878297: remove duplicate catalog source pod when polling #1765
Conversation
…; remove duplicate pod immediately after imageID check; turn down polling logging
…ycles in the catalog reconciler.
@exdx: This pull request references Bugzilla bug 1878297, which is invalid:
Comment 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. |
@exdx: PR needs rebase. 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. |
@exdx: This pull request references Bugzilla bug 1878297, which is invalid:
Comment 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. |
@exdx: This pull request references Bugzilla bug 1878297, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 6 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. |
/retest |
1 similar comment
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/retest |
1 similar comment
/retest |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dinhxuanvu, ecordell, exdx 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/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. |
7 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 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. |
(patch manager) this seems like quite a large and risky change for a relatively low-impact issue. What am I missing? |
The following 4.5 backports should be brought in if we want to support better catalog polling UX for customers in 4.5.z. Polling will continue to work without these fixes but will not work as effectively.
Just depends how much we expect customers to use this feature in 4.5, versus in 4.6 where catalog polling is clearly being used as the default red hat operator catalogs will ship with polling enabled. |
Also worth pointing out that all the fixes work together to provide better UX, we can't really pick and choose which ones we want. In particular #1780 unlocks the rest of the fixes to work as intended. |
@operator-framework/patch-managers Catalog polling is not going to work properly in 4.5.z for Red Hat catalogs as well as for the IBM teams that are publishing catalogs for their customers without this fix going in. If we don't backport, shorter polling intervals won't work correctly (minimum will always be 15 minutes), there will always be multiple catalog pods running on the cluster per catalog (only one serves traffic), and edge cases where the updated catalog fails to start will not be handled well. So polling will still work in the base case, but the UX will be worse. As customers move more to the new index-based catalog format and adopt polling in their existing clusters this can be an issue. cc @kevinrizza |
Also in terms of risk all the changes in this PR are to one file that deals exclusively with polling logic. Catalog polling is an opt-in feature and these changes do not affect any other behavioral aspects of OLM. |
/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. |
@exdx: All pull requests linked via external trackers have merged: Bugzilla bug 1878297 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. |
Description of the change:
Fix of #1758
Motivation for the change:
Reviewer Checklist
/docs