Do not return error if EndpointSlicesServiceKey func errors #1733
Conversation
k8s-ci-robot
added
do-not-merge/invalid-commit-message
* Service key may not be possible if EndpointSlice does not have all the expected labels set, namely the service label. This is a valid EPS and should be ignored when indexing. Returning an error causes the store to panic. Instead return empty list of keys and no error, so that the EPS is ignored.
k8s-ci-robot
added
the
approved
swetharepakula
force-pushed
the
fix-eps-indexer-panic
branch
from
a6ff573 to
41aa75c
Compare
k8s-ci-robot
removed
the
do-not-merge/invalid-commit-message
|
/assign @bowei |
|
/assign @cezarygerard |
|
It's strange that we are allowed to return an error given that it leads to a panic() in all cases? |
|
I haven't found documentation on how the error should be used for IndexFunc, but an error from it will always cause the thread safe store to panic. |
|
can you point where this crash happend? |
|
The panic happens in client-go in the store implementation used for all of the resources we watch. For reference: https://github.com/kubernetes/client-go/blob/master/tools/cache/thread_safe_store.go#L264-L275. For EndpointSlice we use our own indexer which triggers the panic. The issue #1730 mentions that this occurs in GKE and is reproducible when running the cluster manually. I have manually testing with a local instance of ingress-gce that it panics without this change when it encounters an EndpointSlice without the service label and that this fix causes the controller to properly ignore that EndpointSlice. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kl52752, swetharepakula The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[Cherrypick #1733 into release-1.15] Do not return error if EndpointSlicesServiceKey func errors
[Cherrypick #1733 into release-1.14] Do not return error if EndpointSlicesServiceKey func errors
[Cherrypick #1733 into release-1.16] Do not return error if EndpointSlicesServiceKey func errors
[Cherrypick #1733 in release-1.17] Do not return error if EndpointSlicesServiceKey func errors
the expected labels set, namely the service label. This is a valid
EPS and should be ignored when indexing. Returning an error causes
the store to panic. Instead return empty list of keys and no error,
so that the EPS is ignored.
Fixes #1730