Support capsule deploy via foremanctl with dedicated test and fixture updates - #22278
Open
Gauravtalreja1 wants to merge 3 commits into
Open
Support capsule deploy via foremanctl with dedicated test and fixture updates#22278Gauravtalreja1 wants to merge 3 commits into
Gauravtalreja1 wants to merge 3 commits into
Conversation
Gauravtalreja1
added
the
QETestCoverage
Gauravtalreja1
added
No-CherryPick
Contributor
Reviewer's GuideAdds support for deploying capsules via foremanctl/satellitectl alongside the existing installer flow, updates certificate generation to use auth-bundles in foremanctl mode, and introduces new E2E tests and fixtures to validate capsule deployment and auth-bundle behavior. Sequence diagram for capsule deployment via foremanctlsequenceDiagram
autonumber
actor Operator
participant Capsule as Capsule.capsule_setup
participant Satellite as Satellite.capsule_certs_generate
participant Broker as Broker.upstream-pr-install
participant Satellitectl as satellitectl
Operator->>Capsule: capsule_setup(release, capsule_cert_opts)
Capsule->>Capsule: register_to_cdn()
Capsule->>Capsule: setup_rhel_repos()
Capsule->>Capsule: is_foremanctl_available()
alt InstallMethod.FOREMANCTL
Capsule->>Capsule: setup_satellite_repos()
Capsule->>Broker: execute() [pull_requests present]
Capsule->>Capsule: execute('dnf install -y satellitectl')
Capsule->>Capsule: execute('rpm -q satellitectl')
Capsule->>Satellite: capsule_certs_generate(capsule)
Satellite->>Satellitectl: execute('satellitectl auth-bundle hostname')
Satellite-->>Capsule: cert_file_path, install_cmd('satellitectl deploy-proxy ...')
Capsule->>Capsule: execute(install_cmd)
Capsule->>Capsule: execute('systemctl status foreman-proxy.service foreman.target')
else InstallMethod.INSTALLER
Capsule->>Capsule: setup_capsule_repos(release)
Capsule->>Satellite: capsule_certs_generate(capsule)
Satellite->>Satellite: install(InstallerCommand(... 'capsule-certs-generate'))
Satellite-->>Capsule: cert_file_path, InstallerCommand
Capsule->>Capsule: install(InstallerCommand)
Capsule->>Capsule: execute('satellite-maintain service status')
end
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 2 issues, and left some high level feedback:
- In
Satellite.capsule_certs_generate, the FOREMANCTL branch ignoresextra_kwargsand always uses a hardcodeddeploy-proxycommand; if callers rely on passing options viaextra_kwargsthis will behave differently than the installer path—consider either supporting equivalent arguments or clearly separating/renaming the APIs to avoid confusion. - The logic in
Capsule.capsule_setupmixessettings.server.install_methodwithis_foremanctl_available()to deriveproduct_rpm_name, but onlyinstall_methoddrives the repo setup and installer flow; this can lead to situations whererpm -q satellitectlis expected even for INSTALLER-based runs—consider basing the package name solely oninstall_methodor making the decision path more explicit. - There is duplicated satellitectl setup between the
module_cap_ready_rhelfixture and the new FOREMANCTL branch inCapsule.capsule_setup(both configure repos and installsatellitectl); consolidating this into a single helper will reduce divergence and make it clearer which layer is responsible for preparing the capsule host.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Satellite.capsule_certs_generate`, the FOREMANCTL branch ignores `extra_kwargs` and always uses a hardcoded `deploy-proxy` command; if callers rely on passing options via `extra_kwargs` this will behave differently than the installer path—consider either supporting equivalent arguments or clearly separating/renaming the APIs to avoid confusion.
- The logic in `Capsule.capsule_setup` mixes `settings.server.install_method` with `is_foremanctl_available()` to derive `product_rpm_name`, but only `install_method` drives the repo setup and installer flow; this can lead to situations where `rpm -q satellitectl` is expected even for INSTALLER-based runs—consider basing the package name solely on `install_method` or making the decision path more explicit.
- There is duplicated satellitectl setup between the `module_cap_ready_rhel` fixture and the new FOREMANCTL branch in `Capsule.capsule_setup` (both configure repos and install `satellitectl`); consolidating this into a single helper will reduce divergence and make it clearer which layer is responsible for preparing the capsule host.
## Individual Comments
### Comment 1
<location path="robottelo/hosts.py" line_range="2006-2008" />
<code_context>
self.register_to_cdn()
self.setup_rhel_repos()
- self.setup_capsule_repos(release=release)
+ product_rpm_name = (
+ self.container_rpm_name if self.is_foremanctl_available() else self.product_rpm_name
+ )
+ # TODO: Remove this condition once foremanctl is available in capsule repos
+ if settings.server.install_method == InstallMethod.INSTALLER:
</code_context>
<issue_to_address>
**issue (bug_risk):** Align product RPM selection with install method to avoid mismatched package checks
`product_rpm_name` is currently derived only from `is_foremanctl_available()`, not from `settings.server.install_method`. However, you always run `rpm -q {product_rpm_name}` while only installing it in the non-INSTALLER path. If `is_foremanctl_available()` returns `True` while `install_method` is `INSTALLER`, the Installer flow could end up verifying `satellitectl` without ever installing it.
Consider either binding RPM selection directly to `install_method` (FOREMANCTL → `container_rpm_name`, INSTALLER → `product_rpm_name`) or defining separate variables (e.g. `capsule_rpm_name` vs `container_rpm_name`) and using each explicitly per branch to avoid this coupling.
</issue_to_address>
### Comment 2
<location path="robottelo/hosts.py" line_range="2069-2078" />
<code_context>
+ raise CapsuleHostError(
+ f'A core service is not running at capsule host\n{result.stdout}'
+ )
+ if settings.server.install_method == InstallMethod.FOREMANCTL:
+ result = self.execute(installer)
+ if result.status:
</code_context>
<issue_to_address>
**issue (bug_risk):** Respect the `cert_path` argument or clarify its semantics in FOREMANCTL mode
In FOREMANCTL mode, the bundle path is hardcoded to `/var/lib/foremanctl/certs/bundles/{capsule.hostname}.tar.gz`, so the optional `cert_path` argument is effectively ignored, unlike in the Installer branch where it is used.
To avoid surprising callers that rely on `cert_path`, either:
- Use `cert_path` when provided, falling back to the FOREMANCTL default, or
- Reject a non-`None` `cert_path` in FOREMANCTL mode so misuse fails fast rather than being silently ignored.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
evgeni
reviewed
Jul 27, 2026
amolpati30
reviewed
Jul 27, 2026
Collaborator
|
PRT Result |
Satellite-QE
added
the
PRT-Failed
Member
Author
|
evgeni
reviewed
Jul 27, 2026
evgeni
reviewed
Jul 27, 2026
Collaborator
|
PRT Result |
Member
Seems the system has no access to BaseOS repos and thus fails to install things? |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends Robottelo’s host helpers and foremanctl test coverage to support deploying Capsules via the new satellitectl deploy-proxy/auth-bundle workflow, alongside the existing satellite-installer-based approach.
Changes:
- Add a new end-to-end foremanctl Capsule installation test (with repo sync + health/log assertions).
- Extend
Capsule.capsule_setup()andSatellite.capsule_certs_generate()to support a FOREMANCTL install path usingsatellitectl auth-bundleandsatellitectl deploy-proxy. - Update existing foremanctl certificate bundle tests to use
auth-bundlesemantics and validate oauth artifacts in the bundle.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/foreman/foremanctl/test_install_foremanctl.py |
Adds Capsule foremanctl install E2E test/fixtures and renames bundle tests to auth-bundle. |
robottelo/hosts.py |
Adds Capsule/Satellite satellitectl install support and foremanctl-based cert/bundle generation + deploy flow. |
Comments suppressed due to low confidence (2)
robottelo/hosts.py:2051
- For the foremanctl flow,
capsule_certs_generate()returns a bundle path under/var/lib/foremanctl/certs/bundles/....session.remote_copy()expects the destination directory to exist on the Capsule (see other call sites thatmkdir -pbefore copying). On a fresh Capsule host this directory likely won’t exist yet, causing the copy (and then deploy) to fail.
certs_tar, _, installer = self.satellite.capsule_certs_generate(self, **capsule_cert_opts)
self.satellite.session.remote_copy(certs_tar, self)
robottelo/hosts.py:2087
- Same string-join issue as above:
"\n".join(result.stdout)will prevent detectinginactive (dead)in thesystemctl statusoutput.
result = self.execute('systemctl status foreman-proxy.service foreman.target')
if 'inactive (dead)' in '\n'.join(result.stdout):
raise CapsuleHostError(
f'A core service is not running at capsule host\n{result.stdout}'
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2006
to
+2008
| product_rpm_name = ( | ||
| self.container_rpm_name if self.is_foremanctl_available() else self.product_rpm_name | ||
| ) |
… updates Signed-off-by: Gaurav Talreja <gtalreja@redhat.com>
Signed-off-by: Gaurav Talreja <gtalreja@redhat.com>
Gauravtalreja1
force-pushed
the
capsule-setup-with-foremanctl
branch
from
July 28, 2026 21:27
2bedf6d to
4475415
Compare
Member
|
Collaborator
|
PRT Result |
Member
Author
|
Member
|
Member
This failed to deploy Candlepin, which is weird. |
Collaborator
|
PRT Result |
shubhamsg199
force-pushed
the
capsule-setup-with-foremanctl
branch
2 times, most recently
from
July 30, 2026 11:43
3fd7aae to
96e62e1
Compare
Signed-off-by: Gaurav Talreja <gtalreja@redhat.com> Signed-off-by: Shubham Ganar <shubhamsg123m@gmail.com>
shubhamsg199
force-pushed
the
capsule-setup-with-foremanctl
branch
from
July 30, 2026 13:04
96e62e1 to
6847d6a
Compare
Contributor
|
Collaborator
|
PRT Result |
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
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.
Problem Statement
Robottelo currently only supports capsule setup via
satellite-installer. With the introduction offoremanctlthe test framework needs to support deploying and testing capsules usingsatellitectl deploy-proxyalongside the traditional installer path.Solution
Capsule.capsule_setup()in robottelo/hosts.py to support both InstallMethod.INSTALLER and InstallMethod.FOREMANCTL paths:Satellite.capsule_certs_generate()to usesatellitectl auth-bundlefor certificate generation under FOREMANCTL, returning the appropriate cert path and deploy commandcontainer_rpm_name = 'satellitectl'class attribute to both Capsule and Satellitetest_capsule_installation_with_foremanctlwith fixtures for bare RHEL capsule deply, repo sync, and post-install assertionscertificate-bundlewithauth-bundlein testsRelated Issues
theforeman/foremanctl#571
theforeman/foremanctl#698
Summary by Sourcery
Add support for deploying and testing capsules via foremanctl alongside the existing installer-based workflow.
New Features:
Enhancements:
Tests: