-
Notifications
You must be signed in to change notification settings - Fork 185
Bug 1714158: Prevent cert-syncer to act on stale data #557
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
Bug 1714158: Prevent cert-syncer to act on stale data #557
Conversation
|
@tnozicka: This pull request references Bugzilla bug 1714158, which is invalid:
Comment DetailsIn 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 |
2 similar comments
|
/retest |
|
/retest |
|
@tnozicka: This pull request references Bugzilla bug 1714158, which is invalid:
Comment DetailsIn 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. |
|
/bugzilla refresh |
|
@tnozicka: This pull request references Bugzilla bug 1714158, which is invalid:
Comment DetailsIn 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. |
|
/bugzilla refresh |
|
@tnozicka: This pull request references Bugzilla bug 1714158, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn 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. |
c525475 to
17ea1fb
Compare
| continue | ||
|
|
||
| case apierrors.IsNotFound(err) && cm.Optional: | ||
| // Check with the live call it is really missing |
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.
here it is
|
@tnozicka: This pull request references Bugzilla bug 1714158, which is valid. DetailsIn 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. |
| // serving secure traffic. | ||
| // We are disabling this because it doesn't work today and customers aren't going to be able to get the kube service network options right. | ||
| // Customers may only use SNI. I'm leaving this code in case we ever come up with a way to make an SNI-like thing based on IPs. | ||
| var ObserveDefaultUserServingCertificate configobserver.ObserveConfigFunc = (&apiServerObserver{ |
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.
why is this commit good? removal without explanation in the commit msg is not good.
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.
this is bump of openshift/api not something this PR introduced
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.
but it changes pkg/operator/configobservation/apiserver/observe_apiserver.go, not vendor/openshift/api.
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.
this was a backport of breaking api change which just disabled the code, but left it there. this PR won the first bump and dealing with incompatibilities
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts, tnozicka The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds 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. |
|
The 4.2 is in the right state with right priority and the fix in master soaked, approving. |
|
@tnozicka: All pull requests linked via external trackers have merged. Bugzilla bug 1714158 has been moved to the MODIFIED state. DetailsIn 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. |
Requires:
picking up a backport of d0f4b03 from #520 to fix breaking api change in openshift/api#375 we got with the bump
/cc @sttts