Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 9 additions & 13 deletions doozer/doozerlib/cli/scan_sources_konflux.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ def _is_image_enabled(self, image_meta: ImageMetadata) -> bool:

For OKD variant:
- Image is enabled if generally enabled OR has okd.mode: enabled
- Image must have for_payload: true (non-payload images are not built for OKD)
- Image must have for_payload: true OR base_only: true
(base_only images are required by the OKD build pipeline as parent dependencies)

For OCP variant:
- Image must be generally enabled (mode != 'disabled')
Expand All @@ -142,11 +143,14 @@ def _is_image_enabled(self, image_meta: ImageMetadata) -> bool:
# For OKD, image is enabled if generally enabled OR has okd.mode: enabled
if not self._is_okd_enabled(image_meta):
return False
# For OKD, also check for_payload - only payload images are built
# For OKD, include payload images AND base_only images (dependency chain)
for_payload = image_meta.config.for_payload
if for_payload is Missing:
for_payload = False
return for_payload
base_only = image_meta.config.base_only
if base_only is Missing:
base_only = False
return for_payload or base_only
else:
# For OCP, only process generally enabled images (not OKD-only images)
return image_meta.enabled
Expand All @@ -159,8 +163,8 @@ def _is_image_enabled_for_scan(self, image_meta: ImageMetadata) -> bool:
- It's enabled (generally enabled OR okd.mode: enabled), OR
- load_disabled is set (includes all images even if disabled)

For OKD variant, additionally filters out non-payload images (for_payload: false)
since OKD only builds images that are in the payload.
For OKD variant, _is_image_enabled already handles the payload/base_only filter,
so no additional filtering is needed here.

This ensures OKD-only images (mode: disabled, okd.mode: enabled) are scanned
so they can be built for OKD when they change.
Expand All @@ -170,15 +174,7 @@ def _is_image_enabled_for_scan(self, image_meta: ImageMetadata) -> bool:
Return Value(s):
bool: True if image should be included, False otherwise.
"""
# Include if enabled (handles both general and OKD-enabled cases)
if self._is_image_enabled(image_meta):
# For OKD variant, skip non-payload images
if self.variant == BuildVariant.OKD:
for_payload = image_meta.config.for_payload
if for_payload is Missing:
for_payload = False
if not for_payload:
return False
return True

# Include if disabled but load_disabled is set
Expand Down
159 changes: 159 additions & 0 deletions doozer/tests/cli/test_scan_sources_konflux.py
Original file line number Diff line number Diff line change
Expand Up @@ -929,3 +929,162 @@ async def test_skip_check_if_already_changing(self):
with patch.object(self.scanner, '_fetch_art_yaml_from_rebase', new_callable=AsyncMock) as mock_fetch:
await self.scanner.scan_external_image_changes(self.image_meta)
mock_fetch.assert_not_called()


class TestOkdImageFiltering(TestScanSourcesKonflux):
"""Tests for OKD image filtering including base_only images (ART-21083 Gap 2)."""

def _make_okd_scanner(self):
"""Create a scanner with OKD variant."""
scanner = ConfigScanSources(
runtime=self.runtime,
ci_kubeconfig=self.ci_kubeconfig,
session=self.session,
as_yaml=False,
rebase_priv=False,
dry_run=False,
variant='okd',
)
return scanner

def _make_image_meta(self, for_payload=False, base_only=False, enabled=True, okd_mode=Missing):
"""Create a mock ImageMetadata with configurable attributes."""
meta = MagicMock(spec=ImageMetadata)
meta.enabled = enabled
meta.mode = 'enabled' if enabled else 'disabled'
meta.config = MagicMock()
meta.config.for_payload = for_payload
meta.config.base_only = base_only
if okd_mode is Missing:
meta.config.okd = Missing
else:
meta.config.okd = MagicMock()
meta.config.okd.mode = okd_mode
return meta

def test_okd_includes_payload_images(self):
"""OKD scan includes images with for_payload=true."""
scanner = self._make_okd_scanner()
meta = self._make_image_meta(for_payload=True)
self.assertTrue(scanner._is_image_enabled(meta))

def test_okd_includes_base_only_images(self):
"""OKD scan includes base_only images (needed for dependency chain)."""
scanner = self._make_okd_scanner()
meta = self._make_image_meta(base_only=True)
self.assertTrue(scanner._is_image_enabled(meta))

def test_okd_excludes_non_payload_non_base_images(self):
"""OKD scan excludes images that are neither payload nor base_only."""
scanner = self._make_okd_scanner()
meta = self._make_image_meta(for_payload=False, base_only=False)
self.assertFalse(scanner._is_image_enabled(meta))

def test_okd_scan_set_includes_base_only(self):
"""_is_image_enabled_for_scan includes base_only for OKD."""
scanner = self._make_okd_scanner()
meta = self._make_image_meta(base_only=True)
self.assertTrue(scanner._is_image_enabled_for_scan(meta))

def test_ocp_ignores_base_only_flag(self):
"""OCP variant returns image_meta.enabled regardless of base_only — behavior unchanged."""
meta = self._make_image_meta(base_only=True, for_payload=False)
# Default scanner is OCP variant — base_only/for_payload are irrelevant; enabled=True wins
self.assertTrue(self.scanner._is_image_enabled(meta))

def test_okd_includes_image_with_both_payload_and_base_only(self):
"""Image with both for_payload=True and base_only=True is included (OR semantics)."""
scanner = self._make_okd_scanner()
meta = self._make_image_meta(for_payload=True, base_only=True)
self.assertTrue(scanner._is_image_enabled(meta))

def test_okd_excludes_disabled_non_okd_image(self):
"""OKD scan excludes disabled images without okd.mode: enabled."""
scanner = self._make_okd_scanner()
meta = self._make_image_meta(for_payload=True, enabled=False)
self.assertFalse(scanner._is_image_enabled(meta))

def test_okd_includes_disabled_with_okd_mode_enabled(self):
"""OKD scan includes disabled images that have okd.mode: enabled + for_payload."""
scanner = self._make_okd_scanner()
meta = self._make_image_meta(for_payload=True, enabled=False, okd_mode='enabled')
self.assertTrue(scanner._is_image_enabled(meta))


class TestOkdArchChanges(TestScanSourcesKonflux):
"""Tests for OKD arch-change detection (ART-21083 Gap 1)."""

def _make_okd_scanner_with_arches(self, arches):
"""Create an OKD scanner with specific arches set on the runtime."""
self.runtime.arches = arches
self.runtime.build_system = 'konflux'
scanner = ConfigScanSources(
runtime=self.runtime,
ci_kubeconfig=self.ci_kubeconfig,
session=self.session,
as_yaml=False,
rebase_priv=False,
dry_run=False,
variant='okd',
)
return scanner

async def test_arch_expansion_triggers_rebuild(self):
"""Image built for x86_64 only should trigger rebuild when OKD targets x86_64+aarch64."""
scanner = self._make_okd_scanner_with_arches(['x86_64', 'aarch64'])

meta = MagicMock(spec=ImageMetadata)
meta.distgit_key = 'enterprise-base'
meta.qualified_key = 'image:enterprise-base'
meta.get_arches.return_value = ['x86_64', 'aarch64']

build_record = MagicMock(spec=KonfluxBuildRecord)
build_record.arches = ['x86_64']
build_record.nvr = 'enterprise-base-9-1.0'
scanner.latest_image_build_records_map = {'enterprise-base': build_record}
scanner.changing_image_names = set()

await scanner.scan_arch_changes(meta)

self.assertIn('enterprise-base', scanner.changing_image_names)
self.assertEqual(scanner.assessment_code['image:enterprise-base+True'], RebuildHintCode.ARCHES_CHANGE)

async def test_matching_arches_no_rebuild(self):
"""Image built for x86_64+aarch64 should NOT trigger when OKD targets the same."""
scanner = self._make_okd_scanner_with_arches(['x86_64', 'aarch64'])

meta = MagicMock(spec=ImageMetadata)
meta.distgit_key = 'enterprise-base'
meta.get_arches.return_value = ['x86_64', 'aarch64']

build_record = MagicMock(spec=KonfluxBuildRecord)
build_record.arches = ['x86_64', 'aarch64']
build_record.nvr = 'enterprise-base-9-1.0'
scanner.latest_image_build_records_map = {'enterprise-base': build_record}
scanner.changing_image_names = set()

await scanner.scan_arch_changes(meta)

self.assertNotIn('enterprise-base', scanner.changing_image_names)
self.assertEqual(scanner.assessment_code, {})

async def test_okd_two_arch_target_does_not_loop_on_four_arch_build(self):
"""OKD scanner targeting 2 arches should NOT flag a 2-arch build even though OCP group has 4."""
scanner = self._make_okd_scanner_with_arches(['x86_64', 'aarch64'])

meta = MagicMock(spec=ImageMetadata)
meta.distgit_key = 'enterprise-base'
# get_arches() is constrained by runtime.arches (CLI --arches override),
# so it returns 2 OKD arches, not 4 OCP group arches
meta.get_arches.return_value = ['x86_64', 'aarch64']

build_record = MagicMock(spec=KonfluxBuildRecord)
build_record.arches = ['x86_64', 'aarch64']
build_record.nvr = 'enterprise-base-9-1.0'
scanner.latest_image_build_records_map = {'enterprise-base': build_record}
scanner.changing_image_names = set()

await scanner.scan_arch_changes(meta)

self.assertNotIn('enterprise-base', scanner.changing_image_names)
self.assertEqual(scanner.assessment_code, {})
2 changes: 1 addition & 1 deletion pyartcd/pyartcd/pipelines/okd.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
reset_fail_counter,
)

OKD_ARCHES = ['x86_64', 'aarch64']
OKD_ARCHES = ('x86_64', 'aarch64')


class BuildPlan:
Expand Down
2 changes: 2 additions & 0 deletions pyartcd/pyartcd/pipelines/okd_scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from pyartcd import constants, jenkins, locks
from pyartcd.cli import cli, click_coroutine, pass_runtime
from pyartcd.locks import Lock
from pyartcd.pipelines.okd import OKD_ARCHES
from pyartcd.runtime import Runtime


Expand Down Expand Up @@ -79,6 +80,7 @@ def __init__(
f'--assembly={self.assembly}',
'--build-system=konflux',
'--variant=okd',
f'--arches={",".join(OKD_ARCHES)}',
]

async def run(self):
Expand Down
52 changes: 52 additions & 0 deletions pyartcd/tests/pipelines/test_okd_scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,58 @@ async def test_scan_uses_variant_okd(self, mock_cmd_gather, mock_jenkins):
cmd = cmd_calls[0][0][0] # First positional argument of first call
self.assertIn('--variant=okd', cmd)

@patch.dict(os.environ, {'KUBECONFIG': '/path/to/kubeconfig'})
@patch('pyartcd.pipelines.okd_scan.constants.OKD_ENABLED_VERSIONS', ['4.21'])
@patch('pyartcd.pipelines.okd_scan.jenkins')
@patch('pyartcd.pipelines.okd_scan.exectools.cmd_gather_async')
async def test_scan_passes_okd_arches(self, mock_cmd_gather, mock_jenkins):
"""
Test that okd-scan passes --arches with OKD_ARCHES to doozer.
This ensures arch-change detection uses OKD target arches, not OCP group arches.
"""
from pyartcd.pipelines.okd import OKD_ARCHES

self.runtime.dry_run = False

scan_output = yaml.dump({'images': []})
mock_cmd_gather.return_value = (0, scan_output, '')

pipeline = OkdScanPipeline(
runtime=self.runtime,
version='4.21',
data_path='https://github.com/openshift-eng/ocp-build-data',
assembly='stream',
data_gitref='',
image_list='',
)

await pipeline.run()

cmd_calls = mock_cmd_gather.call_args_list
self.assertEqual(len(cmd_calls), 1)
cmd = cmd_calls[0][0][0]
expected_arches_arg = f'--arches={",".join(OKD_ARCHES)}'
self.assertIn(expected_arches_arg, cmd)

def test_okd_scan_uses_same_arches_as_okd_build(self):
"""
Verify OKD_ARCHES is the single source of truth for both okd and okd-scan.
"""
from pyartcd.pipelines.okd import OKD_ARCHES

pipeline = OkdScanPipeline(
runtime=self.runtime,
version='4.21',
data_path='https://github.com/openshift-eng/ocp-build-data',
assembly='stream',
data_gitref='',
image_list='',
)

arches_in_cmd = [arg for arg in pipeline.doozer_base_command if arg.startswith('--arches=')]
self.assertEqual(len(arches_in_cmd), 1)
self.assertEqual(arches_in_cmd[0], f'--arches={",".join(OKD_ARCHES)}')


if __name__ == '__main__':
unittest.main()
Loading