Skip to content

Conversation

tksm
Copy link
Contributor

@tksm tksm commented Feb 7, 2023

Fixes #73

This PR adds the --max-log-requests flag to limit concurrent requests to prevent unintentional load to a cluster.

The behavior and the default are different depending on the presence of the --no-follow flag.

--no-follow default behavior
specified 5 limits the number of concurrent logs to request
not specified 50 exits with an error when if it reaches the concurrent limit

when --no-follow is specified

It limits the number of concurrent logs to request, so there is no apparent change.
I chose 5 for the default limit, the same as in kubectl.

📝 I like the combination of --max-log-requests 1 and --no-follow, which will show logs in order.

when --no-follow is not specified

It exits with an error when if it reaches the concurrent limit.
Though I am not sure how we should choose the default limit, I think 50 is enough for most cases.

$ dist/stern --max-log-requests 3 deployment/nginx
+ nginx-76d6c9b8c-77s2r › nginx
+ nginx-76d6c9b8c-cg85s › nginx
+ nginx-76d6c9b8c-lxpmt › nginx
Error: stern reached the maximum number of log requests (3), use --max-log-requests to increase the limit

Copy link
Member

@rkmathi rkmathi left a comment

Choose a reason for hiding this comment

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

LGTM 😸

@rkmathi rkmathi merged commit 0b939c5 into stern:master Feb 7, 2023
@tksm tksm deleted the max-log-requests branch February 7, 2023 23:24
@felipecrs
Copy link
Contributor

How to remove the limit, -1?

@tksm
Copy link
Contributor Author

tksm commented Mar 30, 2023

@felipecrs Currently, stern does not support removing the requests limit. You need to specify a big enough number of the limit, like --max-log-requests 100000, if you want to avoid the limit.

It is a bit bothersome. #252 might help to configure the default of the request limit in the future.

@felipecrs
Copy link
Contributor

Got it. That's fine, thank you!

@superbrothers
Copy link
Member

@felipecrs We've released a new version that includes support for config file. Please try it out.

$ mkdir -p ~/.config/stern
$ cat <<EOL
max-log-requests: 100000
EOL
$ stern .

@felipecrs
Copy link
Contributor

Thanks a lot! That makes it a lot easier than adding the flag every time, but since I use stern in a script, hardcoding the flag isn't much of a deal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove support for --all-namespaces and add support for multiple --namespace
4 participants