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

[e2e fix] server: correctly fill ctr termination reason #543

Merged
merged 2 commits into from
May 29, 2017

Conversation

runcom
Copy link
Member

@runcom runcom commented May 28, 2017

This patch fixes all port forwarding e2e tests. Those tests were
specifically looking for a termination reason to say that a given
container has finished running. CRI-O wasn't actually returning any
reason field for an exited container.

-> https://github.com/kubernetes/kubernetes/blob/master/test/e2e/portforward.go#L116
-> https://github.com/kubernetes/kubernetes/blob/master/test/utils/conditions.go#L97

Signed-off-by: Antonio Murdaca runcom@redhat.com

This patch fixes all port forwarding e2e tests. Those tests were
specifically looking for a termination reason to say that a given
container has finished running. CRI-O wasn't actually returning any
reason field for an exited container.

-> https://github.com/kubernetes/kubernetes/blob/master/test/e2e/portforward.go#L116
   -> https://github.com/kubernetes/kubernetes/blob/master/test/utils/conditions.go#L97

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@runcom runcom changed the title server: correctly fill ctr termination reason [e2e fix] server: correctly fill ctr termination reason May 28, 2017
`containerdID` is overridden in `s.ctrIDIndex.Get()`, if the ctr is not
found it's overridden by an empty string making the error return
totally unusable.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@rh-atomic-bot
Copy link

63/63 passed on RHEL - Passed.
0/0 k8s node-e2e passed on RHEL - Failed.

63/63 passed on Fedora - Passed.
120/121 k8s node-e2e passed on Fedora - Failed.

63/63 passed on CentOS - Passed.
0/0 k8s node-e2e passed on CentOS - Failed.

Log - https://aos-ci.s3.amazonaws.com/kubernetes-incubator/cri-o/crio-integration-tests-prs/369/fullresults.txt

@runcom
Copy link
Member Author

runcom commented May 28, 2017

All green here as well

@rh-atomic-bot
Copy link

63/63 passed on RHEL - Passed.
0/0 k8s node-e2e passed on RHEL - Failed.

26/27 passed on Fedora - Failed.
120/121 k8s node-e2e passed on Fedora - Failed.

63/63 passed on CentOS - Passed.
0/0 k8s node-e2e passed on CentOS - Failed.

Log - https://aos-ci.s3.amazonaws.com/kubernetes-incubator/cri-o/crio-integration-tests-prs/370/fullresults.txt

@mrunalp
Copy link
Member

mrunalp commented May 29, 2017

Was planning to do this :)
LGTM

@mrunalp mrunalp merged commit 5ed79fb into cri-o:master May 29, 2017
@runcom runcom deleted the fix-ctr-status-reasons branch May 29, 2017 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants