Bug 1389213 - Fix join/isolate project network #11679
Conversation
|
@openshift/networking PTAL |
| @@ -33,7 +33,6 @@ const ( | |||
| sdnScript = "openshift-sdn-ovs" | |||
| setUpCmd = "setup" | |||
| tearDownCmd = "teardown" | |||
| statusCmd = "status" | |||
There was a problem hiding this comment.
Does this omission have anything to do with the bz? Or is it just a cleanup.
There was a problem hiding this comment.
Nope, minor cleanup. Since this cleanup is only couple of lines, I have created a separate commit instead of new pr to avoid additional test/merge cycle.
There was a problem hiding this comment.
LGTM, with the caveat that @rajatchopra's question needs an answer.
|
[test] |
|
LGTM as is though you could also change UpdatePod() to just take a *kapi.Pod, and inline getPodContainerID() into it. |
|
Failing because router doesn't have permission to list services. Bug 1380669 link |
Pass kubeletTypes.ContainerID.ID instead of kubeletTypes.ContainerID.String() to UpdatePod(), Otherwise docker client fails with error: no such container '://<id>'
Remove unused statusCmd variable.
pravisankar
force-pushed
the
fix-broken-join-network
branch
from
7876d8b to
bca9fd2
Compare
|
Evaluated for origin test up to bca9fd2 |
|
Updated, Now UpdatePod() takes kapi.Pod arg and getPodContainerID is called inside the method. cc: @danwinship |
|
LGTM again. @danwinship please merge if you are good with it. |
|
[merge] |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10947/) (Base Commit: f55922e) |
|
@knobunc openshift-bot only has eyes for you |
| ) | ||
|
|
||
| func getPodContainerID(pod *kapi.Pod) string { | ||
| if len(pod.Status.ContainerStatuses) > 0 { | ||
| // Extract only container ID, pod.Status.ContainerStatuses[0].ContainerID is of the format: docker://<containerID> |
|
LGTM |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10947/) (Image: devenv-rhel7_5305) |
|
Evaluated for origin merge up to bca9fd2 |
Pass kubeletTypes.ContainerID.ID instead of kubeletTypes.ContainerID.String() to UpdatePod(),
Otherwise docker client fails with error: no such container '://'
Fixes bug 1389213