ROX-30732: Do not use globals in fake workloads by vikin91 · Pull Request #16629 · stackrox/stackrox

vikin91 · 2025-09-02T13:08:35Z

Description

Refactored the fake workload package to eliminate global variables and implement dependency injection for better testability and maintainability.

Key Changes:

Removed all global state: Moved labelsPool, processPool, ipPool, externalIpPool, containerPool, endpointPool, and registeredHostConnections from global variables into WorkloadManager instance fields
Added dependency injection: Extended WorkloadManagerConfig with builder pattern (With* methods) to configure pools during construction
Improved code organization: Extracted pool data structures and utility functions from flows.go to new pools.go file, reducing flows.go from 370→215 lines
Fixed coupling bug: Corrected pool.remove() method that was incorrectly accessing other global pools

Why: Global mutable state makes testing difficult, creates hidden dependencies between components, and violates Go best practices. This refactoring enables better unit testing, reduces coupling, and follows established dependency injection patterns used elsewhere in the codebase.

All functionality remains unchanged - this is purely an architectural improvement with no behavioral changes.

User-facing documentation

CHANGELOG.md is updated OR update is not needed
documentation PR is created and is linked above OR is not needed

Testing and quality

the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
CI results are inspected

Automated testing

How I validated my change

Added unit tests
Run on the long running cluster and looked at Sensor metrics to confirm known behavior
Being able to write tests for the code added in ROX-30860: Limit parallel port use by processes #16628

openshift-ci · 2025-09-02T13:08:40Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

vikin91 · 2025-09-02T13:08:41Z

This change is part of the following stack:

ROX-30732: Do not use globals in fake workloads #16629 ◀
- ROX-30860: Limit parallel port use by processes #16628

_{Change managed by git-spice.}

rhacs-bot · 2025-09-02T13:26:52Z

Images are ready for the commit at 6c08ff8.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.9.x-653-g6c08ff8bb9.

codecov · 2025-09-02T13:40:15Z

Codecov Report

❌ Patch coverage is 24.86188% with 136 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.61%. Comparing base (98c3b69) to head (5c376e9).
⚠️ Report is 24 commits behind head on master.

Files with missing lines	Patch %	Lines
sensor/kubernetes/fake/pools.go	41.17%	58 Missing and 2 partials ⚠️
sensor/kubernetes/fake/fake.go	0.00%	38 Missing ⚠️
sensor/kubernetes/fake/deployment.go	0.00%	17 Missing ⚠️
sensor/kubernetes/fake/flows.go	21.42%	11 Missing ⚠️
sensor/kubernetes/fake/service.go	0.00%	9 Missing ⚠️
sensor/kubernetes/fake/networkpolicy.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #16629      +/-   ##
==========================================
- Coverage   48.62%   48.61%   -0.02%     
==========================================
  Files        2664     2665       +1     
  Lines      199298   199338      +40     
==========================================
- Hits        96914    96907       -7     
- Misses      94792    94837      +45     
- Partials     7592     7594       +2

Flag	Coverage Δ
go-unit-tests	`48.61% <24.86%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sourcery-ai

Hey there - I've reviewed your changes and they look great!

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

sensor/kubernetes/fake/deployment.go

JoukoVirtanen · 2025-09-04T05:32:25Z

Please run at least a short scale test and check a few tables, API results, or the UI.

vikin91 · 2025-09-04T10:40:53Z

Please run at least a short scale test and check a few tables, API results, or the UI.

I have just started cluster pr-09-04-perfscale and kicked off the long-running workflow on it. I will let it run for few days and check the UI and metrics.

Edit after ~2hrs:

rhacs-bot · 2025-09-04T11:12:11Z

Images are ready for the commit at 5c376e9.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.9.x-689-g5c376e976d.

sensor/kubernetes/fake/fake.go

Changed: func (w *WorkloadManager) getServices(workload ServiceWorkload, ids []string) // was: (..., lblPool *labelsPoolPerNamespace) func (w *WorkloadManager) getNetworkPolicy(workload NetworkPolicyWorkload, id string) // was: (..., lblPool *labelsPoolPerNamespace) func (w *WorkloadManager) getDeployment(workload DeploymentWorkload, idx int, deploymentIDs, replicaSetIDs, podIDs []string) // was: (..., lblPool *labelsPoolPerNamespace) Kept: func getClusterIP(id string, lblPool *labelsPoolPerNamespace) func getNodePort(id string, lblPool *labelsPoolPerNamespace) func getLoadBalancer(id string, lblPool *labelsPoolPerNamespace) func getPod(replicaSet *appsv1.ReplicaSet, id string, ipPool *pool, containerPool *pool) func populatePodContainerStatuses(pod *corev1.Pod, containerPool *pool) func generateAndAddIPToPool(ipPool *pool) string

vikin91 · 2025-09-05T10:29:13Z

UI state after >24h on a long-running cluster:

JoukoVirtanen

LGTM!

openshift-ci bot added the do-not-merge/work-in-progress label Sep 2, 2025

vikin91 mentioned this pull request Sep 2, 2025

ROX-30860: Limit parallel port use by processes #16628

Merged

11 tasks

github-actions bot added area/sensor ai-review labels Sep 2, 2025

vikin91 changed the title ~~refactor(fake): Do not use globals~~ refactor(fake workloads): Do not use globals Sep 2, 2025

vikin91 added the ai-assisted label Sep 2, 2025

vikin91 marked this pull request as ready for review September 2, 2025 15:04

vikin91 requested a review from a team as a code owner September 2, 2025 15:04

openshift-ci bot removed the do-not-merge/work-in-progress label Sep 2, 2025

sourcery-ai bot reviewed Sep 2, 2025

View reviewed changes

vikin91 changed the title ~~refactor(fake workloads): Do not use globals~~ ROX-30732: Do not use globals in fake workloads Sep 2, 2025

vikin91 requested review from JoukoVirtanen, lvalerom and ovalenti September 2, 2025 15:15

vikin91 force-pushed the piotr/fake-workload-remove-globals branch from 502b8cf to 6c08ff8 Compare September 2, 2025 16:35

vikin91 mentioned this pull request Sep 3, 2025

ROX-30390: Add TransitionBased update computer #16522

Merged

10 tasks

JoukoVirtanen reviewed Sep 4, 2025

View reviewed changes

sensor/kubernetes/fake/deployment.go Outdated Show resolved Hide resolved

vikin91 force-pushed the piotr/fake-workload-remove-globals branch from 6c08ff8 to b75539a Compare September 4, 2025 11:07

vikin91 added the auto-retest PRs with this label will be automatically retested if prow checks fails label Sep 4, 2025

This comment was marked as off-topic.

Sign in to view

JoukoVirtanen reviewed Sep 4, 2025

View reviewed changes

sensor/kubernetes/fake/fake.go Outdated Show resolved Hide resolved

vikin91 added 2 commits September 5, 2025 12:21

Empty commit

19bd598

Avoid using globals

4c7f288

vikin91 added 4 commits September 5, 2025 12:21

Move all other globals to WorkloadManager

d5f6a8e

Refactor: Extract func removeContainerAndAssociatedObjects

01f8d8d

Remove duplicated func args from initializePreexistingResourcesWithDeps

240636c

vikin91 force-pushed the piotr/fake-workload-remove-globals branch from b75539a to 7e33a34 Compare September 5, 2025 10:22

Self-review

5c376e9

JoukoVirtanen approved these changes Sep 8, 2025

View reviewed changes

vikin91 merged commit ffd4ef5 into master Sep 9, 2025
91 of 92 checks passed

vikin91 deleted the piotr/fake-workload-remove-globals branch September 9, 2025 08:05

ksurabhi91 pushed a commit that referenced this pull request Sep 11, 2025

ROX-30732: Do not use globals in fake workloads (#16629)

f246219

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ROX-30732: Do not use globals in fake workloads#16629

ROX-30732: Do not use globals in fake workloads#16629
vikin91 merged 7 commits intomasterfrom
piotr/fake-workload-remove-globals

vikin91 commented Sep 2, 2025 •

edited

Loading

Uh oh!

openshift-ci bot commented Sep 2, 2025

Uh oh!

vikin91 commented Sep 2, 2025 •

edited

Loading

Uh oh!

rhacs-bot commented Sep 2, 2025 •

edited

Loading

Uh oh!

codecov bot commented Sep 2, 2025 •

edited

Loading

Uh oh!

sourcery-ai bot left a comment

Uh oh!

Uh oh!

JoukoVirtanen commented Sep 4, 2025

Uh oh!

vikin91 commented Sep 4, 2025 •

edited

Loading

Uh oh!

rhacs-bot commented Sep 4, 2025 •

edited

Loading

Uh oh!

This comment was marked as off-topic.

Uh oh!

vikin91 commented Sep 5, 2025

Uh oh!

JoukoVirtanen left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

vikin91 commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

User-facing documentation

Testing and quality

Automated testing

How I validated my change

Uh oh!

openshift-ci bot commented Sep 2, 2025

Uh oh!

vikin91 commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhacs-bot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JoukoVirtanen commented Sep 4, 2025

Uh oh!

vikin91 commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhacs-bot commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as off-topic.

Uh oh!

vikin91 commented Sep 5, 2025

Uh oh!

JoukoVirtanen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vikin91 commented Sep 2, 2025 •

edited

Loading

vikin91 commented Sep 2, 2025 •

edited

Loading

rhacs-bot commented Sep 2, 2025 •

edited

Loading

codecov bot commented Sep 2, 2025 •

edited

Loading

vikin91 commented Sep 4, 2025 •

edited

Loading

rhacs-bot commented Sep 4, 2025 •

edited

Loading