hubble: Never fail with ErrInvalidRead

[michi-covalent]

Currently GetFlows() fails with the following error when a position in
the ring buffer being read by Ring.read() has been overwritten:

requested data has been overwritten and is no longer available

This turned out to be impractical as it makes it difficult to read all
the flows in the ring buffer (e.g.. hubble observe --all). GetFlows()
would fail if Hubble observes a single flow between the reader rewinding
to the oldest position and retrieving the entry.

This patch modifies Ring.read() so that GetFlows() returns LostEvent
instead of stopping with an error. The caller of GetFlows() can then
decide how to handle LostEvent.

Note that this makes the behavior of Ring.read() consistent with that
of Ring.readFrom() used in the follow mode. It generates LostEvent and
continues following instead of failing with ErrInvalidRead.

Fixes: #17036

Signed-off-by: Michi Mutsuzaki michi@isovalent.com

[michi-covalent]

in addition to unit tests, manually validated the fix with hubble observe --all --debug:

# hubble observe --all --debug | head
time="2021-08-03T23:49:59Z" level=debug msg="Sending GetFlows request" request="number:18446744073709551615"
unknown response type: &{LostEvents:source:HUBBLE_RING_BUFFER  num_events_lost:1}
Aug  3 23:47:12.074: default/pod-to-a-9dc6d768c-rc7nc:49527 <- kube-system/kube-dns-6c7b8dc9f9-x962b:53 to-endpoint FORWARDED (UDP)
Aug  3 23:47:12.074: default/pod-to-a-9dc6d768c-rc7nc:34082 -> default/echo-a-56f9849b6b-jlbpr:8080 to-stack FORWARDED (TCP Flags: SYN)
Aug  3 23:47:12.074: default/pod-to-a-9dc6d768c-rc7nc:34082 -> default/echo-a-56f9849b6b-jlbpr:8080 to-endpoint FORWARDED (TCP Flags: SYN)
Aug  3 23:47:12.074: default/pod-to-a-9dc6d768c-rc7nc:34082 <- default/echo-a-56f9849b6b-jlbpr:8080 to-stack FORWARDED (TCP Flags: SYN, ACK)
Aug  3 23:47:12.074: default/pod-to-a-9dc6d768c-rc7nc:34082 <- default/echo-a-56f9849b6b-jlbpr:8080 to-endpoint FORWARDED (TCP Flags: SYN, ACK)
Aug  3 23:47:12.074: default/pod-to-a-9dc6d768c-rc7nc:34082 -> default/echo-a-56f9849b6b-jlbpr:8080 to-stack FORWARDED (TCP Flags: ACK)
Aug  3 23:47:12.074: default/pod-to-a-9dc6d768c-rc7nc:34082 -> default/echo-a-56f9849b6b-jlbpr:8080 to-endpoint FORWARDED (TCP Flags: ACK)
Aug  3 23:47:12.074: default/pod-to-a-9dc6d768c-rc7nc:34082 -> default/echo-a-56f9849b6b-jlbpr:8080 to-stack FORWARDED (TCP Flags: ACK, PSH)
Aug  3 23:47:12.074: default/pod-to-a-9dc6d768c-rc7nc:34082 -> default/echo-a-56f9849b6b-jlbpr:8080 to-endpoint FORWARDED (TCP Flags: ACK, PSH)

[gandro]

Thanks for doing this! I left an inline comment

[gandro]

Nice work! Should we mark this for backport too?

[michi-covalent]

test-me-please

[michi-covalent]

restarting these 2:

aks: https://github.com/cilium/cilium/actions/runs/1101961941
eks: https://github.com/cilium/cilium/actions/runs/1101961955

Error : .github#L1
An unexpected error has occurred and we've been automatically notified. Errors are sometimes temporary, so please try again.

[michi-covalent]

ci-aks

[michi-covalent]

ci-eks

[michi-covalent]

https://github.com/cilium/cilium/actions/runs/1102872491

legitimate failures 😐

Error: Connectivity test failed: 2 tests failed
📋 Test Report
❌ 2/9 tests failed (2/81 actions), 0 tests skipped, 0 scenarios skipped:
Test [client-ingress]:
  ❌ client-ingress/client-to-client/ping-1: cilium-test/client2-666976c95b-fpfxz (10.240.0.42) -> cilium-test/client2-666976c95b-fpfxz (10.240.0.42:0)
Test [client-egress]:
  ❌ client-egress/pod-to-pod/curl-0: cilium-test/client2-666976c95b-fpfxz (10.240.0.42) -> cilium-test/echo-other-node-697d5d69b7-jk4kp (10.240.0.49:8080)

[gandro]

Looking at the failure, it is not related to the PR, it's a connectivity problem, not a problem with flow visibility (i.e. Hubble). Given that more and more users are hitting the error that this PR is addressing, I'll mark this ready-to-merge.