Skip to content

WIP: ROX-33958: BusyBox-style binary consolidation for main image#19819

Open
janisz wants to merge 11 commits intomasterfrom
ROX-33958/resue-components
Open

WIP: ROX-33958: BusyBox-style binary consolidation for main image#19819
janisz wants to merge 11 commits intomasterfrom
ROX-33958/resue-components

Conversation

@janisz
Copy link
Copy Markdown
Contributor

@janisz janisz commented Apr 3, 2026

Description

Consolidate 8 separate binaries into a single binary using BusyBox-style dispatch pattern to reduce image size.

  • Before: 1.93 GB
  • After: 1.22 GB

User-facing documentation

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

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

change me!

Consolidate 8 separate binaries into a single binary using BusyBox-style
dispatch pattern to reduce image size by ~54-64% (from ~1.1GB to ~400-500MB).

**Changes:**
- Refactor each component to use app package pattern:
  - migrator/app, compliance/cmd/compliance/app
  - sensor/admission-control/app, sensor/kubernetes/app
  - sensor/upgrader/app, config-controller/app
  - compliance/virtualmachines/roxagent/app
- Add build tags (//go:build !centralall) to component main.go files
- Update central/main.go with BusyBox dispatcher and app package imports
- Modify Makefile to build only central binary with centralall tag
- Update Dockerfile to create symlinks instead of copying separate binaries

**Implementation:**
Each component now has:
1. app/app.go - Contains Run() function with main logic
2. main.go - Thin wrapper that calls app.Run() (excluded with centralall tag)

central/main.go dispatcher checks os.Args[0] and routes to appropriate app.Run().

**Testing:**
All refactored components validated with gopls - no diagnostics.
Individual components still build independently without centralall tag.

**Benefits:**
- 54-64% image size reduction
- Better code organization (app logic separate from entry point)
- Improved testability (app.Run() can be tested directly)
- No code duplication
- Minimal changes to existing code

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>

Remove unnecessary build tags from BusyBox consolidation

The //go:build !centralall tags were not needed because Go's package system
naturally handles the separation:
- Building ./central only compiles central package + its dependencies (app packages)
- Component main.go files are in separate packages and won't be included
- Simpler implementation without conditional compilation

This makes the code cleaner and easier to understand.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>

Update Konflux Dockerfile for BusyBox consolidation

Apply the same BusyBox-style consolidation to the Konflux build:
- Copy only the central binary instead of 8 separate binaries
- Create symlinks for all component entry points
- Matches changes made to image/rhel/Dockerfile

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@janisz janisz requested review from a team and rhacs-bot as code owners April 3, 2026 12:19
@rhacs-bot rhacs-bot requested a review from a team April 3, 2026 12:19
@janisz janisz changed the title ROX-33958: Implement BusyBox-style binary consolidation for main image WIP: ROX-33958: Implement BusyBox-style binary consolidation for main image Apr 3, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 2 issues, and left some high level feedback:

  • Importing the new app packages into central causes their init functions (e.g., memlimit.SetMemoryLimit in admission-control, kubernetes sensor, and compliance) to run even when the central mode is selected; consider moving these side-effectful calls from init() into the respective Run() functions so central's behavior and limits are not unintentionally altered.
  • The BusyBox-style dispatcher in central defaults to running Main() for any unexpected binary name; consider logging a warning or handling unknown names explicitly so misconfigured symlinks or typos don't silently fall back to central.
  • In the Dockerfiles you create multiple symlinks for the same logical tools (e.g., admission-control, kubernetes-sensor, config-controller both under /stackrox/bin and directly under /stackrox); it would be helpful to either minimize duplication or add a brief comment clarifying which paths are required for backward compatibility versus new usage.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Importing the new app packages into central causes their init functions (e.g., memlimit.SetMemoryLimit in admission-control, kubernetes sensor, and compliance) to run even when the central mode is selected; consider moving these side-effectful calls from init() into the respective Run() functions so central's behavior and limits are not unintentionally altered.
- The BusyBox-style dispatcher in central defaults to running Main() for any unexpected binary name; consider logging a warning or handling unknown names explicitly so misconfigured symlinks or typos don't silently fall back to central.
- In the Dockerfiles you create multiple symlinks for the same logical tools (e.g., admission-control, kubernetes-sensor, config-controller both under /stackrox/bin and directly under /stackrox); it would be helpful to either minimize duplication or add a brief comment clarifying which paths are required for backward compatibility versus new usage.

## Individual Comments

### Comment 1
<location path="central/main.go" line_range="288-295" />
<code_context>
 	log.Info("Central terminated")
 }

-func main() {
+// Main is the exported entry point for the central binary.
+func Main() {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider logging when an unknown binary name falls back to running Central

An unexpected `argv[0]` currently falls back to `Main()` with no signal to operators. In BusyBox-style deployments this can hide misconfigurations (e.g., typos in container `command`/`entrypoint`) and start Central when another component was intended. Please add a warning or info log in the `default:` case that includes the resolved `binaryName` to surface these issues while keeping the fallback behavior.

Suggested implementation:

```golang
	default:
		log.Infof("Unknown binary name %q, falling back to Central", binaryName)
		Main()

```

I only see part of `central/main.go`, so you may need to adapt the SEARCH block to your exact switch statement. The intent is to modify the `default:` branch of the dispatcher `switch` (the one that currently just calls `Main()`) to first emit a log line including the resolved `binaryName`, then call `Main()` as before. Ensure that `binaryName` is in scope in that `default:` branch and that `log.Infof` (or a similar info-level logging method) exists on your logger; if your logger uses a different method name or severity (e.g., `Warnf`), adjust the call accordingly.
</issue_to_address>

### Comment 2
<location path="image/rhel/konflux.Dockerfile" line_range="81-90" />
<code_context>
 RUN GOARCH=$(uname -m) ; \
     case $GOARCH in x86_64) GOARCH=amd64 ;; aarch64) GOARCH=arm64 ;; esac ; \
-    ln -s /assets/downloads/cli/roxctl-linux-$GOARCH /stackrox/roxctl ; \
+    mkdir -p /stackrox/bin && \
+    ln -s /stackrox/central /stackrox/bin/migrator && \
+    ln -s /stackrox/central /stackrox/bin/compliance && \
+    ln -s /stackrox/central /stackrox/bin/kubernetes-sensor && \
+    ln -s /stackrox/central /stackrox/bin/sensor-upgrader && \
+    ln -s /stackrox/central /stackrox/bin/admission-control && \
+    ln -s /stackrox/central /stackrox/bin/roxagent && \
+    ln -s /stackrox/central /stackrox/config-controller && \
+    ln -s /stackrox/central /stackrox/admission-control && \
+    ln -s /stackrox/central /stackrox/kubernetes-sensor && \
+    ln -s /assets/downloads/cli/roxctl-linux-$GOARCH /stackrox/roxctl && \
     ln -s /assets/downloads/cli/roxctl-linux-$GOARCH /assets/downloads/cli/roxctl-linux
</code_context>
<issue_to_address>
**suggestion:** Symlink layout is a bit inconsistent and may be confusing for consumers

The image now exposes some components in both `/stackrox/bin/<component>` and as top-level symlinks (e.g., `/stackrox/config-controller`, `/stackrox/admission-control`, `/stackrox/kubernetes-sensor`), while others like `migrator` and `compliance` only exist under `/stackrox/bin`. Please either standardize on a single convention or ensure every component entrypoint is available in both places to avoid path-dependent behavior in scripts or manifests.
</issue_to_address>

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.

janisz and others added 5 commits April 3, 2026 14:22
The wrapper functions (migratorMain, complianceMain, etc.) added no value -
they just called app.Run(). Simplified by calling app.Run() directly from
the switch statement.

Removes 27 lines of redundant code.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Keep the old build targets so we use the same toolchain for building both
central (consolidated) and individual components. The consolidated binary
works via BusyBox-style dispatch, but individual binaries are still useful
for development and testing.

Both build patterns now work:
- Build central: imports all app packages, dispatches based on argv[0]
- Build individual components: each component's main.go calls app.Run()

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Main() doesn't need to be exported since it's only called from the
dispatcher in the same package. Renamed to centralRun() to follow
Go convention of unexported functions.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Components use a two-tier symlink system:
1. Wrapper scripts in /stackrox/ (from static-bin/) redirect to
2. Binary symlinks in /stackrox/bin/ that point to central

Removed:
- /stackrox/{admission-control,kubernetes-sensor,config-controller}
  (wrappers already exist in static-bin/, only need /stackrox/bin/ targets)
- /stackrox/bin/roxagent (no wrapper exists, unused in K8s deployments)

This preserves the wrapper architecture while eliminating duplicate paths.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Since central now imports all component app packages (migrator, compliance,
admission-control, kubernetes-sensor, sensor-upgrader, config-controller,
roxagent), building central pulls in all their code.

Removed from main-build-nodeps:
- compliance/cmd/compliance
- config-controller
- migrator
- sensor/admission-control
- sensor/kubernetes
- sensor/upgrader
- compliance/virtualmachines/roxagent

These are now accessed via symlinks to the consolidated central binary.
Operator remains separate as it's not part of the main image.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

🚀 Build Images Ready

Images are ready for commit c7378ac. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-561-gc7378acbb9

Restore /stackrox/bin/roxagent symlink that was present in original image.
While roxagent has no wrapper script (used in VM deployments, not K8s),
the symlink should exist for compatibility with existing deployment tooling.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 49.60%. Comparing base (118ca3c) to head (5d9f83e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
roxctl/main.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #19819   +/-   ##
=======================================
  Coverage   49.60%   49.60%           
=======================================
  Files        2763     2763           
  Lines      208271   208254   -17     
=======================================
  Hits       103309   103309           
+ Misses      97294    97278   -16     
+ Partials     7668     7667    -1     
Flag Coverage Δ
go-unit-tests 49.60% <0.00%> (+<0.01%) ⬆️

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.

Move memlimit.SetMemoryLimit() from init() to Run() in component apps:
- compliance/cmd/compliance/app/app.go
- sensor/admission-control/app/app.go
- sensor/kubernetes/app/app.go

When central imports these app packages, their init() functions were running
even when central mode was active, calling SetMemoryLimit() multiple times.
Moving this to Run() ensures it only executes when the specific component
is active.

Add warning log for unknown binary names in central dispatcher:
- central/main.go default case now logs when an unexpected argv[0] falls
  back to central mode, helping surface misconfigurations

Keep config-controller init() unchanged:
- Kubernetes scheme registration is safe to run at import time
- The registered schemes are only used when controller-runtime starts
- Moving to Run() would require complex idempotency guards

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
@janisz janisz changed the title WIP: ROX-33958: Implement BusyBox-style binary consolidation for main image WIP: ROX-33958: BusyBox-style binary consolidation for main image Apr 3, 2026
janisz and others added 3 commits April 3, 2026 15:49
Remove copying of config-controller binary since we only build central now
(which contains all consolidated component code). The config-controller
is accessed via symlink to central, not as a separate binary.

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Allow central to import app packages from other components:
- compliance/cmd/compliance/app
- compliance/virtualmachines/roxagent/app
- config-controller/app
- migrator/app
- sensor/admission-control/app
- sensor/kubernetes/app
- sensor/upgrader/app

These imports are necessary for the BusyBox-style dispatcher in
central/main.go, which routes to the appropriate component based on argv[0].

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Create roxctl/app package and integrate into central binary:
- Created roxctl/app/app.go with Run() function
- Updated roxctl/main.go to call app.Run()
- Moved getCommandPath() to roxctl/main_test.go for test usage
- Added roxctl case to central/main.go dispatcher
- Updated import validator to allow central to import roxctl/app
- Changed /stackrox/roxctl symlink to point to central instead of assets

Benefits:
- /stackrox/roxctl now uses consolidated binary
- Provides option to remove roxctl binaries from /assets later for space savings
- roxctl binaries in /assets still available for user downloads (for now)

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Copy link
Copy Markdown
Contributor

@davdhacs davdhacs left a comment

Choose a reason for hiding this comment

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

looks excellent. Is it finished/ready for review?
Could you include a way to verify that the startup code for each didn't change when migrated over? (like maybe claude could make bash oneliner to show no diff after excluding the intended change, or do a git mv for each with adding the new stripped down in-module main handler/wrapper?)

binaryName := filepath.Base(os.Args[0])

switch binaryName {
case "central":
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a great idea! I haven't looked at busybox to see that it was doing this. Brilliant that no args are required to do the switch (file/link name as the arg!).

ln -s /stackrox/central /stackrox/bin/admission-control && \
ln -s /stackrox/central /stackrox/bin/config-controller && \
ln -s /stackrox/central /stackrox/bin/roxagent && \
ln -s /assets/downloads/cli/roxctl-linux-$GOARCH /stackrox/roxctl && \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is gone as we can use central for this too and this will allow removal of roxctl from dev images:

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest central-db-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest scanner-v4-db-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest main-on-push

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest main-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest scanner-v4-db-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest central-db-on-push

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest central-db-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest roxctl-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest main-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest scanner-v4-on-push

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest scanner-v4-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest roxctl-on-push

@AlexVulaj
Copy link
Copy Markdown
Contributor

Great work on this! In particular I'm a fan of the dispatch logic using filepath.Base(os.Args[0]).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest roxctl-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest operator-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest operator-bundle-on-push

2 similar comments
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest operator-bundle-on-push

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

/konflux-retest operator-bundle-on-push

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 3, 2026

@janisz: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/gke-upgrade-tests 5d9f83e link false /test gke-upgrade-tests
ci/prow/gke-operator-e2e-tests 5d9f83e link false /test gke-operator-e2e-tests
ci/prow/gke-qa-e2e-tests 5d9f83e link false /test gke-qa-e2e-tests
ci/prow/gke-ui-e2e-tests 5d9f83e link true /test gke-ui-e2e-tests
ci/prow/gke-scanner-v4-install-tests 5d9f83e link false /test gke-scanner-v4-install-tests
ci/prow/gke-nongroovy-e2e-tests 5d9f83e link true /test gke-nongroovy-e2e-tests
ci/prow/ocp-4-12-qa-e2e-tests 5d9f83e link false /test ocp-4-12-qa-e2e-tests
ci/prow/ocp-4-12-compliance-e2e-tests 5d9f83e link false /test ocp-4-12-compliance-e2e-tests
ci/prow/ocp-4-12-scanner-v4-install-tests 5d9f83e link false /test ocp-4-12-scanner-v4-install-tests
ci/prow/ocp-4-12-nongroovy-e2e-tests 5d9f83e link false /test ocp-4-12-nongroovy-e2e-tests
ci/prow/ocp-4-21-compliance-e2e-tests 5d9f83e link false /test ocp-4-21-compliance-e2e-tests
ci/prow/ocp-4-21-qa-e2e-tests 5d9f83e link false /test ocp-4-21-qa-e2e-tests
ci/prow/ocp-4-21-scanner-v4-install-tests 5d9f83e link false /test ocp-4-21-scanner-v4-install-tests
ci/prow/ocp-4-21-nongroovy-e2e-tests 5d9f83e link false /test ocp-4-21-nongroovy-e2e-tests
ci/prow/ocp-4-21-operator-e2e-tests 5d9f83e link false /test ocp-4-21-operator-e2e-tests
ci/prow/ocp-4-12-operator-e2e-tests 5d9f83e link false /test ocp-4-12-operator-e2e-tests

Full PR test history. Your PR dashboard.

Details

Instructions 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.

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.

3 participants