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

Add max log requests for stern tailer #522

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

warango4
Copy link
Contributor

Pull request

What this PR does / why we need it

With Stern new version (v1.24.0), a new flag was introduced to decide which is the maximum number of concurrent requests. In Apps Plugin code, it never gets to have a default number other than 0, that's why, if not specified in Stern configuration, logs would never appear since there weren't any allowed logs to show.
A number of the maximum Int16 (32767) was set as the max log requests for Stern, which means there can be up to that amount of pods logging at the same time for Apps Plugin workload tail or workload create/update/apply --tail to work.

Another change introduced in Stern was the ALL_STATES option for containers state filter. It was also added in Apps Plugin so all containers are shown (running,waiting,terminated).

Which issue(s) this PR fixes

Fixes #519

Describe testing done for PR

  • Created different workloads with --tail option to check that is logging.

Additional information or special notes for your reviewer

@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2023

Codecov Report

Merging #522 (9b3e44e) into main (336df50) will decrease coverage by 0.32%.
The diff coverage is 4.16%.

@@            Coverage Diff             @@
##             main     #522      +/-   ##
==========================================
- Coverage   86.27%   85.95%   -0.32%     
==========================================
  Files          65       65              
  Lines        4109     4123      +14     
==========================================
- Hits         3545     3544       -1     
- Misses        438      453      +15     
  Partials      126      126              
Impacted Files Coverage Δ
pkg/cli-runtime/client.go 66.20% <0.00%> (ø)
pkg/cli-runtime/flags.go 73.80% <0.00%> (ø)
pkg/commands/workload.go 87.89% <0.00%> (-2.16%) ⬇️
pkg/commands/clustersupplychain_list.go 92.10% <100.00%> (-0.11%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

With Stern new version (v1.24.0), a new flag was introduced to
decide which is the maximum number of concurrent requests.
In Apps Plugin code, it never gets to have a default number other than 0,
that's why, if not specified in Stern configuration, logs would never appear
since there were not available requests to do.
A number of the maximum Int16 (32767) was set as the max log requests
for Stern, which means there can be up to that amount of pods logging at the
same time for Apps Plugin `workload tail` or
`workload create/update/apply --tail` to work.

Another change introduced in Stern was the `ALL_STATES` option for containers
state filter. It was also added in Apps Plugin so all containers are shown
(`running,waiting,terminated`).

Signed-off-by: Wendy Arango <warango@vmware.com>
Copy link
Contributor

@shaheerkootteeri shaheerkootteeri left a comment

Choose a reason for hiding this comment

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

LGTM

@warango4 warango4 merged commit 838fd42 into vmware-tanzu:main Apr 3, 2023
@warango4 warango4 deleted the issue519 branch April 3, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stern latest version (v1.24.0) introduced bugs in Apps Plugin
5 participants