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: fix PortForward panic #542

Merged
merged 1 commit into from
May 28, 2017

Conversation

runcom
Copy link
Member

@runcom runcom commented May 28, 2017

During "Port forwarding" e2e tests, the following panic happened:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x64981d]

goroutine 52788 [running]:
panic(0x1830ee0, 0xc4200100c0)
        /usr/lib/golang/src/runtime/panic.go:500 +0x1a1
github.com/kubernetes-incubator/cri-o/oci.(*Runtime).UpdateStatus(0xc4202afc00,
0x0, 0x0, 0x0)
        /home/amurdaca/go/src/github.com/kubernetes-incubator/cri-o/oci/oci.go:549
+0x7d
github.com/kubernetes-incubator/cri-o/server.streamService.PortForward(0xc42026e000,
0x0, 0x0, 0x0, 0x0, 0xc420d9af40, 0x40, 0xc400000050, 0x7fe660659a28,
0xc4201cd0e0, ...)

The issue is streamService.PortForward assumed the first argument to
be the sandbox's infra container ID, thus trying to get it from memory
store using .state.containers.Get. Since that ID is of the sandbox
itself, it fails to get the container object from memory and panics in
UpdateStatus.

Fix it by looking for the sandbox's infra container ID starting from a
sandbox ID.

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

@runcom
Copy link
Member Author

runcom commented May 28, 2017

@mrunalp PTAL

@runcom runcom mentioned this pull request May 28, 2017
@runcom
Copy link
Member Author

runcom commented May 28, 2017

You can see here the first argument is the sandbox ID, not the infraContainer ID, https://github.com/kubernetes/kubernetes/pull/35661/files#diff-16c44fd3516b9794e49c760f0af2d777R81

@mrunalp
Copy link
Member

mrunalp commented May 28, 2017 via email

During "Port forwarding" e2e tests, the following panic happened:

```
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x64981d]

goroutine 52788 [running]:
panic(0x1830ee0, 0xc4200100c0)
        /usr/lib/golang/src/runtime/panic.go:500 +0x1a1
github.com/kubernetes-incubator/cri-o/oci.(*Runtime).UpdateStatus(0xc4202afc00,
0x0, 0x0, 0x0)
        /home/amurdaca/go/src/github.com/kubernetes-incubator/cri-o/oci/oci.go:549
+0x7d
github.com/kubernetes-incubator/cri-o/server.streamService.PortForward(0xc42026e000,
0x0, 0x0, 0x0, 0x0, 0xc420d9af40, 0x40, 0xc400000050, 0x7fe660659a28,
0xc4201cd0e0, ...)
```

The issue is `streamService.PortForward` assumed the first argument to
be the sandbox's infra container ID, thus trying to get it from memory
store using `.state.containers.Get`. Since that ID is of the sandbox
itself, it fails to get the container object from memory and panics in
`UpdateStatus`.

Fix it by looking for the sandbox's infra container ID starting from a
sandbox ID.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@rhatdan
Copy link
Contributor

rhatdan commented May 28, 2017

LGTM

@runcom runcom changed the title server: fix PortForward panic [e2e fix] server: fix PortForward panic May 28, 2017
@rh-atomic-bot
Copy link

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

23/24 passed on Fedora - Failed.
120/121 k8s node-e2e passed on Fedora - Failed.

60/60 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/366/fullresults.txt

@runcom
Copy link
Member Author

runcom commented May 28, 2017

all green except oom test

@mrunalp mrunalp merged commit 988f29f into cri-o:master May 28, 2017
@runcom runcom deleted the port-forward-panic branch May 28, 2017 18:42
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

5 participants