deploy: set gracePeriodSeconds on deployer deletion #9802
Conversation
|
[test] |
|
I don't know that this is what you want. How is this helping? You're not giving the deployer a chance to signal other items. |
|
A shorter grace period is probably ok... but I'd say 10-15s, not the "insta kill". |
What do you mean by "signal other items"? Isn't the delete event handler for pods going to work (and signal the deployment to be reconciled again) ? |
|
first run is clean, [test] |
|
If this is fixing your problem then you are probably hitting another On Tue, Jul 12, 2016 at 12:33 PM, Michail Kargakis <[email protected]
|
|
All this is probably doing is avoiding whatever race is in the deployment On Tue, Jul 12, 2016 at 4:25 PM, OpenShift Bot [email protected]
|
|
I can't find anything suspicious in the deployment controller. Note that the test uses a very ambitious timeout of 10 seconds. Updating both the timeout and gracePeriodSeconds of the deployer pod |
|
Evaluated for origin test up to c366fd3 |
|
The 10s test timeout was probably too much, agree. |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6196/) |
|
Assuming LGTY [merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6196/) (Image: devenv-rhel7_4581) |
|
Evaluated for origin merge up to c366fd3 |
| for _, deployerPod := range deployerList.Items { | ||
| if err := c.pn.Pods(deployerPod.Namespace).Delete(deployerPod.Name, &kapi.DeleteOptions{}); err != nil && !kerrors.IsNotFound(err) { | ||
| if err := c.pn.Pods(deployerPod.Namespace).Delete(deployerPod.Name, &kapi.DeleteOptions{GracePeriodSeconds: &gracePeriod}); err != nil && !kerrors.IsNotFound(err) { |
There was a problem hiding this comment.
Actually, the correct place to put this is where you create the deployer pod, not here.
Ref #9681