Skip to content
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 1462067 - Don't set propagationPolicy when deleting pod immediately #1792

Merged
merged 1 commit into from
Jul 5, 2017

Conversation

spadgett
Copy link
Member

@spadgett spadgett commented Jul 3, 2017

This change removes the propagation policy from delete options when force deleting a pod from the web console. It makes the web console DELETE request consistent with oc delete --now --force (only when delete immediately is checked).

@jwforres WIP because I haven't been able to verify against a pod that is really stuck in terminating. I'm concerned we might have a GC bug with propagation policy foreground and grace period 0.

@derekwaynecarr Do you know how I can purposely get a pod stuck in Terminating state to test this fix?

cc @deads2k

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1462067

@spadgett spadgett requested a review from jwforres July 3, 2017 17:21
Copy link
Member

@jwforres jwforres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change LGTM

@jwforres
Copy link
Member

jwforres commented Jul 5, 2017

We've been trying to avoid making any special exceptions in the console though, so far every weird case we have hit has been a bug with GC.

@spadgett
Copy link
Member Author

spadgett commented Jul 5, 2017

If you're trying to force delete something, foreground propagation seems wrong. But this comment worries me:

However console (v3.6.133) doesn't work, it still put pod in Terminating status. Then try again by CLI command and now pod can't be removed.

https://bugzilla.redhat.com/show_bug.cgi?id=1462067#c12

@jwforres
Copy link
Member

jwforres commented Jul 5, 2017

Agreed this change is OK then and we need to spawn a separate issue for the fact that GC + Terminating can get pods permanently stuck in storage.

@spadgett spadgett changed the title [WIP] Bug 1462067 - Don't set propagationPolicy when deleting pod immediately Bug 1462067 - Don't set propagationPolicy when deleting pod immediately Jul 5, 2017
@jwforres
Copy link
Member

jwforres commented Jul 5, 2017

[merge]

@jwforres
Copy link
Member

jwforres commented Jul 5, 2017

looks like the webdriver start flake [merge]

@spadgett
Copy link
Member Author

spadgett commented Jul 5, 2017

Flake

Selenium standalone server started at http://172.18.5.43:37253/wd/hub
ERROR - Unable to start a WebDriver session.

/data/src/github.com/openshift/origin-web-console/node_modules/selenium-webdriver/lib/atoms/error.js:113
  var template = new Error(this.message);
                 ^
UnknownError: Unable to connect to host 127.0.0.1 on port 7055 after 45000 ms. Firefox console output:
Error: cannot open display: :10

[merge]

@spadgett
Copy link
Member Author

spadgett commented Jul 5, 2017

Flake #1684

@openshift-bot
Copy link

Evaluated for origin web console merge up to 636745e

@openshift-bot
Copy link

openshift-bot commented Jul 5, 2017

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1618/) (Base Commit: 1c22134) (PR Branch Commit: 636745e)

@openshift-bot openshift-bot merged commit 5fdc228 into openshift:master Jul 5, 2017
@spadgett spadgett deleted the force-delete-pod branch July 5, 2017 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants