-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Set DeleteStrategy for all Openshift resources #14337
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
Set DeleteStrategy for all Openshift resources #14337
Conversation
All registry.Store objects already set a non-nil DeleteStrategy. This change ensures that all future objects do so as well. Signed-off-by: Monis Khan <[email protected]>
DeleteStrategy is no longer optional, and thus must be set by all resources. Signed-off-by: Monis Khan <[email protected]>
…ptional Signed-off-by: Monis Khan <[email protected]>
Evaluated for origin test up to 00d9a88 |
@deads2k I can add said test as a separate PR if you think it is necessary on top of the changes to |
The errors in API server logs occur in master branch as well so not related to this PR. |
cc @spadgett @jwforres @benjaminapetersen if any of you guys want to see if this fixes the web console issue. |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1727/) (Base Commit: 423315b) |
LGTM - let me know if this fixes web console. |
Thanks will test in the morning |
If your upstream merges, I'm ok with it. |
LGTM (guess this qualifies as bug fix). |
confirmed on @enj 's machine that delete route from the console works now |
@mfojtik and yeah this definitely qualifies as a bug fix, otherwise you wont be able to delete routes in the console |
I will merge this once kubernetes/kubernetes/pull/46390 is merged. |
upstream PR is now approved for merge [merge][severity:bug] |
Evaluated for origin merge up to 00d9a88 |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/839/) (Base Commit: 39664e5) (Extended Tests: bug) (Image: devenv-rhel7_6281) |
[severity:blocker] |
DeleteStrategy
is no longer optional, and thus must be set by all resources.Fixes #14198
[test]
Seeing a lot of this in master log so something is off: