Skip to content

Fix query db in containerized envs - #22140

Merged
Gauravtalreja1 merged 7 commits into
SatelliteQE:masterfrom
aidenfine:SAT-47278-fix-query-db
Jul 23, 2026
Merged

Fix query db in containerized envs#22140
Gauravtalreja1 merged 7 commits into
SatelliteQE:masterfrom
aidenfine:SAT-47278-fix-query-db

Conversation

@aidenfine

@aidenfine aidenfine commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Problem Statement

fix the query_db function to support, containerized Satellite env where psql runs in separate container.

SAT-47278

Solution

Update _execute_db_query function to check install method. This will change the base cmd that is ran when running sql queries.

PRT test Cases example

trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py::test_repository_rpms_id_type tests/foreman/api/test_notifications.py::test_positive_notification_for_long_running_tasks tests/foreman/api/test_repository.py::TestRepositorySync::test_positive_sync_repo_null_contents_changed tests/foreman/cli/test_oscap.py::TestOpenScap::test_positive_associate_scap_policy_with_hostgroups tests/foreman/maintain/test_health.py::test_positive_health_check_corrupted_roles tests/foreman/maintain/test_health.py::test_positive_health_check_duplicate_permissions tests/foreman/ui/test_host.py::test_positive_search_by_report_origin_shows_all_hosts

Summary by Sourcery

Update database query helper to support different Satellite installation methods and refactor tests to use it consistently instead of ad‑hoc psql shell commands.

Bug Fixes:

  • Fix database querying in containerized Satellite environments by selecting the appropriate psql invocation based on install method.

Enhancements:

  • Extend query_db to choose the correct base psql command for foremanctl-based and traditional installations.
  • Adjust tests to rely on query_db return values rather than shell command status/stdout parsing where appropriate.

Tests:

  • Refactor multiple API, CLI, UI, and maintain health-check tests to use the shared query_db helper for database setup and verification.
  • Mark selected health-check tests with the foreman_installer marker to scope them to relevant environments.

@aidenfine
aidenfine requested review from a team as code owners July 13, 2026 19:19
@sourcery-ai

sourcery-ai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Refactors tests and host database helpers to use a new, install-method-aware query_db helper instead of manually invoking psql via shell, adding support for containerized Satellite (foremanctl/podman) deployments and slightly tightening some test assertions.

Sequence diagram for query_db behavior by install method

sequenceDiagram
    participant Caller
    participant Host
    participant _execute_db_query
    participant SystemExecute as execute

    Caller->>Host: query_db(query, db, output_format)
    Host->>Host: [install_method is FOREMANCTL]
    alt install_method is FOREMANCTL
        Host->>Host: build base_cmd podman exec postgresql psql -U foreman -d db
    else install_method is not FOREMANCTL
        Host->>Host: build base_cmd sudo -u postgres psql -d db
    end
    Host->>_execute_db_query: _execute_db_query(cmd)
    _execute_db_query->>SystemExecute: execute(cmd)
    SystemExecute-->>_execute_db_query: result
    alt result.status != 0
        _execute_db_query->>Caller: raise CLIReturnCodeError
    else result.status == 0
        _execute_db_query-->>Host: result
        Host-->>Caller: formatted query result
    end
Loading

File-Level Changes

Change Details Files
Make query_db aware of the Satellite install method so DB queries work both on traditional and containerized (foremanctl) installs.
  • Import InstallMethod enum in hosts module to introspect the current Satellite install method.
  • Update _execute_db_query to build the base psql command conditionally: use podman exec postgresql psql -U foreman for FOREMANCTL installs, otherwise use sudo -u postgres psql.
  • Leave JSON vs raw output behavior intact while routing all DB access through the new base command.
robottelo/hosts.py
Update tests to use query_db instead of raw psql shell invocations and adjust expectations to match the new helper’s return type.
  • Replace target_sat.execute/sat_maintain.execute psql calls with target_sat.query_db or sat_maintain.query_db across API, CLI, UI, and maintain test modules.
  • Switch assertions from checking process status/stdout to comparing query_db results directly (e.g., asserting empty lists for 0-row results or substring checks on raw string output).
  • Extract SQL strings to clearer variables where needed and simplify quoting/escaping now that queries are passed directly to query_db.
tests/foreman/maintain/test_health.py
tests/foreman/ui/test_host.py
tests/foreman/api/test_notifications.py
tests/foreman/api/test_repository.py
tests/foreman/cli/test_oscap.py
tests/foreman/api/test_contentview.py
Scope certain health-check tests to foreman-installer environments where the DB access pattern is valid.
  • Add @pytest.mark.foreman_installer marker to corrupted-roles and duplicate-permissions health check tests to ensure they only run on compatible setups.
tests/foreman/maintain/test_health.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@aidenfine aidenfine added the No-CherryPick PR doesnt need CherryPick to previous branches label Jul 13, 2026

@sourcery-ai sourcery-ai Bot left a comment

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.

Hey - I've left some high level feedback:

  • Consider moving from robottelo.enums import InstallMethod to the module level instead of importing inside query_db, to avoid repeated imports and make dependencies clearer.
  • The podman exec postgresql psql -U foreman -d {db} base command in query_db assumes a specific container name and user; if these differ across environments, you may want to derive them from configuration or install_method metadata rather than hard-coding.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider moving `from robottelo.enums import InstallMethod` to the module level instead of importing inside `query_db`, to avoid repeated imports and make dependencies clearer.
- The `podman exec postgresql psql -U foreman -d {db}` base command in `query_db` assumes a specific container name and user; if these differ across environments, you may want to derive them from configuration or `install_method` metadata rather than hard-coding.

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.

@aidenfine

Copy link
Copy Markdown
Contributor Author
trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py::test_repository_rpms_id_type tests/foreman/api/test_notifications.py::test_positive_notification_for_long_running_tasks tests/foreman/api/test_repository.py::TestRepositorySync::test_positive_sync_repo_null_contents_changed tests/foreman/cli/test_oscap.py::TestOpenScap::test_positive_associate_scap_policy_with_hostgroups tests/foreman/maintain/test_health.py::test_positive_health_check_corrupted_roles tests/foreman/maintain/test_health.py::test_positive_health_check_duplicate_permissions tests/foreman/ui/test_host.py::test_positive_search_by_report_origin_shows_all_hosts

@aidenfine aidenfine changed the title Sat 47278 fix query db Fix query db in containerized envs Jul 13, 2026
@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 16115
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_contentview.py::test_repository_rpms_id_type tests/foreman/api/test_notifications.py::test_positive_notification_for_long_running_tasks tests/foreman/api/test_repository.py::TestRepositorySync::test_positive_sync_repo_null_contents_changed tests/foreman/cli/test_oscap.py::TestOpenScap::test_positive_associate_scap_policy_with_hostgroups tests/foreman/maintain/test_health.py::test_positive_health_check_corrupted_roles tests/foreman/maintain/test_health.py::test_positive_health_check_duplicate_permissions tests/foreman/ui/test_host.py::test_positive_search_by_report_origin_shows_all_hosts --external-logging
Test Result : ================= 7 passed, 89 warnings in 1206.35s (0:20:06) ==================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Jul 13, 2026
Comment thread robottelo/hosts.py Outdated
Comment thread tests/foreman/maintain/test_health.py Outdated
@vijaysawant

Copy link
Copy Markdown
Contributor

Could you please add related Jira link into description?

@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Jul 16, 2026
@vsedmik

vsedmik commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
trigger: test-robottelo
pytest: tests/foreman/cli/test_pulp.py

@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 16160
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_pulp.py --external-logging
Test Result : ================= 20 warnings, 19 errors in 5108.03s (1:25:08) =================

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Jul 20, 2026
Comment thread robottelo/hosts.py Outdated
@vsedmik

vsedmik commented Jul 21, 2026

Copy link
Copy Markdown
Contributor
trigger: test-robottelo
pytest: tests/foreman/cli/test_pulp.py

@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 16179
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_pulp.py --external-logging
Test Result : ================= 19 passed, 43 warnings in 1119.26s (0:18:39) =================

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Jul 21, 2026

@vsedmik vsedmik left a comment

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.

Looks good to me, PRT passed, pending proposal from Gaurav.

@LadislavVasina1 LadislavVasina1 left a comment

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.

ACK, pending comments

@aidenfine
aidenfine force-pushed the SAT-47278-fix-query-db branch from 3808d51 to 9223dcc Compare July 23, 2026 01:44
@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Jul 23, 2026
@aidenfine

Copy link
Copy Markdown
Contributor Author
trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py::test_repository_rpms_id_type tests/foreman/api/test_notifications.py::test_positive_notification_for_long_running_tasks tests/foreman/api/test_repository.py::TestRepositorySync::test_positive_sync_repo_null_contents_changed tests/foreman/cli/test_oscap.py::TestOpenScap::test_positive_associate_scap_policy_with_hostgroups tests/foreman/maintain/test_health.py::test_positive_health_check_corrupted_roles tests/foreman/maintain/test_health.py::test_positive_health_check_duplicate_permissions tests/foreman/ui/test_host.py::test_positive_search_by_report_origin_shows_all_hosts

@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 16204
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_contentview.py::test_repository_rpms_id_type tests/foreman/api/test_notifications.py::test_positive_notification_for_long_running_tasks tests/foreman/api/test_repository.py::TestRepositorySync::test_positive_sync_repo_null_contents_changed tests/foreman/cli/test_oscap.py::TestOpenScap::test_positive_associate_scap_policy_with_hostgroups tests/foreman/maintain/test_health.py::test_positive_health_check_corrupted_roles tests/foreman/maintain/test_health.py::test_positive_health_check_duplicate_permissions tests/foreman/ui/test_host.py::test_positive_search_by_report_origin_shows_all_hosts --external-logging
Test Result : ================= 7 passed, 90 warnings in 1249.56s (0:20:49) ==================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Jul 23, 2026
Comment thread robottelo/hosts.py Outdated
@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Jul 23, 2026
@aidenfine

Copy link
Copy Markdown
Contributor Author
trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py::test_repository_rpms_id_type tests/foreman/api/test_notifications.py::test_positive_notification_for_long_running_tasks tests/foreman/api/test_repository.py::TestRepositorySync::test_positive_sync_repo_null_contents_changed tests/foreman/cli/test_oscap.py::TestOpenScap::test_positive_associate_scap_policy_with_hostgroups tests/foreman/maintain/test_health.py::test_positive_health_check_corrupted_roles tests/foreman/maintain/test_health.py::test_positive_health_check_duplicate_permissions tests/foreman/ui/test_host.py::test_positive_search_by_report_origin_shows_all_hosts

@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 16219
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/api/test_contentview.py::test_repository_rpms_id_type tests/foreman/api/test_notifications.py::test_positive_notification_for_long_running_tasks tests/foreman/api/test_repository.py::TestRepositorySync::test_positive_sync_repo_null_contents_changed tests/foreman/cli/test_oscap.py::TestOpenScap::test_positive_associate_scap_policy_with_hostgroups tests/foreman/maintain/test_health.py::test_positive_health_check_corrupted_roles tests/foreman/maintain/test_health.py::test_positive_health_check_duplicate_permissions tests/foreman/ui/test_host.py::test_positive_search_by_report_origin_shows_all_hosts --external-logging
Test Result : ================= 7 passed, 92 warnings in 1341.65s (0:22:21) ==================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Jul 23, 2026
@Gauravtalreja1

Copy link
Copy Markdown
Member
trigger: test-robottelo
pytest: tests/foreman/api/test_contentview.py::test_repository_rpms_id_type tests/foreman/api/test_notifications.py::test_positive_notification_for_long_running_tasks tests/foreman/api/test_repository.py::TestRepositorySync::test_positive_sync_repo_null_contents_changed tests/foreman/cli/test_oscap.py::TestOpenScap::test_positive_associate_scap_policy_with_hostgroups tests/foreman/maintain/test_health.py::test_positive_health_check_corrupted_roles tests/foreman/maintain/test_health.py::test_positive_health_check_duplicate_permissions tests/foreman/ui/test_host.py::test_positive_search_by_report_origin_shows_all_hosts
deployment_type: container

@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 16223
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/api/test_contentview.py::test_repository_rpms_id_type tests/foreman/api/test_notifications.py::test_positive_notification_for_long_running_tasks tests/foreman/api/test_repository.py::TestRepositorySync::test_positive_sync_repo_null_contents_changed tests/foreman/cli/test_oscap.py::TestOpenScap::test_positive_associate_scap_policy_with_hostgroups tests/foreman/maintain/test_health.py::test_positive_health_check_corrupted_roles tests/foreman/maintain/test_health.py::test_positive_health_check_duplicate_permissions tests/foreman/ui/test_host.py::test_positive_search_by_report_origin_shows_all_hosts --external-logging
Test Result : ===== 3 passed, 2 deselected, 79 warnings, 2 errors in 1055.87s (0:17:35) ======

@Satellite-QE Satellite-QE added PRT-Failed Indicates that latest PRT run is failed for the PR and removed PRT-Passed Indicates that latest PRT run is passed for the PR labels Jul 23, 2026
@aidenfine

Copy link
Copy Markdown
Contributor Author

Taking look at failures now. Wasnt aware that was a deployment_type PRT arg.

@aidenfine

Copy link
Copy Markdown
Contributor Author

@Gauravtalreja1 Can you also take a look at the failure they seem unrelated to the query_db, and just things are already didn't work in containerized envs. So unrelated to this PR/Jira scope?

@Gauravtalreja1
Gauravtalreja1 merged commit de98b57 into SatelliteQE:master Jul 23, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No-CherryPick PR doesnt need CherryPick to previous branches PRT-Failed Indicates that latest PRT run is failed for the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants