Skip to content

Conversation

pondrejk
Copy link
Contributor

Problem Statement

to update tests to look at the prod image registry

Solution

Related Issues

@pondrejk pondrejk self-assigned this Oct 20, 2025
@pondrejk pondrejk requested a review from a team as a code owner October 20, 2025 11:43
@pondrejk pondrejk added CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.18.z Introduced in or relating directly to Satellite 6.18 labels Oct 20, 2025
@pondrejk pondrejk marked this pull request as draft October 20, 2025 11:43
Copy link

@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 there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `pytest_fixtures/component/mcp.py:66-69` </location>
<code_context>
def _setup_mcp_server(target_sat, settings_obj, is_downstream=False):
    """Helper function to set up MCP server.

    :param target_sat: The target satellite instance
    :param settings_obj: The settings object (foreman_mcp or foreman_mcp_downstream)
    :param is_downstream: Whether this is a downstream setup
    :return: Container name for cleanup
    """
    if not target_sat.network_type.has_ipv4:
        target_sat.enable_ipv6_dnf_and_rhsm_proxy()
        target_sat.enable_ipv6_system_proxy()
    container_name = (
        f'mcp_server{"_downstream" if is_downstream else ""}-{datetime.timestamp(datetime.now())}'
    )
    image_name = settings_obj.image_path.split('/')[-1]
    assert (
        target_sat.execute(f'firewall-cmd --permanent --add-port="{settings_obj.port}/tcp"').status
        == 0
    )
    assert target_sat.execute('firewall-cmd --reload').status == 0
    target_sat.ensure_podman_installed()
    if is_downstream:
        target_sat.podman_login(
            settings_obj.registry_username,
            settings_obj.registry_password,
            settings_obj.registry,
        )
        registry = settings_obj.registry
        ca_mountpoint = '/opt/app-root/src/ca.pem'
    else:
        registry = settings_obj.registry
        ca_mountpoint = '/app/ca.pem'
    if not target_sat.network_type.has_ipv4:
        target_sat.execute('podman network create --ipv6 ipv6')

    authfile_arg = f"--authfile {PODMAN_AUTHFILE_PATH}" if is_downstream else ""
    network_arg = "--network ipv6" if not target_sat.network_type.has_ipv4 else ""

    pull_cmd = f'podman pull {authfile_arg} {registry}/{settings_obj.image_path}'.strip()
    assert target_sat.execute(pull_cmd).status == 0

    run_cmd = (
        f'podman run {network_arg} {authfile_arg} '
        f'-v /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem:{ca_mountpoint}:ro,Z '
        f'--name {container_name} -d --pull=never -it -p {settings_obj.port}:8080 '
        f'{image_name} --foreman-url https://{target_sat.hostname} --host 0.0.0.0'
    ).strip()
    target_sat.execute(run_cmd)
    wait_for(
        lambda: target_sat.execute(f'curl localhost:{settings_obj.port}/mcp/').status == 0,
        timeout=60,
        delay=2,
    )
    result = target_sat.execute(f'podman inspect -f "{{{{.State.Status}}}}" {container_name}')
    log = target_sat.execute(f'podman logs {container_name}')
    assert result.stdout.strip() == 'running', (
        f'failed to start container {container_name}: {log.stdout}'
        f'failed to start container {container_name}: {log.stderr}'
    )
    return container_name

</code_context>

<issue_to_address>
**issue (code-quality):** Hoist repeated code outside conditional statement ([`hoist-statement-from-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/hoist-statement-from-if/))
</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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.18.z Introduced in or relating directly to Satellite 6.18 AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing CherryPick PR needs CherryPick to previous branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant