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

tetra: fix getevents cmd unexpected timeout #2765

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

mtardy
Copy link
Member

@mtardy mtardy commented Aug 2, 2024

Description

After commit 663a1e2, we stop connecting to the server prior making the RPC. It creates a buggy situation when we are using streams: the connection get cancelled at timeout even if it's successfully streaming. Let's remove the timeout for the getevents command and only use the context based on the interrupt signals.

Changelog

Fix a bug where the tetra getevents command would timeout even if the connection was successful.

@mtardy mtardy added the release-note/bug This PR fixes an issue in a previous release of Tetragon. label Aug 2, 2024
@mtardy mtardy requested a review from a team as a code owner August 2, 2024 18:05
@mtardy mtardy requested a review from willfindlay August 2, 2024 18:05
Copy link
Contributor

@willfindlay willfindlay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a minor nit

cmd/tetra/common/client.go Outdated Show resolved Hide resolved
After commit 663a1e2, we stop
connecting to the server prior making the RPC. It creates a buggy
situation when we are using streams: the connection get cancelled at
timeout even if it's successfully streaming. Let's remove the timeout
for the getevents command and only use the context based on the
interrupt signals.

Signed-off-by: Mahe Tardy <[email protected]>
@mtardy mtardy force-pushed the pr/mtardy/tetra-fix-getevents branch from 9a4fa71 to 5b47fe0 Compare August 12, 2024 09:09
Copy link

netlify bot commented Aug 12, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 5b47fe0
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/66b9d13787fd060008740cf7
😎 Deploy Preview https://deploy-preview-2765--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kkourt kkourt merged commit d6f55c3 into main Aug 12, 2024
44 checks passed
@kkourt kkourt deleted the pr/mtardy/tetra-fix-getevents branch August 12, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/bug This PR fixes an issue in a previous release of Tetragon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants