Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
Skipping CI for Draft Pull Request. |
|
Images are ready for the commit at a63c2b3. To use with deploy scripts, first |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14904 +/- ##
=======================================
Coverage 48.96% 48.96%
=======================================
Files 2550 2550
Lines 187222 187225 +3
=======================================
+ Hits 91674 91683 +9
+ Misses 88299 88294 -5
+ Partials 7249 7248 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9aba1ec to
2f5c078
Compare
407376c to
6e4d36f
Compare
6e4d36f to
09f293e
Compare
Reviewer's Guide by SourceryThis pull request introduces a boolean option to control the generation of unclosed endpoints in fake workloads. This option, Sequence diagram for generating network connections with unclosed endpointssequenceDiagram
participant W as WorkloadManager
participant NW as NetworkWorkload
participant EP as EndpointPool
W->>NW: Get NetworkWorkload config
alt GenerateUnclosedEndpoints is true
W->>EP: Add networkEndpoint (unclosed)
EP-->>W: networkEndpoint
W->>W: Append networkEndpoint to networkEndpoints
else GenerateUnclosedEndpoints is false
W->>W: Skip adding unclosed endpoint
end
W-->>W: Return networkEndpoints
Updated class diagram for NetworkWorkloadclassDiagram
class NetworkWorkload {
FlowInterval time.Duration
BatchSize int
GenerateUnclosedEndpoints bool
}
note for NetworkWorkload "Added GenerateUnclosedEndpoints field"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @vikin91 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a comment to the
GenerateUnclosedEndpointsfield inNetworkWorkloadto explain its purpose. - Since the default value of
GenerateUnclosedEndpointsis true, consider explicitly setting it to true in the workload files where it's currently missing for clarity.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@sourcery-ai I actually did that in this PR. The comment is in the sample file and in all profiles the value is being set to true. |
|
/retest |
|
@vikin91: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Description
Generating unclosed endpoints in the fake-workload is meant to stress-load Central in a situation when many ports are opened for listening in the cluster. They are not closed on purpose to not trigger any cleanups.
The presence of the unclosed endpoints had actually stressed the memory of Sensor that lead to OOMs (ROX-28242). That load on Sensor was caused by "improper" handling of the endpoint data during enrichment of processes: some endpoints were never removed from the input to the enrichment loop on purpose, whereas others were kept there by accident. The fix for that is proposed in #14538.
In this PR, I add an option to disable the generation of unclosed endpoints in the fake workloads. This option is enabled by default (no change to the previous state), but it can be disabled for some workflows.
Disabling this option was helpful in investigating Sensor OOMs, so I think there is a value in persisting it.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
Summary by Sourcery
Add a configurable option to control the generation of unclosed network endpoints in fake workloads
New Features:
GenerateUnclosedEndpointsin network workload configuration to control the generation of unclosed network endpointsEnhancements:
Chores:
generateUnclosedEndpointsflag to true