desiring LRPs with empty lists of ports causes OPI panics

cwlbraa commented 4 years ago

Description

Originally reported by as https://github.com/cloudfoundry/cf-for-k8s/issues/287, pushing the spring-music sample app causes OPI to panic when desiring the "task" process. In the issue, @cselzo investigated and found that this was due to the app having 2 processes that don't have any open ports. Neither of those processes intend to be externally accessible, so we think this should be a valid LRP to desire.

Steps to reproduce

Push the spring-music sample app using the v7 CLI and cf-for-k8s. The v7 CLI uses CAPI's /v3/apps/:guid/restart endpoint, making the failure easier to trace as it bubbles up a errors when talking to OPI.

What was expected to happen

Successfully create a StatefulSet with no open ports.

What actually happened

 {"timestamp":"2020-07-22T14:38:13.213363528Z","level":"debug","source":"handler","message":"handler.desire-app.requested","data":{"app_guid":"e9ae3554-fb1c-40cb-b310-464124ad2a95"
 ,"guid":"dc3121ef-0c16-433e-be4e-12ee667cc24c-f2ad251f-541c-45d0-a728-075157e2ea0e","session":"51798","version":"f2ad251f-541c-45d0-a728-075157e2ea0e"}}
 2020/07/22 14:38:13 http: panic serving 127.0.0.1:35244: runtime error: index out of range [0] with length 0
 goroutine 119659 [running]:
 net/http.(*conn).serve.func1(0xc000352c80)
     net/http/server.go:1767 +0x139
 panic(0x181c8e0, 0xc00076d980)
     runtime/panic.go:679 +0x1b2
 code.cloudfoundry.org/eirini/bifrost.(*OPIConverter).ConvertLRP(0xc00048a080, 0xc00074eb70, 0x24, 0xc00074eba0, 0x24, 0xc00003e690, 0x49, 0xc0002c1990, 0x4, 0xc00074ec00, ...)
     code.cloudfoundry.org/eirini@/bifrost/convert.go:50 +0x1120
 code.cloudfoundry.org/eirini/bifrost.(*LRP).Transfer(0xc0005b3d10, 0x1b9eae0, 0xc0008b8180, 0xc00074eb70, 0x24, 0xc00074eba0, 0x24, 0xc00003e690, 0x49, 0xc0002c1990, ...)
     code.cloudfoundry.org/eirini@/bifrost/lrp.go:36 +0xc0
 code.cloudfoundry.org/eirini/handler.(*App).Desire(0xc00000d380, 0x1b98da0, 0xc00047a620, 0xc0000dd200, 0xc000594960, 0x1, 0x3)
     code.cloudfoundry.org/eirini@/handler/app_handler.go:45 +0x55b
 github.com/julienschmidt/httprouter.(*Router).ServeHTTP(0xc0003484e0, 0x1b98da0, 0xc00047a620, 0xc0000dd200)
     github.com/julienschmidt/httprouter@v1.3.0/router.go:387 +0xa3d
 net/http.serverHandler.ServeHTTP(0xc0005b4ee0, 0x1b98da0, 0xc00047a620, 0xc0000dd200)
     net/http/server.go:2802 +0xa4
 net/http.(*conn).serve(0xc000352c80, 0x1b9eae0, 0xc00041dac0)
     net/http/server.go:1890 +0x875
 created by net/http.(*Server).Serve
     net/http/server.go:2928 +0x384
 2020/07/22 14:38:13 http: panic serving 127.0.0.1:35246: runtime error: index out of range [0] with length 0
 goroutine 119664 [running]:
 net/http.(*conn).serve.func1(0xc0002921e0)
     net/http/server.go:1767 +0x139
 panic(0x181c8e0, 0xc0005bc560)
     runtime/panic.go:679 +0x1b2
 code.cloudfoundry.org/eirini/bifrost.(*OPIConverter).ConvertLRP(0xc00048a080, 0xc00074f200, 0x24, 0xc00074f230, 0x24, 0xc00003edc0, 0x49, 0xc0007b4378, 0x4, 0xc00074f260, ...)
     code.cloudfoundry.org/eirini@/bifrost/convert.go:50 +0x1120
 code.cloudfoundry.org/eirini/bifrost.(*LRP).Transfer(0xc0005b3d10, 0x1b9eae0, 0xc0008b8480, 0xc00074f200, 0x24, 0xc00074f230, 0x24, 0xc00003edc0, 0x49, 0xc0007b4378, ...)
     code.cloudfoundry.org/eirini@/bifrost/lrp.go:36 +0xc0
 code.cloudfoundry.org/eirini/handler.(*App).Desire(0xc00000d380, 0x1b98da0, 0xc00047ab60, 0xc00095f200, 0xc000595a40, 0x1, 0x3)
     code.cloudfoundry.org/eirini@/handler/app_handler.go:45 +0x55b
 github.com/julienschmidt/httprouter.(*Router).ServeHTTP(0xc0003484e0, 0x1b98da0, 0xc00047ab60, 0xc00095f200)
     github.com/julienschmidt/httprouter@v1.3.0/router.go:387 +0xa3d
 net/http.serverHandler.ServeHTTP(0xc0005b4ee0, 0x1b98da0, 0xc00047ab60, 0xc00095f200)
     net/http/server.go:2802 +0xa4
 net/http.(*conn).serve(0xc0002921e0, 0x1b9eae0, 0xc0008b83c0)
     net/http/server.go:1890 +0x875
 created by net/http.(*Server).Serve
     net/http/server.go:2928 +0x384

cf-gitbot commented 4 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/173954689

The labels on this github issue will be updated when the story is started.

danail-branekov commented 4 years ago

Hey @cwlbraa

It would be pretty easy for us to check for empty ports array in the panicing converter.

However, is my understanding that cf7 supports multi-processes apps via requesting multiple LRPs for a single app (one per process) correct? The reason to ask this question is that we have a story to natively support apps with multiple processes using K8S pods sidecars but if there is no demand, we should probably reconsider?

Also, why is the failure not reproducible with cf6? Does cf6 not request multiple LRPs?

cwlbraa commented 4 years ago

It's likely easier to see the failure with cf7 because the v3 restart endpoint errors clearly, but I also didn't personally test with cf6. I'd bet it falls, but does so more quietly.

To your other question, which seems a bit orthogonal: the CF v3 API supports multiprocess apps AND sidecar processes. In the CF API a v3 multiprocess app should make multiple pods, but a v3/process with sidecars would need additional containers inside the v3/process pod. If eirini allowed extra containers in a single LRP we'd use them to implement sidecars.

emalm commented 4 years ago

FYI, this issue should be closed out, as the related https://github.com/cloudfoundry/cf-for-k8s/issues/287 and https://github.com/cloudfoundry/capi-k8s-release/issues/55 issues are as well.

cloudfoundry / eirini