-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
RemoveRoute does not need service key cleanup #10855
Conversation
LGTM. That matches what @DirectXMan12 and I were wondering about. |
Looks correct. [test] |
[testextended][extended:core(router)] |
LGTM |
Is this 1.3.0 or 1.3.1? |
d53b92b
to
12d7c69
Compare
Evaluated for origin testextended up to 12d7c69 |
@liggitt Should be 1.3.0 if its not too late. |
Evaluated for origin test up to 12d7c69 |
Also, in a follow up please add tests that verify this and the previous error are caught. |
continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin_extended/476/) (Extended Tests: core(router)) |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8768/) |
Tests have succeeded. [merge] |
[merge] |
@liggitt OK, since this is not a regression, I've been talked into 1.3.1 with a release note in 1.3. |
Evaluated for origin merge up to 12d7c69 |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8772/) (Image: devenv-rhel7_4996) |
[merge] but don't forget this needs a cherry pick.
|
Remember we are in cherry pick mode now.
|
@rajatchopra can you make sure that this gets over to 1.3.1 please? |
Done. Number ose/388 |
Fixes issue #10853
The old service keys do not let the route be deleted (or even their service references). RemoveRoute should just yank the entire route out.
@knobunc @DirectXMan12
Bug 1374772