-
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
sdn: fix single-tenant pod setup and leave docker0 around #11588
Conversation
LGTM but can you also fix the fact that setup() doesn't call getScriptError() on |
as usual I'm a little dense. Why does it matter if we store it as a string of it we store it as an int and then turn it into a string when we use it? |
@eparis I made that change so we don't have this problem again. The problem is that I forgot to initialize the string to "0". Now it's a uint32 and it will always be 0, even if we forget somewhere else :) |
Single-tenant needs to set vnid=0 for pod setup. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1388556
355c729
to
dca211b
Compare
@danwinship fixed. |
[merge] |
[Test]ing while waiting on the merge queue |
We no longer use docker for IPAM, so we don't need to mess with docker networking at all. Don't kill docker0 anymore on openshift-sdn startup.
[merge] |
[test] was a flake/bug #11560 |
flake was #11560 |
Evaluated for origin test up to 2000b4b |
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10692/) (Base Commit: 8bb33be) |
Evaluated for origin merge up to 2000b4b |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10708/) (Base Commit: a553668) (Image: devenv-rhel7_5252) |
Single-tenant needs to set vnid=0 for pod setup.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1388556
@eparis @openshift/networking