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

sdn: fix initialization order to prevent crash on node startup #13767

Merged

Conversation

dcbw
Copy link
Contributor

@dcbw dcbw commented Apr 14, 2017

OsdnNode.Start()
(node.pm == nil at this point)
-> node.policy.Start() (which is multitenant policy)
-> mp.vnids.Start()
-> go vmap.watchNetNamespaces()
-> (net namespace event happens)
-> watchNetNamespaces()
-> vmap.policy.AddNetNamespace() (policy is multitenant)
-> mp.updatePodNetwork()
-> mp.node.podManager.UpdateLocalMulticastRules() (and podManager is still nil)

Create the PodManager earlier so it's not nil if we get early events.

Fixes: #13742
(cherry picked from commit fc95b86)

@openshift/networking @smarterclayton

@dcbw dcbw force-pushed the 15-sdn-multicast-init-order branch from 84502cb to b60f14a Compare April 18, 2017 16:47
@smarterclayton
Copy link
Contributor

[test]

@smarterclayton
Copy link
Contributor

smarterclayton commented Apr 20, 2017 via email

@dcbw dcbw force-pushed the 15-sdn-multicast-init-order branch from b60f14a to 521c79f Compare April 20, 2017 16:52
@dcbw
Copy link
Contributor Author

dcbw commented Apr 20, 2017

(updated to match final code in #13766)

Copy link
Contributor

@danwinship danwinship left a comment

Choose a reason for hiding this comment

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

(updated to match final code in #13766)

I'd said there that I wasn't comfortable putting the kubelet init change into 1.5 without it getting testing in master first, so if we're making that change here, then it's not ready to land yet

OsdnNode.Start()
   (node.pm == nil at this point)
   -> node.policy.Start()  (which is multitenant policy)
   -> mp.vnids.Start()
   -> go vmap.watchNetNamespaces()
   -> (net namespace event happens)
   -> watchNetNamespaces()
   -> vmap.policy.AddNetNamespace() (policy is multitenant)
   -> mp.updatePodNetwork()
   -> mp.node.podManager.UpdateLocalMulticastRules() (and podManager is still nil)

Create the PodManager earlier so it's not nil if we get early events.

Fixes: openshift#13742
@dcbw dcbw force-pushed the 15-sdn-multicast-init-order branch from 521c79f to 030e3ec Compare April 20, 2017 20:48
@dcbw
Copy link
Contributor Author

dcbw commented Apr 20, 2017

@danwinship updated to move the kubelet init bits back.

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 030e3ec

@danwinship
Copy link
Contributor

[merge]

@eparis
Copy link
Member

eparis commented Apr 22, 2017

[merge] now that i think things are alivish

@eparis
Copy link
Member

eparis commented Apr 22, 2017

[merge]

Failed to connect to the host via ssh: Warning: Permanently added '172.18.1.62' (ECDSA) to the list of known hosts. Permission denied (publickey,gssapi-keyex,gssapi-with-mic).

fun times

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 030e3ec

@openshift-bot
Copy link
Contributor

openshift-bot commented Apr 23, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/880/) (Base Commit: 2aadab3) (Image: devenv-rhel7_6171)

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/880/) (Base Commit: 2aadab3)

@smarterclayton
Copy link
Contributor

smarterclayton commented Apr 23, 2017 via email

@openshift-bot openshift-bot merged commit 214e4c3 into openshift:release-1.5 Apr 24, 2017
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.

5 participants