Skip to content

Compare nginx version against the major OS version #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
phracek merged 3 commits into from
Apr 21, 2026
Merged

Compare nginx version against the major OS version #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d4430d
Compare nginx version against the major OS version.
phracek Apr 20, 2026
66f9114
Comparsion is valid only for RHEL OS tests
phracek Apr 20, 2026
0e41d4c
Add skip section in case of version is not minimal
phracek Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions 1.20/test/test_container_sizes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../../test/test_container_sizes.py
1 change: 1 addition & 0 deletions 1.22/test/test_container_sizes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../../test/test_container_sizes.py
1 change: 0 additions & 1 deletion 1.24/test/test-lib-nginx.sh
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion 1.24/test/test-lib-openshift.sh
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion 1.24/test/test-lib-remote-openshift.sh
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion 1.24/test/test-openshift.yaml
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions 1.24/test/test_container_sizes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../../test/test_container_sizes.py
1 change: 1 addition & 0 deletions 1.26/test/test_container_sizes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../../test/test_container_sizes.py
24 changes: 21 additions & 3 deletions test/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,16 @@
}

Vars = namedtuple(
"Vars", [
"OS", "TAG", "VERSION", "IMAGE_NAME", "VERSION_NO_MICRO", "SHORT_VERSION", "TEST_DIR"
]
"Vars",
[
"OS",
"TAG",
"VERSION",
"IMAGE_NAME",
"VERSION_NO_MICRO",
"SHORT_VERSION",
"TEST_DIR",
],
)
OS = os.getenv("TARGET").lower()
VERSION = os.getenv("VERSION")
Expand All @@ -37,5 +44,16 @@


def skip_clear_env_tests():
"""
Skip the test if the OS is RHEL 8 and the version is 8.2.
"""
if VARS.OS == "rhel8" and VERSION == "8.2":
skip(f"Skipping clear env tests for {VARS.VERSION} on {VARS.OS}.")


def skip_if_version_not_minimal():
"""
Skip the test if the version is not minimal.
"""
if "minimal" not in VARS.VERSION:
skip("Skipping container size comparison for non-minimal version.")
52 changes: 33 additions & 19 deletions test/test_container_basics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,20 @@


class TestNginxContainer:
"""
Test container basics
"""

def setup_method(self):
"""
Setup method
"""
self.app = ContainerTestLib(image_name=VARS.IMAGE_NAME, s2i_image=True)

def teardown_method(self):
"""
Teardown method
"""
self.app.cleanup()

def test_run_s2i_usage(self):
Expand All @@ -26,41 +35,46 @@ def test_docker_run_usage(self):
"""
Test if container is runnable
"""
assert PodmanCLIWrapper.call_podman_command(
cmd=f"run --rm {VARS.IMAGE_NAME} &>/dev/null",
return_output=False
) == 0
assert (
PodmanCLIWrapper.call_podman_command(
cmd=f"run --rm {VARS.IMAGE_NAME} &>/dev/null", return_output=False
)
== 0
)

def test_scl_usage(self):
"""
Test if nginx -v returns proper output
"""

assert f"nginx/{VARS.VERSION_NO_MICRO}" in PodmanCLIWrapper.podman_run_command_and_remove(
cid_file_name=VARS.IMAGE_NAME,
cmd="nginx -v"
assert (
f"nginx/{VARS.VERSION_NO_MICRO}"
in PodmanCLIWrapper.podman_run_command_and_remove(
cid_file_name=VARS.IMAGE_NAME, cmd="nginx -v"
)
)

@pytest.mark.parametrize(
"dockerfile",
[
"Dockerfile",
"Dockerfile.s2i"
]
)
@pytest.mark.parametrize("dockerfile", ["Dockerfile", "Dockerfile.s2i"])
def test_dockerfiles(self, dockerfile):
"""
Test if building nginx-container based on
examples/Dockerfile works
"""
dp = DockerfileProcessor(dockerfile_path=f"{VARS.TEST_DIR}/examples/{dockerfile}")
dp.update_env_in_dockerfile(version=VARS.VERSION_NO_MICRO, what_to_replace="ENV NGINX_VERSION")
dp.update_variable_in_dockerfile(version=VARS.VERSION_NO_MICRO, variable="NGINX_VERSION")
dp = DockerfileProcessor(
dockerfile_path=f"{VARS.TEST_DIR}/examples/{dockerfile}"
)
dp.update_env_in_dockerfile(
version=VARS.VERSION_NO_MICRO, what_to_replace="ENV NGINX_VERSION"
)
dp.update_variable_in_dockerfile(
version=VARS.VERSION_NO_MICRO, variable="NGINX_VERSION"
)
new_docker_file = dp.create_temp_dockerfile()

assert self.app.build_test_container(
dockerfile=new_docker_file, app_url="https://github.com/sclorg/nginx-container.git",
app_dir="nginx-container"
dockerfile=new_docker_file,
app_url="https://github.com/sclorg/nginx-container.git",
app_dir="nginx-container",
)
assert self.app.test_app_dockerfile()
cip = self.app.get_cip()
Expand Down
53 changes: 53 additions & 0 deletions test/test_container_sizes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import pytest

from container_ci_suite.container_lib import ContainerTestLib
from container_ci_suite.compare_images import ContainerCompareClass
from container_ci_suite.utils import get_public_image_name, get_previous_os_version

from conftest import VARS, skip_if_version_not_minimal


class TestNginxContainerSizes:
"""
Test container sizes
"""

def setup_method(self):
"""
Setup method
"""
self.app = ContainerTestLib(image_name=VARS.IMAGE_NAME, s2i_image=True)

def teardown_method(self):
"""
Teardown method
"""
self.app.cleanup()

def test_compare_container_sizes(self):
"""
Test the size of the Nginx container against the
already published container images.
"""
skip_if_version_not_minimal()
if not VARS.OS.startswith("rhel"):
pytest.skip("Skipping container size comparison for non-RHEL OS.")
published_image_name = get_public_image_name(
os_name=get_previous_os_version(VARS.OS),
base_image_name="nginx",
version=VARS.VERSION,
stage_registry=True,
)
Comment thread
phracek marked this conversation as resolved.
is_less_uncopressed = ContainerCompareClass.is_uncompressed_image_smaller(
built_image_name=VARS.IMAGE_NAME,
published_image=published_image_name,
)
is_less_compressed = ContainerCompareClass.is_compressed_image_smaller(
built_image_name=VARS.IMAGE_NAME,
published_image_name=published_image_name,
)
Comment thread
phracek marked this conversation as resolved.
if not is_less_uncopressed or not is_less_compressed:
pytest.skip(
f"Container size is not less than the published image {published_image_name}. "
f"Uncompressed image size: {is_less_uncopressed}, Compressed image size: {is_less_compressed}"
)
Comment on lines +27 to +53
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 21, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm get_previous_os_version behavior for the lowest supported RHEL and return type of the comparison helpers.
python - <<'PY'
import inspect
try:
    from container_ci_suite.utils import get_previous_os_version, get_public_image_name
    print("get_previous_os_version:", inspect.signature(get_previous_os_version))
    for os_name in ("rhel8", "rhel9", "rhel10"):
        try:
            print(os_name, "->", get_previous_os_version(os_name))
        except Exception as e:
            print(os_name, "raises", type(e).__name__, e)
except Exception as e:
    print("import failed:", e)
PY

Repository: sclorg/nginx-container

Length of output: 117

🏁 Script executed:

#!/bin/bash
# Explore repository structure to find test file and helper function implementations
git ls-files | head -20
echo "---"
fd -t f "test_container_sizes" 
fd -t f "\.py$" | grep -E "(utils|container)" | head -20

Repository: sclorg/nginx-container

Length of output: 780

🏁 Script executed:

#!/bin/bash
# Read the test file mentioned in the review
head -100 test/test_container_sizes.py

Repository: sclorg/nginx-container

Length of output: 1925

🏁 Script executed:

#!/bin/bash
# Search for the helper functions referenced in the test
rg "def get_previous_os_version|def get_public_image_name|class ContainerCompareClass" -A 10

Repository: sclorg/nginx-container

Length of output: 48

🏁 Script executed:

#!/bin/bash
# Check conftest.py and look for dependency definitions
cat -n test/conftest.py

Repository: sclorg/nginx-container

Length of output: 1763

🏁 Script executed:

#!/bin/bash
# Look for dependency/requirements files
fd -t f "requirements|setup|pyproject" | xargs ls -la 2>/dev/null
git ls-files | grep -E "requirements|setup|pyproject"

Repository: sclorg/nginx-container

Length of output: 1281

🏁 Script executed:

#!/bin/bash
# Check .gitmodules to see if container_ci_suite is a submodule
cat .gitmodules 2>/dev/null || echo "No .gitmodules found"

Repository: sclorg/nginx-container

Length of output: 163

🏁 Script executed:

#!/bin/bash
# Search for any existing try/except patterns in test files or error handling
rg "try:|except|raise" test/ --type py | head -30

Repository: sclorg/nginx-container

Length of output: 48

🏁 Script executed:

#!/bin/bash
# Check GitHub workflows to understand test execution
cat -n .github/workflows/container-tests.yml | head -100

Repository: sclorg/nginx-container

Length of output: 684

🏁 Script executed:

#!/bin/bash
# Search for all uses of get_previous_os_version in the test directory
rg "get_previous_os_version" test/

Repository: sclorg/nginx-container

Length of output: 261

🏁 Script executed:

#!/bin/bash
# Check for any documentation about test dependencies
cat README.md | grep -A 5 -B 5 "test\|pytest\|container_ci_suite" | head -40

Repository: sclorg/nginx-container

Length of output: 1310

🏁 Script executed:

#!/bin/bash
# Check if there's any mention of how to handle edge cases
rg "rhel8|rhel9|rhel10" test/ -B 2 -A 2

Repository: sclorg/nginx-container

Length of output: 1132

Wrap helper calls in try/except and add logging for comparison results.

The test will error (not skip) if get_previous_os_version raises when called with an unsupported RHEL version. Since the code only defines tags for rhel8, rhel9, and rhel10 in conftest.py, calling get_previous_os_version("rhel8") could fail if there's no defined previous version. Wrap the get_public_image_name call (and potentially the comparison methods) in a try/except block that skips the test on lookup failure.

Additionally, add logging of the raw is_less_uncompressed and is_less_compressed values at info level. If these helpers silently return None/falsy on network errors (e.g., registry unreachable), the current skip message won't distinguish a true size comparison failure from a transient network issue, making CI flakiness harder to diagnose.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test/test_container_sizes.py` around lines 27 - 53, The test should catch
lookup/network failures and log comparison results: wrap the call to
get_public_image_name(... get_previous_os_version(VARS.OS) ...) in a try/except
that calls pytest.skip when get_previous_os_version or get_public_image_name
raises (include a message with the exception), and similarly wrap the calls to
ContainerCompareClass.is_uncompressed_image_smaller and
is_compressed_image_smaller to catch exceptions and skip with an explanatory
message; after obtaining their return values, log the raw is_less_uncompressed
and is_less_compressed at info level and explicitly treat None or other falsy
non-boolean returns as a transient error (skip with a message indicating
possible network/registry issue) rather than a plain size failure before calling
pytest.skip for genuine size comparisons.

Comment on lines +41 to +53
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 21, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Typo: is_less_uncopressed → is_less_uncompressed.

Local variable misspelling (missing m) repeated 3 times. Harmless but leaks into the skip message shown in CI logs.

Proposed fix 
-        is_less_uncopressed = ContainerCompareClass.is_uncompressed_image_smaller(
+        is_less_uncompressed = ContainerCompareClass.is_uncompressed_image_smaller(
             built_image_name=VARS.IMAGE_NAME,
             published_image=published_image_name,
         )
         is_less_compressed = ContainerCompareClass.is_compressed_image_smaller(
             built_image_name=VARS.IMAGE_NAME,
             published_image_name=published_image_name,
         )
-        if not is_less_uncopressed or not is_less_compressed:
+        if not is_less_uncompressed or not is_less_compressed:
             pytest.skip(
                 f"Container size is not less than the published image {published_image_name}. "
-                f"Uncompressed image size: {is_less_uncopressed}, Compressed image size: {is_less_compressed}"
+                f"Uncompressed image size: {is_less_uncompressed}, Compressed image size: {is_less_compressed}"
             )
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test/test_container_sizes.py` around lines 41 - 53, Rename the misspelled
local variable is_less_uncopressed to is_less_uncompressed everywhere it's used
in test/test_container_sizes.py: update the assignment from
ContainerCompareClass.is_uncompressed_image_smaller(...) to store into
is_less_uncompressed, update the conditional that checks it (if not
is_less_uncompressed or not is_less_compressed) and update the pytest.skip
message interpolation to reference is_less_uncompressed instead of
is_less_uncopressed so the variable name matches the intended meaning and avoids
the typo in CI logs.