-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[ON HOLD] *: update kube vendor to latest master #510
Conversation
@sameo CRI has removed netns path exposure from sandbox status, therefore network tests are now failing. |
f3a836b
to
bd9a088
Compare
@runcom I'll try to fix those in the next 2 days. I'm travelling this week, I'll keep you posted. |
@sameo no worry, I'll try to replace calls to netns exec with a simple container exec followed by an ip (or ifconfig) command to get container IP and verify tests. |
@sameo I fixed them, this should be all green now |
bd9a088
to
14f482a
Compare
|
||
check_pod_cidr $pod_id | ||
check_pod_cidr $ctr_id | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need a cleanup_ctrs
call here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
|
||
ping_pod $pod_id | ||
ping_pod $ctr_id | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
[ "$status" -eq 0 ] | ||
|
||
ping_pod_from_pod $pod2_id $pod1_id | ||
ping_pod_from_pod $ctr2_id $ctr1_id | ||
[ "$status" -eq 0 ] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@runcom besides the missing ctrs cleanups, LGTM. |
14f482a
to
73f661f
Compare
LGTM |
cmd/ocic/container.go
Outdated
@@ -10,10 +10,10 @@ import ( | |||
|
|||
"github.com/urfave/cli" | |||
"golang.org/x/net/context" | |||
remocommandconsts "k8s.io/apimachinery/pkg/util/remotecommand" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, and don't hold up the submit for this, but could you go back and change this variable name to "remotecommandconsts"? (Add 'te' after 'remo')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's copy-pasted from kube, let's follow up on this so I don't have to rebase #511 again :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nit, otherwise LGTM.
73f661f
to
09d34b7
Compare
retest |
all, this is rebased but I'd like to get #505 in firstly |
retest |
Needs rebase now. Let us get this one in. I want to get in one more update after this to enable sharing pid namespaces. |
09d34b7
to
c0b3f8c
Compare
rebased |
4e952d3
to
0b3d813
Compare
0/0 passed on RHEL - Passed. 0/0 passed on Fedora - Passed. 0/0 passed on CentOS - Passed. |
0b3d813
to
7dc8ea7
Compare
54/54 passed on RHEL - Passed. 54/54 passed on Fedora - Passed. 54/54 passed on CentOS - Passed. |
Yeah I will run them locally.
… On May 22, 2017, at 6:52 AM, Antonio Murdaca ***@***.***> wrote:
need to investigate why node-e2e aren't working with latest kube. Still testing for #533 so @mrunalp if you could run node-e2e with this PR we may move this forward sooner.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
note that it needs you to have go1.8.1+ |
dfc217a
to
d3763cd
Compare
26/47 passed on RHEL - Failed. 26/47 passed on Fedora - Failed. 26/47 passed on CentOS - Failed. |
needs rebase and fix |
We can get this in soon. I just want us to make sure that we manually test port forward, exec, attach etc. on this once. |
@mrunalp I'll run kube e2e on this branch before merging so we know port forward, exec, attach etc works fine, there are new CRI routes also in kube master, and we're behind right now |
63/63 passed on RHEL - Passed. 26/27 passed on Fedora - Failed. 63/63 passed on CentOS - Passed. |
oom test flaky, node-e2e is good here, need just rebase |
d3763cd
to
0c25357
Compare
function crioctl() { | ||
"$OCIC_BINARY" --connect "$CRIO_SOCKET" "$@" | ||
} | ||
|
||
# Run crictl using the binary specified by $CRICTL_BINARY. | ||
function crictl() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just as a note: We probably want the crictl integration to be part of its own commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, will take care of it later this week probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed now instead :)
08f5ea3
to
01ba8fc
Compare
63/63 passed on RHEL - Passed. 63/63 passed on Fedora - Passed. 63/63 passed on CentOS - Passed. |
01ba8fc
to
ae45c17
Compare
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
we need #564 before getting this in. |
ae45c17
to
7c51aef
Compare
26/46 passed on RHEL - Failed. 26/46 passed on Fedora - Failed. 26/46 passed on CentOS - Failed. |
@@ -193,6 +199,14 @@ | |||
chdir: /root/src/github.com/opencontainers/runc | |||
async: 600 | |||
poll: 10 | |||
- name: make crictl | |||
shell: | | |||
go install github.com/kubernetes-incubator/cri-tools/cmd/crictl && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is breaking - we need to stick version here :)
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
7c51aef
to
3ec20bd
Compare
0/0 passed on RHEL - Passed. 0/0 passed on Fedora - Passed. 0/0 passed on CentOS - Passed. |
I'll rework this to update master to |
@runcom Is this still being worked? |
Yes, kind of, it's next on my to-do list after the registry patch |
Closing in favor of #732 |
Close #509
Signed-off-by: Antonio Murdaca runcom@redhat.com