-
Notifications
You must be signed in to change notification settings - Fork 19
OCPBUGS-59211: pkg/migrator: Log some errors on V(2) #233
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
base: main
Are you sure you want to change the base?
Conversation
|
@tchap: This pull request references Jira Issue OCPBUGS-59211, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tchap The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
7502784 to
c7fb955
Compare
|
/retest-required |
c7fb955 to
4dc956c
Compare
Some errors that can happen when writing resources back during migration are not particularly significant, yet they can cause a lot or log spam. This change marks some errors coming from the try function to be logged on V(2) so that there are not that many log messages. These include: * conflict * not found * UID precondition failed (effectively a conflict)
4dc956c to
b725809
Compare
|
Has this been proposed upstream yet? Based on your description, it sounds like the logs may be legitimately misleveled. It'd be a shame to carry a patch forever if it could be done upstream first. |
No really, happy to send an upstream PR. For some reason I deduced the upstream repo seems rather unmaintained. |
K8s has moved this project in-tree. I would expect this means upstream is to be archived? |
|
AFAIU the in-tree SVM is still in alpha, and it operates on different GVRs than out-of-tree SVM, so it will be more of a replacement than a hoist of the existing SVM into k/k. I think we should continue to contribute to maintenance of the upstream project at least until the in-tree SVM graduates to enabled-by-default and we've adopted it. |
|
@tchap: The following test failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
Created kubernetes-sigs#129 |
|
/jira refresh |
|
@flavianmissi: This pull request references Jira Issue OCPBUGS-59211, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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 openshift-eng/jira-lifecycle-plugin repository. |
|
Honestly this is pretty problematic to get reviewed upstream. OWNERS are either on long-term vacation or not reading messages on Slack... |
is there an upstream sig we can bring this up with @tchap? it's in everyone's interest that upstream projects are actively maintained. |
I can try to find out, but in any case you need an OWNER to merge the PR, and none of them is available 😐 |
|
SIG API Machinery owns this upstream. |
Some errors that can happen when writing resources back during migration are not particularly significant, yet they can cause a lot of log spam.
This change marks some errors coming from the try function to be logged on V(2) so that there are not that many log messages. These include:
See the linked issue for the context. The original issue is about logging too many messages regarding OAuth tokens being deleted before written back. So I extended the fix a little bit, but I am open to any pointers in a good direction.