Kill docker daemon after configuring cbr0 #22293
Conversation
|
Labelling this PR as size/M |
k8s-github-robot
added
kind/api-change
|
GCE e2e build/test passed for commit 297a94c6db758b43fb5c6a069c9f3a15d52fb1ef. |
|
GCE e2e build/test passed for commit d32987bd7448a4e4679803025ecabd2d5c9cf04d. |
|
GCE e2e build/test passed for commit 7e48eae82437c0afa8539391e1bd05a5cb5605d0. |
dchen1107
force-pushed
the
test
branch
2 times, most recently
from
6d9c6ef to
f1d4662
Compare
|
kill -9 might have bad effects on plugins if they're relying on a signal from docker when it receives SIGTERM to save state to a checkpoint (that's stored somewhere other than /var/lib/docker/network, so we won't remove it). It's generally not a polite thing. I'm ok with this if the alternative is too complex ( |
|
GCE e2e build/test passed for commit 6d9c6ef35b7f740c3565985a2bed40e400ed35a3. |
|
GCE e2e build/test passed for commit f1d46627264848a91adf8ca4d8bf2193e0e7f636. |
|
Why you care about sigkill here? It should be issued only when node coming up without any kube container running since network configuration is not done, thus the node is not ready. Why you care about checkpoint files through SIGTERM? We are removing all checkpoints /var/lib/docker/network anyway, right? I can add more comments on why we want to kill, and please note that the newly introduced flag is marked as deprecated already. |
Not for all plugins. Not even for all network plugins, CNI won't store its checkpoint under /var/lib/docker.
SGTM. We decided not to do: #19854, so the CIDR can't change. Please add as comment. I'll apply lgtm after rebase and comment. |
|
@dchen1107 PR needs rebase |
k8s-github-robot
added
needs-rebase
…rue so that babysitter process can restart it again with proper configurations and checkpoint file.
|
Add more comments. Thanks! |
|
LGTM, thanks! |
bprashanth
added
lgtm
| @@ -164,6 +164,11 @@ | |||
| {% set hairpin_mode = "--hairpin-mode=" + pillar['hairpin_mode'] -%} | |||
| {% endif -%} | |||
|
|
|||
| {% set babysit_daemons = "" -%} | |||
| {% if grains['cloud'] is defined and grains.cloud in [ 'aws', 'gce' ] %} | |||
dchen1107
added
the
priority/important-soon
|
GCE e2e build/test passed for commit 960bea3. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit 960bea3. |
|
Automatic merge from submit-queue |
Auto commit by PR queue bot
Address #21523