[6.19.z] Replace library/busybox with lighter container repo - #20853
Merged
lpramuk merged 1 commit intoFeb 25, 2026
Merged
Conversation
(cherry picked from commit 8ac73bc)
Satellite-QE
added
6.19.z
Auto_Cherry_Picked
Collaborator
Author
|
Satellite-QE
added
the
AutoMerge_Cherry_Picked
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces the heavy library/busybox default container image with the lighter jmalloc/echo-server in configuration and adapts the container pull-and-run CLI test to work with the new image and output format while simplifying container lifecycle handling. Sequence diagram for the updated container pull and run CLI testsequenceDiagram
actor Tester
participant TestCase as ContainerManagementTest
participant CLI as ContainerCLI
participant Registry as ContainerRegistry
participant Runtime as ContainerRuntime
Tester->>TestCase: execute test_container_pull_and_run
TestCase->>CLI: pull_image(jmalloc/echo-server)
CLI->>Registry: request image jmalloc/echo-server
Registry-->>CLI: image layers
CLI-->>TestCase: pull result
TestCase->>CLI: run_container_detached(jmalloc/echo-server)
CLI->>Runtime: run -d jmalloc/echo-server
Runtime-->>CLI: container_id
CLI-->>TestCase: container_id
TestCase->>CLI: check_container_status(container_id)
CLI->>Runtime: inspect container_id
Runtime-->>CLI: status
CLI-->>TestCase: status
TestCase->>CLI: cleanup_container(container_id)
CLI->>Runtime: stop container_id
Runtime-->>CLI: stopped
CLI->>Runtime: remove container_id
Runtime-->>CLI: removed
CLI-->>TestCase: cleanup complete
TestCase-->>Tester: test result
Updated class diagram for container configuration validator defaultsclassDiagram
class Validator {
+str key
+bool must_exist
+type is_type_of
+any default
+validate(value)
}
class ContainerConfig {
+str container_upstream_name = jmalloc/echo-server
+list container_alternative_upstream_names
+get_value(key)
+set_value(key, value)
}
class ConfigValidators {
+Validator container_upstream_name_validator
+Validator container_alternative_upstream_names_validator
+load_defaults(ContainerConfig config)
}
ConfigValidators o-- Validator : uses
ContainerConfig o-- ConfigValidators : configured_by
ContainerConfig ..> Validator : validated_by
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
test_positive_pull_image,container_idcan be referenced in thefinallyblock before assignment if thedocker runcommand fails or the regex doesn’t match; initializecontainer_id = Nonebefore the try and use that consistently in the finally. - The container ID extraction using
re.match(r'^[0-9a-f]+$', result.stdout)is quite fragile (e.g., trailing newlines or extra output); consider a more robust approach such asresult.stdout.split()[0]to obtain the ID from thedocker run -doutput.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `test_positive_pull_image`, `container_id` can be referenced in the `finally` block before assignment if the `docker run` command fails or the regex doesn’t match; initialize `container_id = None` before the try and use that consistently in the finally.
- The container ID extraction using `re.match(r'^[0-9a-f]+$', result.stdout)` is quite fragile (e.g., trailing newlines or extra output); consider a more robust approach such as `result.stdout.split()[0]` to obtain the ID from the `docker run -d` output.
## Individual Comments
### Comment 1
<location path="tests/foreman/cli/test_container_management.py" line_range="102-103" />
<code_context>
+ f'docker run -d {repo["published-at"]}'
+ )
assert result.status == 0
+ match = re.match(r'^[0-9a-f]+$', result.stdout)
+ if match:
+ container_id = match.group(0)
finally:
</code_context>
<issue_to_address>
**issue (bug_risk):** Potential use of `container_id` before assignment and missing assertion that it was parsed correctly
`container_id` is only set when the regex matches, but it’s always referenced in the `finally` block. If `docker run -d` exits 0 but `stdout` is empty or doesn’t match, `container_id` will be undefined and cleanup will be skipped. Initialize `container_id = None` before the inner `try`, and when `result.status == 0` assert that `container_id is not None` (e.g., `assert container_id, 'Expected docker run to return a container ID'`) so failures are explicit and cleanup logic always has a defined value.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+102
to
+103
| match = re.match(r'^[0-9a-f]+$', result.stdout) | ||
| if match: |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Potential use of container_id before assignment and missing assertion that it was parsed correctly
container_id is only set when the regex matches, but it’s always referenced in the finally block. If docker run -d exits 0 but stdout is empty or doesn’t match, container_id will be undefined and cleanup will be skipped. Initialize container_id = None before the inner try, and when result.status == 0 assert that container_id is not None (e.g., assert container_id, 'Expected docker run to return a container ID') so failures are explicit and cleanup logic always has a defined value.
Contributor
|
Collaborator
Author
|
PRT Result |
Satellite-QE
added
the
PRT-Failed
Contributor
|
trigger: test-robottelo |
Collaborator
Author
|
PRT Result |
Satellite-QE
added
PRT-Passed
lpramuk
enabled auto-merge (squash)
February 25, 2026 13:19
lpramuk
disabled auto-merge
February 25, 2026 13:20
lpramuk
deleted the
cherry-pick-6.19.z-8ac73bc97a2531bb9b282f00adced329b16c0c93
branch
February 25, 2026 13:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherrypick of PR: #20671
Problem Statement
Gradually over time the
library/busyboxrepo is becoming more and more huge.The problem with repo sync arises especially in IPv6 environment where the traffic goes over IPv6-to-4 proxy.
Testing revealed that syncing over IPv4 takes approx. 10 min while syncing over IPv6 takes 25 min and tends to time out quite often.
Solution
Find more suitable replacement for testing container repo target
Related Issues
SAT-41831
Summary by Sourcery
Update default container image configuration and adjust container management tests to work with the new image behavior.
Bug Fixes:
Enhancements:
Tests: